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

Reply via email to