From 7576d48d1f4a3612c401bfc6874834a483a2df4b Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 23 Jan 2017 08:33:24 +0000 Subject: [PATCH] [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 --- lib/Transforms/IPO/GlobalDCE.cpp | 27 +++++++++++++++++-- lib/Transforms/Scalar/JumpThreading.cpp | 4 --- test/Other/new-pm-defaults.ll | 4 --- .../crash-assertingvh.ll | 4 +++ 4 files changed, 29 insertions(+), 10 deletions(-) rename test/Transforms/{JumpThreading => GlobalDCE}/crash-assertingvh.ll (64%) diff --git a/lib/Transforms/IPO/GlobalDCE.cpp b/lib/Transforms/IPO/GlobalDCE.cpp index 7a04de3d12d..9304e783d49 100644 --- 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(M)) + FAM = &FAMProxy->getManager(); + // The second pass drops the bodies of functions which are dead... std::vector 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 1870c3deb4f..ecc6a0afbad 100644 --- 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(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 713cc022974..225b49afbf7 100644 --- 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 e7843199223..184d6ae7d8e 100644 --- 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" -- 2.40.0