[PATCH] D84959: [NewPM][LVI] Abandon LVI after CVP

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

I had a random thought, maybe CVP itself shouldn't be invalidating LVI, but rather it should be explicitly invalidated in `PassBuilder::buildFunctionSimplificationPipeline()` with a `InvalidateAnalysisPass<LazyValueAnalysis>`. Since the description mentions that this is based on the knowledge that no passes after CVP will use LVI. ???



================
Comment at: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll:233
+; CHECK-O3-NEXT: Invalidating analysis: RegionInfoAnalysis
+; CHECK-O23SZ-NEXT: Clearing all analysis results for: foo
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
----------------
tejohnson wrote:
> aeubanks wrote:
> > nikic wrote:
> > > The test diffs looks reasonable to me now, apart from this part. Is this expected?
> > That is weird, I'll take a look.
> I was just looking at this. The message "Clearing all analysis results for: <possibly invalidated loop>" comes from the callsite to AnalysisManager<IRUnitT, ExtraArgTs...>::clear from LoopAnalysisManagerFunctionProxy::Result::invalidate. Looking at the conditions for that call, I'm guessing that the following is no longer true:
> PAC.preservedSet<AllAnalysesOn<Function>>() 
> since the abandon() call will remove that ID from the preserved set.
> 
> So it is probably "expected" by the code change here. Not sure if there is a way to avoid it, i.e. is that code overly conservative.
`DominatorTree::invalid()` checks `PAC.preserved() || PAC.preservedSet<AllAnalysesOn<Function>>() || PAC.preservedSet<CFGAnalyses>()`. Even though `PAC.preservedSet<AllAnalysesOn<Function>>()` will be false, `PAC.preserved()` should still be true.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84959/new/

https://reviews.llvm.org/D84959