[PATCH] D85109: [NewPM] Clear abandoned analyses set when preserveSet is called

[Yuanfang Chen via Phabricator via llvm-commits]

ychen updated this revision to Diff 282514.
ychen added a comment.
Herald added subscribers: dexonsmith, steven_wu, hiraditya.

- update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85109

Files:
  llvm/include/llvm/IR/PassManager.h
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/unittests/IR/PassManagerTest.cpp


Index: llvm/unittests/IR/PassManagerTest.cpp
===================================================================
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -396,10 +396,19 @@
   PA.abandon<TestModuleAnalysis>();
   EXPECT_FALSE(PA.getChecker<TestModuleAnalysis>().preserved());
 
-  // Even if the sets are preserved, the abandoned analyses' checker won't
-  // return true for those sets.
+  // If the sets are preserved after the analyses are abandoned, the abandoned
+  // analyses' checker will return true for those sets.
   PA.preserveSet<AllAnalysesOn<Function>>();
   PA.preserveSet<AllAnalysesOn<Module>>();
+  EXPECT_TRUE(PA.getChecker<TestFunctionAnalysis>()
+                  .preservedSet<AllAnalysesOn<Function>>());
+  EXPECT_TRUE(PA.getChecker<TestModuleAnalysis>()
+                  .preservedSet<AllAnalysesOn<Module>>());
+
+  // If the sets are preserved before the analyses are abandoned, the abandoned
+  // analyses' checker will return false for those sets.
+  PA.abandon<TestFunctionAnalysis>();
+  PA.abandon<TestModuleAnalysis>();
   EXPECT_FALSE(PA.getChecker<TestFunctionAnalysis>()
                    .preservedSet<AllAnalysesOn<Function>>());
   EXPECT_FALSE(PA.getChecker<TestModuleAnalysis>()
Index: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
===================================================================
--- llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
@@ -216,26 +216,10 @@
 ; CHECK-O-NEXT: Finished CGSCC pass manager run.
 ; CHECK-O-NEXT: Finished {{.*}}Module pass manager run.
 ; CHECK-O-NEXT: Finished {{.*}}Module pass manager run.
-; CHECK-O23SZ-NEXT: Clearing all analysis results for: <possibly invalidated loop>
-; CHECK-O23SZ-NEXT: Invalidating analysis: DominatorTreeAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: MemorySSAAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: LoopAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: PostDominatorTreeAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: BranchProbabilityAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: BlockFrequencyAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: ScalarEvolutionAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: InnerAnalysisManagerProxy
-; CHECK-O23SZ-NEXT: Invalidating analysis: PhiValuesAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: MemoryDependenceAnalysis
-; CHECK-O23SZ-NEXT: Invalidating analysis: DemandedBitsAnalysis
-; CHECK-O3-NEXT: Invalidating analysis: DominanceFrontierAnalysis
-; CHECK-O3-NEXT: Invalidating analysis: RegionInfoAnalysis
-; CHECK-O23SZ-NEXT: Clearing all analysis results for: foo
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
 ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis on bar
 ; CHECK-EXT: Running pass: {{.*}}::Bye
 ; CHECK-O-NEXT: Finished {{.*}}Module pass manager run.
-; CHECK-O23SZ-NEXT: Clearing all analysis results for: foo
 ; CHECK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-O-NEXT: Running pass: PrintModulePass
 
Index: llvm/include/llvm/IR/PassManager.h
===================================================================
--- llvm/include/llvm/IR/PassManager.h
+++ llvm/include/llvm/IR/PassManager.h
@@ -186,9 +186,22 @@
       PreservedIDs.insert(ID);
   }
 
-  /// Mark an analysis set as preserved.
+  /// Mark an analysis set as preserved. This is only called by inner pass
+  /// manager to tell outer pass manager that inner analyses are up-to-date.
+  /// Regular passes should not call this.
+  /// FIXME: rename this to `preserveSetAndClearAbandonedAnalyses`?
   template <typename AnalysisSetT> void preserveSet() {
     preserveSet(AnalysisSetT::ID());
+    NotPreservedAnalysisIDs.clear();
+  }
+
+  // TODO: Using `preserve<CFGAnalyses>` instead so we could add comment to
+  // preserveSet saying that it is restricted to usage introduced in D23691.
+  // `preserveSet<CFGAnalyses>` is semantically correct but
+  // `preserve<CFGAnalyses>` helps reason about the code which IMHO is more
+  // important.
+  template <> void preserveSet<CFGAnalyses>() {
+    preserveSet(CFGAnalyses::ID());
   }
 
   /// Mark an analysis set as preserved using its ID.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85109.282514.patch
Type: text/x-patch
Size: 4224 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200803/962b53c6/attachment-0001.bin>