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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope marked an inline comment as done.
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
----------------
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.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll:75
+
+attributes #0 = { nofree norecurse nosync nounwind memory(argmem: write) }
----------------
fhahn wrote:
> are those needed?
Last I checked it was needed to get make the RUN on line 8 to crash. But since it crashes due to dangling references/pointers that might be a bit of a coincidence.

In order to just validate that the pass manager is doing invalidations, then I think the actual IR isn't important at all (i.e. we could remove more things and not only the attributes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146206