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
----------------
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?
================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1079
+ if (CodeGenOpts.PrepareForThinLTO &&
+ CodeGenOpts.ThinLTOIndexFile.empty()) {
+ // We are here if we in ThinLTO PreLink.
----------------
tejohnson wrote:
> You should only have to check PrepareForThinLTO to see if we are in the
> pre-link.
It needed for clang/test/CodeGen/cspgo-instrumentation_thinlto.c:26:53
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
