[PATCH] D146206: [LAA] Fix transitive analysis invalidation bug by implementing LoopAccessInfoManager::invalidate

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks accepted this revision.
aeubanks added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll:38
+; CHECK-SCEV: Running pass: InvalidateAnalysisPass
+; CHECK-SCEV-NEXT: Invalidating analysis: ScalarEvolutionAnalysis on foo
+; CHECK-SCEV-NEXT: Invalidating analysis: LoopAccessAnalysis on foo
----------------
I was wondering if the order of invalidation is guaranteed to be consistent, and I think it is as long as the order we fetch analyses is consistent because `AnalysisResultListT` is a `std::list`


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll:8-11
+; CHECK-AA-DAG: Running analysis: ScalarEvolutionAnalysis on foo
+; CHECK-AA-DAG: Running analysis: DominatorTreeAnalysis on foo
+; CHECK-AA-DAG: Running analysis: LoopAnalysis on foo
+; CHECK-AA-DAG: Running analysis: AAManager on foo
----------------
bjope wrote:
> aeubanks wrote:
> > can we make this consistent by making the call order consistent?
> I don't know why analyses tend to appear in different order (both when running and invalidating), e.g. when comparing an opt built using clang or with gcc. Maybe some set iteration that isn't guaranteeing iteration order.
> 
> I've updated this patch to use a bit less DAG checks. I don't think it was important to check everything.
> 
> But it is an interesting idea for the future to see if we can get it more consistent (lots of tests that are checking "Running analysis:" and "Invalidating analysis:" seem to use -DAG, which I think is a bit easy to forget when adding new tests).
it's the order we call sub-analyses in `LoopAccessAnalysis::run`. the evaluation order of arguments is not necessarily consistent across all platforms. fetching them before the call to `LoopAccessInfoManager` should work (I've done that for other passes/analyses before)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146206