tejohnson added inline comments.
================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1066
PassBuilder::OptimizationLevel Level)
{
+ if (CodeGenOpts.OptimizationLevel == 0) {
+ if (!CodeGenOpts.ThinLTOIndexFile.empty())
----------------
Some comments around the cases being checked in this if-then-else would be
helpful.
================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1328
+ CodeGenOpts.ThinLTOIndexFile.empty()) {
+ // This is testing distributed ThinLTO PostLink. O0 called optimized in
+ // PreLink.
----------------
I don't understand what you mean by "O0 called optimized"?
Also maybe make it clear that the first sentence goes with the second check?
Also, it isn't clear to me how this is preventing the sanitizers from being
added in the ThinLTO pre-link compiles for optimized compiles, as shown in your
tests. Unlike regular LTO pre-link optimized builds, which are still getting
the sanitizers.
================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1541
+ Triple TargetTriple(M->getTargetTriple());
+ Conf.OptimizerLastPassBuilderHook = [&](PassBuilder &PB) {
+ addSanitizers(TargetTriple, CGOpts, LOpts, PB);
----------------
This will add sanitizers for the ThinLTO post-link compiles in a distributed
build environment. Does something need to set these up for ThinLTO post-link
compiles in the in-process ThinLTO case? In that case thinBackend() is invoked
via LTO.cpp through the linker.
================
Comment at: llvm/include/llvm/LTO/Config.h:51
std::vector<std::string> PassPlugins;
- /// For adding passes that run right before codegen.
+ /// For adding passes that run by optimizer.
+ std::function<void(PassBuilder &)> OptimizerLastPassBuilderHook;
----------------
This one should say NewPM only.
================
Comment at: llvm/include/llvm/LTO/Config.h:53
+ std::function<void(PassBuilder &)> OptimizerLastPassBuilderHook;
+ /// For adding passes that run right before codegen (NewPM only).
std::function<void(legacy::PassManager &)> PreCodeGenPassesHook;
----------------
This one should say legacy PM only
================
Comment at: llvm/include/llvm/LTO/Config.h:54
+ /// For adding passes that run by opt.
+ std::function<void(PassBuilder &)> OptPassBuilderHook;
Optional<Reloc::Model> RelocModel = Reloc::PIC_;
----------------
vitalybuka wrote:
> aeubanks wrote:
> > vitalybuka wrote:
> > > tejohnson wrote:
> > > > Is this essentially the new PM equivalent to PreCodeGenPassesHook above
> > > > (since this new hook runs at the end of opt)?
> > > > Also, might be good to name this like OptimizerLastPassBuilderHook or
> > > > something like that to indicate its position within the opt pipeline.
> > > Kind of. PreCodeGenPassesHook is codegen and it's Legacy PM only.
> > I think the idea is that this callback adds PassBuilder callbacks in
> > various parts of the the pipeline. Perhaps just `PassBuilderHook`? It's
> > weird that we only allow one, but I guess we can make it a vector in the
> > future if necessary.
> I guess the point of OptimizerLastPassBuilderHook name is to show that this
> is not any PassBuilder, but the one used in ::opt()
My point was just that they are effectively at the same place (the end of opt
is pretty much the beginning of codegen), so I was wondering if it was worth
making the legacy and new PM hooks be at the same place. But it probably
doesn't matter much.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96456/new/
https://reviews.llvm.org/D96456
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
