[llvm] 81d6310 - [LAA] Fix transitive analysis invalidation bug by implementing LoopAccessInfoManager::invalidate

[Bjorn Pettersson via llvm-commits]

Author: Bjorn Pettersson
Date: 2023-03-17T09:33:16+01:00
New Revision: 81d6310da1fc54f0ca0de6fa13246d6071edb0cf

URL: https://github.com/llvm/llvm-project/commit/81d6310da1fc54f0ca0de6fa13246d6071edb0cf
DIFF: https://github.com/llvm/llvm-project/commit/81d6310da1fc54f0ca0de6fa13246d6071edb0cf.diff

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

The default invalidate method for analysis results is just looking
at the preserved state of the pass itself. It does not consider if
the analysis has an internal state that depend on other analyses.
Thus, we need to implement LoopAccessInfoManager::invalidate in order
to catch if LoopAccessAnalysis needs to be invalidated due to
transitive analyses such as AAManager is being invalidated. Otherwise
we might end up having references to an AAManager that is stale.

Fixes https://github.com/llvm/llvm-project/issues/61324

Differential Revision: https://reviews.llvm.org/D146206

Added: 
    llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll

Modified: 
    llvm/include/llvm/Analysis/LoopAccessAnalysis.h
    llvm/lib/Analysis/LoopAccessAnalysis.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
index a49e24ada4406..759619396ccb8 100644
--- a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
+++ b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
@@ -785,6 +785,9 @@ class LoopAccessInfoManager {
   const LoopAccessInfo &getInfo(Loop &L);
 
   void clear() { LoopAccessInfoMap.clear(); }
+
+  bool invalidate(Function &F, const PreservedAnalyses &PA,
+                  FunctionAnalysisManager::Invalidator &Inv);
 };
 
 /// This analysis provides dependence information for the memory accesses

diff  --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 9e110567e98e4..aee14edd3fd63 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2704,6 +2704,24 @@ void LoopAccessLegacyAnalysis::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
 }
 
+bool LoopAccessInfoManager::invalidate(
+    Function &F, const PreservedAnalyses &PA,
+    FunctionAnalysisManager::Invalidator &Inv) {
+  // Check whether our analysis is preserved.
+  auto PAC = PA.getChecker<LoopAccessAnalysis>();
+  if (!PAC.preserved() && !PAC.preservedSet<AllAnalysesOn<Function>>())
+    // If not, give up now.
+    return true;
+
+  // Check whether the analyses we depend on became invalid for any reason.
+  // Skip checking TargetLibraryAnalysis as it is immutable and can't become
+  // invalid.
+  return Inv.invalidate<AAManager>(F, PA) ||
+         Inv.invalidate<ScalarEvolutionAnalysis>(F, PA) ||
+         Inv.invalidate<LoopAnalysis>(F, PA) ||
+         Inv.invalidate<DominatorTreeAnalysis>(F, PA);
+}
+
 LoopAccessInfoManager LoopAccessAnalysis::run(Function &F,
                                               FunctionAnalysisManager &AM) {
   return LoopAccessInfoManager(

diff  --git a/llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll b/llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll
new file mode 100644
index 0000000000000..fb3af609dd2c6
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll
@@ -0,0 +1,64 @@
+; Test that access-info gets invalidated when the analyses it depends on are
+; invalidated.
+
+; This is a reproducer for https://github.com/llvm/llvm-project/issues/61324.
+; We want to see that LoopAccessAnalysis+AAManger is being updated at the end,
+; instead of crashing when using a stale AAManager.
+;
+; RUN: opt < %s -disable-output -debug-pass-manager -passes='function(require<access-info>,invalidate<aa>),print<access-info>' 2>&1 | FileCheck %s --check-prefix=CHECK-AA
+;
+; CHECK-AA: Running pass: RequireAnalysisPass
+; CHECK-AA-NEXT: Running analysis: LoopAccessAnalysis on foo
+; CHECK-AA: Running pass: InvalidateAnalysisPass
+; CHECK-AA-NEXT: Invalidating analysis: AAManager on foo
+; CHECK-AA-NEXT: Invalidating analysis: LoopAccessAnalysis on foo
+; CHECK-AA-NEXT: Running pass: LoopAccessInfoPrinterPass on foo
+; CHECK-AA-NEXT: Running analysis: LoopAccessAnalysis on foo
+; CHECK-AA-NEXT: Running analysis: AAManager on foo
+
+
+; Verify that an explicit invalidate request for access-info result in an
+; invalidation.
+;
+; RUN: opt < %s -disable-output -debug-pass-manager -passes='function(require<access-info>,invalidate<access-info>)' 2>&1 | FileCheck %s --check-prefix=CHECK-INV-AA
+;
+; CHECK-INV-AA: Running pass: RequireAnalysisPass
+; CHECK-INV-AA-NEXT: Running analysis: LoopAccessAnalysis on foo
+; CHECK-INV-AA: Running pass: InvalidateAnalysisPass
+; CHECK-INV-AA-NEXT: Invalidating analysis: LoopAccessAnalysis on foo
+
+
+; Invalidation of scalar-evolution should transitively invalidate access-info.
+;
+; RUN: opt < %s -disable-output -debug-pass-manager -passes='function(require<access-info>,invalidate<scalar-evolution>)' 2>&1 | FileCheck %s --check-prefix=CHECK-SCEV
+;
+; CHECK-SCEV: Running pass: RequireAnalysisPass
+; CHECK-SCEV-NEXT: Running analysis: LoopAccessAnalysis on foo
+; CHECK-SCEV: Running pass: InvalidateAnalysisPass
+; CHECK-SCEV-NEXT: Invalidating analysis: ScalarEvolutionAnalysis on foo
+; CHECK-SCEV-NEXT: Invalidating analysis: LoopAccessAnalysis on foo
+
+
+; Invalidation of domtree should transitively invalidate access-info.
+;
+; RUN: opt < %s -disable-output -debug-pass-manager -passes='function(require<access-info>,invalidate<domtree>)' 2>&1 | FileCheck %s --check-prefix=CHECK-DT
+;
+; CHECK-DT: Running pass: RequireAnalysisPass
+; CHECK-DT-NEXT: Running analysis: LoopAccessAnalysis on foo
+; CHECK-DT: Running pass: InvalidateAnalysisPass
+; CHECK-DT-NEXT: Invalidating analysis: DominatorTreeAnalysis on foo
+; CHECK-DT: Invalidating analysis: LoopAccessAnalysis on foo
+
+
+define void @foo(ptr nocapture writeonly %a, ptr nocapture writeonly %b) memory(argmem: write) {
+entry:
+  br label %for.cond1
+
+for.cond1:
+  store i16 0, ptr %b, align 1
+  store i16 0, ptr %a, align 1
+  br i1 true, label %for.end6, label %for.cond1
+
+for.end6:
+  ret void
+}