From 81d6310da1fc54f0ca0de6fa13246d6071edb0cf Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Thu, 16 Mar 2023 02:00:47 +0100 Subject: [PATCH] [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 --- llvm/include/llvm/Analysis/LoopAccessAnalysis.h | 3 + llvm/lib/Analysis/LoopAccessAnalysis.cpp | 18 ++++++ .../Analysis/LoopAccessAnalysis/invalidation.ll | 64 ++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 llvm/test/Analysis/LoopAccessAnalysis/invalidation.ll diff --git a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h index a49e24a..7596193 100644 --- a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h +++ b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h @@ -785,6 +785,9 @@ public: 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 9e11056..aee14ed 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(); + if (!PAC.preserved() && !PAC.preservedSet>()) + // 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(F, PA) || + Inv.invalidate(F, PA) || + Inv.invalidate(F, PA) || + Inv.invalidate(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 0000000..fb3af60 --- /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,invalidate),print' 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,invalidate)' 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,invalidate)' 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,invalidate)' 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 +} -- 2.7.4