vitalybuka added inline comments.
================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1328 + CodeGenOpts.ThinLTOIndexFile.empty()) { + // This is testing distributed ThinLTO PostLink. O0 called optimized in + // PreLink. ---------------- tejohnson wrote: > 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. Reworded: OptimizerLastEPCallbacks is good for anything that not SkipThinLTOPostLink We need protection only for OptimizerLastEPCallbacks+O0 ================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1541 + Triple TargetTriple(M->getTargetTriple()); + Conf.OptimizerLastPassBuilderHook = [&](PassBuilder &PB) { + addSanitizers(TargetTriple, CGOpts, LOpts, PB); ---------------- tejohnson wrote: > 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. Yes. And I don't have good idea how to handle that. Maybe addSanitizer() needs to be moved into PassBuilder, but not sure how to get CodeGenOpts and LangOpts there. For now this patch does not brake that case as it's already broken. Maybe we can resolve this later if needed. ================ 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; ---------------- tejohnson wrote: > This one should say legacy PM only Codegen one is invoked on codegen PM which is different from opt PM For now codegen PM is always legacy 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