[PM] Replace the hard invalidate in JumpThreading for LVI with correct

invalidation of deleted functions in GlobalDCE.

This was always testing a bug really triggered in GlobalDCE. Right now
we have analyses with asserting value handles into IR. As long as those
remain, when *deleting* an IR unit, we cannot wait for the normal
invalidation scheme to kick in even though it was designed to work
correctly in the face of these kinds of deletions. Instead, the pass
needs to directly handle invalidating the analysis results pointing at
that IR unit.

I've tought the Inliner about this and this patch teaches GlobalDCE.
This will handle the asserting VH case in the existing test as well as
other issues of the same fundamental variety. I've moved the test into
the GlobalDCE directory and added a comment explaining what is going on.

Note that we cannot simply require LVI here because LVI is too lazy.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@292773 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/IPO/GlobalDCE.cpp b/lib/Transforms/IPO/GlobalDCE.cpp
index 7a04de3d12dbfc589f3422d6be873eea464062cf..9304e783d49256dd9843a00eb4551f730f213b59 100644 (file)
--- a/lib/Transforms/IPO/GlobalDCE.cpp
+++ b/lib/Transforms/IPO/GlobalDCE.cpp
@@ -50,7 +50,14 @@ namespace {
       if (skipModule(M))
         return false;
 
+      // We need a minimally functional dummy module analysis manager. It needs
+      // to at least know about the possibility of proxying a function analysis
+      // manager.
+      FunctionAnalysisManager DummyFAM;
       ModuleAnalysisManager DummyMAM;
+      DummyMAM.registerPass(
+          [&] { return FunctionAnalysisManagerModuleProxy(DummyFAM); });
+
       auto PA = Impl.run(M, DummyMAM);
       return !PA.areAllPreserved();
     }
@@ -78,7 +85,7 @@ static bool isEmptyFunction(Function *F) {
   return RI.getReturnValue() == nullptr;
 }
 
-PreservedAnalyses GlobalDCEPass::run(Module &M, ModuleAnalysisManager &) {
+PreservedAnalyses GlobalDCEPass::run(Module &M, ModuleAnalysisManager &MAM) {
   bool Changed = false;
 
   // Remove empty functions from the global ctors list.
@@ -137,13 +144,29 @@ PreservedAnalyses GlobalDCEPass::run(Module &M, ModuleAnalysisManager &) {
       }
     }
 
+  // Because we may have cached analyses for functions we take extra care when
+  // deleting them if there is an active proxy. If there isn't, then we get to
+  // assume that everything in the function AM has been cleared already.
+  // FIXME: Note that all of this will happen automatically when this pass
+  // finishes. Unfortuantely there are analyses which hold asserting VHs to
+  // IR units. We could make those weak VHs that would assert if ever used
+  // without asserting eagerly and then all of this knowledge of the analysis
+  // manager could go away.
+  FunctionAnalysisManager *FAM = nullptr;
+  if (auto *FAMProxy =
+          MAM.getCachedResult<FunctionAnalysisManagerModuleProxy>(M))
+    FAM = &FAMProxy->getManager();
+
   // The second pass drops the bodies of functions which are dead...
   std::vector<Function *> DeadFunctions;
   for (Function &F : M)
     if (!AliveGlobals.count(&F)) {
       DeadFunctions.push_back(&F);         // Keep track of dead globals
-      if (!F.isDeclaration())
+      if (!F.isDeclaration()) {
+        if (FAM)
+          FAM->clear(F);
         F.deleteBody();
+      }
     }
 
   // The third pass drops targets of aliases which are dead...

diff --git a/lib/Transforms/Scalar/JumpThreading.cpp b/lib/Transforms/Scalar/JumpThreading.cpp
index 1870c3deb4f339f414ee151f649aab268975f9db..ecc6a0afbad89f03f22be143a1c11d85cc5fd849 100644 (file)
--- a/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/lib/Transforms/Scalar/JumpThreading.cpp
@@ -149,10 +149,6 @@ PreservedAnalyses JumpThreadingPass::run(Function &F,
   bool Changed =
       runImpl(F, &TLI, &LVI, HasProfileData, std::move(BFI), std::move(BPI));
 
-  // FIXME: We need to invalidate LVI to avoid PR28400. Is there a better
-  // solution?
-  AM.invalidate<LazyValueAnalysis>(F);
-
   if (!Changed)
     return PreservedAnalyses::all();
   PreservedAnalyses PA;

diff --git a/test/Other/new-pm-defaults.ll b/test/Other/new-pm-defaults.ll
index 713cc022974a40a2605404489438f04354a822a5..225b49afbf71249e57774a367ebb8b075636fb61 100644 (file)
--- a/test/Other/new-pm-defaults.ll
+++ b/test/Other/new-pm-defaults.ll
@@ -73,9 +73,7 @@
 ; CHECK-O-NEXT: Running pass: SpeculativeExecutionPass
 ; CHECK-O-NEXT: Running pass: JumpThreadingPass
 ; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
-; CHECK-O-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: CorrelatedValuePropagationPass
-; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
@@ -111,9 +109,7 @@
 ; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running pass: JumpThreadingPass
-; CHECK-O-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: CorrelatedValuePropagationPass
-; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: DSEPass
 ; CHECK-O-NEXT: Running pass: ADCEPass
 ; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis

diff --git a/test/Transforms/JumpThreading/crash-assertingvh.ll b/test/Transforms/GlobalDCE/crash-assertingvh.ll
similarity index 64%
rename from test/Transforms/JumpThreading/crash-assertingvh.ll
rename to test/Transforms/GlobalDCE/crash-assertingvh.ll
index e784319922396bc639314cc6dd8759474665b7f1..184d6ae7d8e9600b5aba85010ee74ab1858d9c3b 100644 (file)
--- a/test/Transforms/JumpThreading/crash-assertingvh.ll
+++ b/test/Transforms/GlobalDCE/crash-assertingvh.ll
@@ -1,3 +1,7 @@
+; Make sure that if a pass like jump threading populates a function analysis
+; like LVI with asserting handles into the body of a function, those don't begin
+; to assert when global DCE deletes the body of the function.
+;
 ; RUN: opt -disable-output < %s -passes='module(function(jump-threading),globaldce)'
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"