diff --git a/lib/Transforms/IPO/Inliner.cpp b/lib/Transforms/IPO/Inliner.cpp index ded3ca22927..dfc1a6ba738 100644 --- a/lib/Transforms/IPO/Inliner.cpp +++ b/lib/Transforms/IPO/Inliner.cpp @@ -751,9 +751,6 @@ bool LegacyInlinerBase::removeDeadFunctions(CallGraph &CG, PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, CGSCCAnalysisManager &AM, LazyCallGraph &CG, CGSCCUpdateResult &UR) { - FunctionAnalysisManager &FAM = - AM.getResult(InitialC, CG) - .getManager(); const ModuleAnalysisManager &MAM = AM.getResult(InitialC, CG).getManager(); bool Changed = false; @@ -762,21 +759,6 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, Module &M = *InitialC.begin()->getFunction().getParent(); ProfileSummaryInfo *PSI = MAM.getCachedResult(M); - std::function GetAssumptionCache = - [&](Function &F) -> AssumptionCache & { - return FAM.getResult(F); - }; - auto GetBFI = [&](Function &F) -> BlockFrequencyInfo & { - return FAM.getResult(F); - }; - - auto GetInlineCost = [&](CallSite CS) { - Function &Callee = *CS.getCalledFunction(); - auto &CalleeTTI = FAM.getResult(Callee); - return getInlineCost(CS, Params, CalleeTTI, GetAssumptionCache, {GetBFI}, - PSI); - }; - // We use a worklist of nodes to process so that we can handle if the SCC // structure changes and some nodes are no longer part of the current SCC. We // also need to use an updatable pointer for the SCC as a consequence. @@ -816,6 +798,31 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, DEBUG(dbgs() << "Inlining calls in: " << F.getName() << "\n"); + // Get a FunctionAnalysisManager via a proxy for this particular node. We + // do this each time we visit a node as the SCC may have changed and as + // we're going to mutate this particular function we want to make sure the + // proxy is in place to forward any invalidation events. We can use the + // manager we get here for looking up results for functions other than this + // node however because those functions aren't going to be mutated by this + // pass. + FunctionAnalysisManager &FAM = + AM.getResult(*C, CG) + .getManager(); + std::function GetAssumptionCache = + [&](Function &F) -> AssumptionCache & { + return FAM.getResult(F); + }; + auto GetBFI = [&](Function &F) -> BlockFrequencyInfo & { + return FAM.getResult(F); + }; + + auto GetInlineCost = [&](CallSite CS) { + Function &Callee = *CS.getCalledFunction(); + auto &CalleeTTI = FAM.getResult(Callee); + return getInlineCost(CS, Params, CalleeTTI, GetAssumptionCache, {GetBFI}, + PSI); + }; + // Get the remarks emission analysis for the caller. auto &ORE = FAM.getResult(F); diff --git a/test/Other/new-pm-defaults.ll b/test/Other/new-pm-defaults.ll index 81aac0b8856..0cbc18d01ab 100644 --- a/test/Other/new-pm-defaults.ll +++ b/test/Other/new-pm-defaults.ll @@ -62,8 +62,8 @@ ; CHECK-O-NEXT: Running analysis: LazyCallGraphAnalysis ; CHECK-O-NEXT: Starting CGSCC pass manager run. ; CHECK-O-NEXT: Running pass: InlinerPass -; CHECK-O-NEXT: Running analysis: FunctionAnalysisManagerCGSCCProxy ; CHECK-O-NEXT: Running analysis: OuterAnalysisManagerProxy<{{.*}}LazyCallGraph{{.*}}> +; CHECK-O-NEXT: Running analysis: FunctionAnalysisManagerCGSCCProxy ; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis ; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass ; CHECK-O-NEXT: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> diff --git a/test/Transforms/Inline/cgscc-incremental-invalidate.ll b/test/Transforms/Inline/cgscc-incremental-invalidate.ll new file mode 100644 index 00000000000..7c6d0fc04ce --- /dev/null +++ b/test/Transforms/Inline/cgscc-incremental-invalidate.ll @@ -0,0 +1,111 @@ +; Test for a subtle bug when computing analyses during inlining and mutating +; the SCC structure. Without care, this can fail to invalidate analyses. +; +; RUN: opt < %s -passes='cgscc(inline,function(verify))' -debug-pass-manager -S 2>&1 | FileCheck %s + +; First we check that the passes run in the way we expect. Otherwise this test +; may stop testing anything. +; +; CHECK-LABEL: Starting llvm::Module pass manager run. +; CHECK: Running pass: InlinerPass on (test1_h, test1_g, test1_f) +; CHECK: Running analysis: FunctionAnalysisManagerCGSCCProxy on (test1_h, test1_g, test1_f) +; CHECK: Running analysis: DominatorTreeAnalysis on test1_f +; CHECK: Running analysis: DominatorTreeAnalysis on test1_g +; CHECK: Invalidating all non-preserved analyses for: (test1_h, test1_g, test1_f) +; CHECK: Invalidating all non-preserved analyses for: test1_h +; CHECK-NOT: Invalidating anaylsis: +; CHECK: Invalidating all non-preserved analyses for: test1_g +; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g +; CHECK: Invalidating all non-preserved analyses for: test1_f +; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f +; CHECK: Running analysis: DominatorTreeAnalysis on test1_h +; CHECK: Invalidating all non-preserved analyses for: (test1_h, test1_g) +; CHECK: Invalidating all non-preserved analyses for: test1_h +; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_h + +; An external function used to control branches. +declare i1 @flag() +; CHECK-LABEL: declare i1 @flag() + +; The utility function with interesting control flow that gets inlined below to +; perturb the dominator tree. +define internal void @callee() { +entry: + %ptr = alloca i8 + %flag = call i1 @flag() + br i1 %flag, label %then, label %else + +then: + store volatile i8 42, i8* %ptr + br label %return + +else: + store volatile i8 -42, i8* %ptr + br label %return + +return: + ret void +} + +; The 'test1_' prefixed functions work to carefully test that incrementally +; reducing an SCC in the inliner cannot accidentially leave stale function +; analysis results due to failing to invalidate them for all the functions. + +; We visit this function first in the inliner, and while we inline callee +; perturbing the CFG, we don't inline anything else and the SCC structure +; remains in tact. +define void @test1_f() { +; CHECK-LABEL: define void @test1_f() +entry: + ; We force this edge to survive inlining. + call void @test1_g() noinline +; CHECK: call void @test1_g() + + ; Pull interesting CFG into this function. + call void @callee() +; CHECK-NOT: call void @callee() + + ret void +; CHECK: ret void +} + +; Next we visit this function and here we inline the edge to 'test1_f' +; separating it into its own SCC. The current SCC is now just 'test1_g' and +; 'test1_h'. +define void @test1_g() { +; CHECK-LABEL: define void @test1_g() +entry: + ; This edge gets inlined away. + call void @test1_f() +; CHECK-NOT: call void @test1_f() +; CHECK: call void @test1_g() + + ; We force this edge to survive inlining. + call void @test1_h() noinline +; CHECK: call void @test1_h() + + ; Pull interesting CFG into this function. + call void @callee() +; CHECK-NOT: call void @callee() + + ret void +; CHECK: ret void +} + +; Finally the inliner visits this last function. It can't actually break any +; cycles here, but because we visit this function we compute fresh analyses for +; it. These analyses are then invalidated when we inline callee disrupting the +; CFG, and it is important that they be freed. +define void @test1_h() { +; CHECK-LABEL: define void @test1_h() +entry: + call void @test1_g() +; CHECK: call void @test1_g() + + ; Pull interesting CFG into this function. + call void @callee() +; CHECK-NOT: call void @callee() + + ret void +; CHECK: ret void +}