vitalybuka 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: > 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? 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