[llvm] 3655f07 - Make dependency between certain analysis passes transitive

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Tue May 4 02:51:49 PDT 2021


Author: Bjorn Pettersson
Date: 2021-05-04T11:50:08+02:00
New Revision: 3655f0757f2b4b61419446b326410118658826ba

URL: https://github.com/llvm/llvm-project/commit/3655f0757f2b4b61419446b326410118658826ba
DIFF: https://github.com/llvm/llvm-project/commit/3655f0757f2b4b61419446b326410118658826ba.diff

LOG: Make dependency between certain analysis passes transitive

LazyBlockFrequenceInfoPass, LazyBranchProbabilityInfoPass and
LoopAccessLegacyAnalysis all cache pointers to their nestled required
analysis passes. One need to use addRequiredTransitive to describe
that the nestled passes can't be freed until those analysis passes
no longer are used themselves.

There is still a bit of a mess considering the getLazyBPIAnalysisUsage
and getLazyBFIAnalysisUsage functions. Those functions are used from
both Transform, CodeGen and Analysis passes. I figure it is OK to
use addRequiredTransitive also when being used from Transform and
CodeGen passes. On the other hand, I figure we must do it when
used from other Analysis passes. So using addRequiredTransitive should
be more correct here. An alternative solution would be to add a
bool option in those functions to let the user tell if it is a
analysis pass or not. Since those lazy passes will be obsolete when
new PM has conquered the world I figure we can leave it like this
right now.

Intention with the patch is to fix PR49950. It at least solves the
problem for the reproducer in PR49950. However, that reproducer
need five passes in a specific order, so there are lots of various
"solutions" that could avoid the crash without actually fixing the
root cause.

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

Added: 
    llvm/test/Other/pr49950.ll

Modified: 
    llvm/lib/Analysis/LazyBlockFrequencyInfo.cpp
    llvm/lib/Analysis/LazyBranchProbabilityInfo.cpp
    llvm/lib/Analysis/LoopAccessAnalysis.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/LazyBlockFrequencyInfo.cpp b/llvm/lib/Analysis/LazyBlockFrequencyInfo.cpp
index 6107cacb9533..636baf82eedf 100644
--- a/llvm/lib/Analysis/LazyBlockFrequencyInfo.cpp
+++ b/llvm/lib/Analysis/LazyBlockFrequencyInfo.cpp
@@ -45,8 +45,8 @@ void LazyBlockFrequencyInfoPass::getAnalysisUsage(AnalysisUsage &AU) const {
   // We require DT so it's available when LI is available. The LI updating code
   // asserts that DT is also present so if we don't make sure that we have DT
   // here, that assert will trigger.
-  AU.addRequired<DominatorTreeWrapperPass>();
-  AU.addRequired<LoopInfoWrapperPass>();
+  AU.addRequiredTransitive<DominatorTreeWrapperPass>();
+  AU.addRequiredTransitive<LoopInfoWrapperPass>();
   AU.setPreservesAll();
 }
 
@@ -61,8 +61,8 @@ bool LazyBlockFrequencyInfoPass::runOnFunction(Function &F) {
 
 void LazyBlockFrequencyInfoPass::getLazyBFIAnalysisUsage(AnalysisUsage &AU) {
   LazyBranchProbabilityInfoPass::getLazyBPIAnalysisUsage(AU);
-  AU.addRequired<LazyBlockFrequencyInfoPass>();
-  AU.addRequired<LoopInfoWrapperPass>();
+  AU.addRequiredTransitive<LazyBlockFrequencyInfoPass>();
+  AU.addRequiredTransitive<LoopInfoWrapperPass>();
 }
 
 void llvm::initializeLazyBFIPassPass(PassRegistry &Registry) {

diff  --git a/llvm/lib/Analysis/LazyBranchProbabilityInfo.cpp b/llvm/lib/Analysis/LazyBranchProbabilityInfo.cpp
index 83698598e156..95de4949b21c 100644
--- a/llvm/lib/Analysis/LazyBranchProbabilityInfo.cpp
+++ b/llvm/lib/Analysis/LazyBranchProbabilityInfo.cpp
@@ -46,9 +46,9 @@ void LazyBranchProbabilityInfoPass::getAnalysisUsage(AnalysisUsage &AU) const {
   // We require DT so it's available when LI is available. The LI updating code
   // asserts that DT is also present so if we don't make sure that we have DT
   // here, that assert will trigger.
-  AU.addRequired<DominatorTreeWrapperPass>();
-  AU.addRequired<LoopInfoWrapperPass>();
-  AU.addRequired<TargetLibraryInfoWrapperPass>();
+  AU.addRequiredTransitive<DominatorTreeWrapperPass>();
+  AU.addRequiredTransitive<LoopInfoWrapperPass>();
+  AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
   AU.setPreservesAll();
 }
 
@@ -63,9 +63,9 @@ bool LazyBranchProbabilityInfoPass::runOnFunction(Function &F) {
 }
 
 void LazyBranchProbabilityInfoPass::getLazyBPIAnalysisUsage(AnalysisUsage &AU) {
-  AU.addRequired<LazyBranchProbabilityInfoPass>();
-  AU.addRequired<LoopInfoWrapperPass>();
-  AU.addRequired<TargetLibraryInfoWrapperPass>();
+  AU.addRequiredTransitive<LazyBranchProbabilityInfoPass>();
+  AU.addRequiredTransitive<LoopInfoWrapperPass>();
+  AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
 }
 
 void llvm::initializeLazyBPIPassPass(PassRegistry &Registry) {

diff  --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index cd086328ec68..35350582b4b5 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2281,12 +2281,12 @@ bool LoopAccessLegacyAnalysis::runOnFunction(Function &F) {
 }
 
 void LoopAccessLegacyAnalysis::getAnalysisUsage(AnalysisUsage &AU) const {
-    AU.addRequired<ScalarEvolutionWrapperPass>();
-    AU.addRequired<AAResultsWrapperPass>();
-    AU.addRequired<DominatorTreeWrapperPass>();
-    AU.addRequired<LoopInfoWrapperPass>();
+  AU.addRequiredTransitive<ScalarEvolutionWrapperPass>();
+  AU.addRequiredTransitive<AAResultsWrapperPass>();
+  AU.addRequiredTransitive<DominatorTreeWrapperPass>();
+  AU.addRequiredTransitive<LoopInfoWrapperPass>();
 
-    AU.setPreservesAll();
+  AU.setPreservesAll();
 }
 
 char LoopAccessLegacyAnalysis::ID = 0;

diff  --git a/llvm/test/Other/pr49950.ll b/llvm/test/Other/pr49950.ll
new file mode 100644
index 000000000000..7a65d2670cc0
--- /dev/null
+++ b/llvm/test/Other/pr49950.ll
@@ -0,0 +1,78 @@
+; RUN: opt < %s -o /dev/null -enable-new-pm=0 -block-freq -opt-remark-emitter -memoryssa -inject-tli-mappings -pgo-memop-opt -verify-loop-info -debug-pass=Details 2>&1 | FileCheck %s
+
+; REQUIRES: asserts
+
+; This is a heavily reduced reproducer for the problem found in
+; https://bugs.llvm.org/show_bug.cgi?id=49950 when doing fuzzy
+; testing (including non-standard pipelines).
+;
+; The problem manifested as having a pass structure like this
+; when it failed (as given by using -debug-pass=Details):
+;
+;   Target Library Information
+;   Target Transform Information
+;   Profile summary info
+;   Assumption Cache Tracker
+;     ModulePass Manager
+;       FunctionPass Manager
+;         Dominator Tree Construction
+;         Natural Loop Information
+;         Post-Dominator Tree Construction
+;         Branch Probability Analysis
+;         Block Frequency Analysis
+;   --      Branch Probability Analysis
+;         Lazy Branch Probability Analysis
+;         Lazy Block Frequency Analysis
+;         Optimization Remark Emitter
+;         Basic Alias Analysis (stateless AA impl)
+;         Function Alias Analysis Results
+;         Memory SSA
+;   --      Dominator Tree Construction
+;   --      Function Alias Analysis Results
+;   --      Basic Alias Analysis (stateless AA impl)
+;   --      Memory SSA
+;         Inject TLI Mappings
+;   --      Inject TLI Mappings
+;         PGOMemOPSize
+;   --      Block Frequency Analysis
+;   --      Post-Dominator Tree Construction
+;   --      Optimization Remark Emitter
+;   --      Lazy Branch Probability Analysis
+;   --      Natural Loop Information
+;   --      Lazy Block Frequency Analysis
+;   --      PGOMemOPSize
+;         Module Verifier
+;   --      Module Verifier
+;   --    Target Library Information
+;   --    Profile summary info
+;   --    Assumption Cache Tracker
+;       Bitcode Writer
+;   --    Bitcode Writer
+;
+; One might notice that "Dominator Tree Construction" is dropped after
+; "Memory SSA", while for example "Natural Loop Information" stick around
+; a bit longer. This despite "Dominator Tree Construction" being transitively
+; required by "Natural Loop Information".
+; The end result was that we got crashes when doing verification of loop
+; info after "Inject TLI Mappings" (since the dominator tree had been
+; removed too early).
+
+; Verify that both domintator tree and loop info are kept until after
+; PGOMemOPSize:
+;
+; CHECK:     Dominator Tree Construction
+; CHECK-NOT: --      Dominator Tree Construction
+; CHECK:     Memory SSA
+; CHECK-NOT: --      Dominator Tree Construction
+; CHECK:     Inject TLI Mappings
+; CHECK-NOT: --      Dominator Tree Construction
+; CHECK:     PGOMemOPSize
+; CHECK-DAG: --      Dominator Tree Construction
+; CHECK-DAG: --      Natural Loop Information
+; CHECK-DAG: --      PGOMemOPSize
+; CHECK:     Bitcode Writer
+
+define void @foo() {
+entry:
+  ret void
+}
\ No newline at end of file


        


More information about the llvm-commits mailing list