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

Fri Mar 17 01:36:41 PDT 2023

bjope added inline comments.

================
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:
> > 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)
> Ok. That sounds like a reasonable cause. I'll fix that for LoopAccessAnalysis::run.
FWIW, I made a follow-up commit here: https://reviews.llvm.org/rG951a980dc7aa61b1d414e7d565166ee8071367c6
It should make the analysis execution order more consistent, at least for analysis passes running other analyses.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146206