tejohnson added inline comments.
================ Comment at: clang/lib/CodeGen/BackendUtil.cpp:1070-1071 + // ThinLTOIndexFile is provideds so we must be in ThinLTO PostLink. + // For -O0 ThinLTO PreLink does basic optimization and triggers + // OptimizerLastEPCallbacks. PostLink will not + // run optimizations and this callback should ---------------- vitalybuka wrote: > vitalybuka wrote: > > tejohnson wrote: > > > vitalybuka wrote: > > > > aeubanks wrote: > > > > > We could fix this. It doesn't make sense for only -O0 to run > > > > > OptimizerLastEPCallbacks prelink. Would that help simplify this? > > > > Do you mean move optimizer of -O0 into PostLink as well? > > > > Yes, this would be more consistent. > > > > Still I guess it should a separate patch. > > > > > > > Or does it make sense to go the other way, i.e. find a way to add the > > > sanitizer passes to the end of the pre-link of ThinLTO like for regular > > > LTO? The ThinLTO pre-link doesn't execute the module optimization > > > pipeline, but presumably at the end of buildThinLTOPreLinkDefaultPipeline > > > we could invoke the OptimizerLastEPCallbacks? That avoids the issue of > > > trying to get this working separately for in-process ThinLTO. And would > > > be more consistent with regular LTO. > > I started from PreLink approach but @aeubanks advised the PostLink. At this > > point I slightly prefer it over PreLink. > > > > Sanitizers In PostLink pros: > > + instrumentation of optimized code produces smaller binary > > + optimizer called after instrumentation may removes some checks and we > > miss some bugs (It's the case for non-LTO for at least msan. It stops > > reporting some bugs. Maybe just a bug in msan.) > > + StackSafetyAnalysis supports ThinLTO and can be used to optimize > > sanitizers. > > + consistent with non-LTO pipeline > > + OptimizerLastEPCallbacks called once per module (I guess not true for > > full LTO). > > > > > > Sanitizers In PreLink pros: > > + in-process ThinLTO. does not need special handling > > + Simpler patch > > + Can keep inconsistent -O0 pipeline (not sure why) > > + consistent with regular LTO (but why regular LTO is not consistent with > > ThinLTO here?) > > > > WDYT if ThinLTO PreLink with sanitizer use buildO0DefaultPipeline? > > Sanitizers In PostLink pros: > > + instrumentation of optimized code produces smaller binary > > Strangely it's an opposite. On our "internal large binary" I see smaller > Asan/Msan sizes if we instrument PreLink. > So this pro can be discarded. > > At this point I don't have strong preference. > > How can we agree which approach to use? > I like the approach of doing the instrumentation in the prelink for now. The consistency with O0 and with regular LTO is a bonus, and the ability to handle in-process ThinLTO is a big benefit. It may have some benefits as you found with the binary size because we can apply ThinLTO optimizations to the instrumented code. I'm curious - what is the issue with StackSafetyAnalysis when this is done in the pre link? 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
