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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 00:33:25 PST 2017


Author: chandlerc
Date: Mon Jan 23 02:33:24 2017
New Revision: 292773

URL: http://llvm.org/viewvc/llvm-project?rev=292773&view=rev
Log:
[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.

Added:
    llvm/trunk/test/Transforms/GlobalDCE/crash-assertingvh.ll
      - copied, changed from r292772, llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll
Removed:
    llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/GlobalDCE.cpp
    llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
    llvm/trunk/test/Other/new-pm-defaults.ll

Modified: llvm/trunk/lib/Transforms/IPO/GlobalDCE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalDCE.cpp?rev=292773&r1=292772&r2=292773&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalDCE.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalDCE.cpp Mon Jan 23 02:33:24 2017
@@ -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(Mod
       }
     }
 
+  // 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...

Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=292773&r1=292772&r2=292773&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Mon Jan 23 02:33:24 2017
@@ -149,10 +149,6 @@ PreservedAnalyses JumpThreadingPass::run
   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;

Modified: llvm/trunk/test/Other/new-pm-defaults.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/new-pm-defaults.ll?rev=292773&r1=292772&r2=292773&view=diff
==============================================================================
--- llvm/trunk/test/Other/new-pm-defaults.ll (original)
+++ llvm/trunk/test/Other/new-pm-defaults.ll Mon Jan 23 02:33:24 2017
@@ -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

Copied: llvm/trunk/test/Transforms/GlobalDCE/crash-assertingvh.ll (from r292772, llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll)
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalDCE/crash-assertingvh.ll?p2=llvm/trunk/test/Transforms/GlobalDCE/crash-assertingvh.ll&p1=llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll&r1=292772&r2=292773&rev=292773&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll (original)
+++ llvm/trunk/test/Transforms/GlobalDCE/crash-assertingvh.ll Mon Jan 23 02:33:24 2017
@@ -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"

Removed: llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll?rev=292772&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/crash-assertingvh.ll (removed)
@@ -1,19 +0,0 @@
-; 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"
-target triple = "x86_64-unknown-linux-gnu"
-
-declare i32 @bar()
-
-define internal i32 @foo() {
-entry:
-  %call4 = call i32 @bar()
-  %cmp5 = icmp eq i32 %call4, 0
-  br i1 %cmp5, label %if.then6, label %if.end8
-
-if.then6:
-  ret i32 0
-
-if.end8:
-  ret i32 1
-}




More information about the llvm-commits mailing list