From e9a44d27d28ff6d5f375ba8f6bde9fdddd36d96f Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Fri, 9 Nov 2018 10:23:46 +0000 Subject: [PATCH] [CallSiteSplitting] Only record conditions up to the IDom(call site). We can stop recording conditions once we reached the immediate dominator for the block containing the call site. Conditions in predecessors of the that node will be the same for all paths to the call site and splitting is not beneficial. This patch makes CallSiteSplitting dependent on the DT anlysis. because the immediate dominators seem to be the easiest way of finding the node to stop at. I had to update some exiting tests, because they were checking for conditions that were true/false on all paths to the call site. Those should now be handled by instcombine/ipsccp. Reviewers: davide, junbuml Reviewed By: junbuml Differential Revision: https://reviews.llvm.org/D44627 llvm-svn: 346483 --- lib/Transforms/Scalar/CallSiteSplitting.cpp | 53 +++++++++++++------ test/Other/new-pm-lto-defaults.ll | 4 +- test/Other/opt-O3-pipeline.ll | 1 + .../callsite-split-or-phi.ll | 41 +++++++++++--- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/lib/Transforms/Scalar/CallSiteSplitting.cpp b/lib/Transforms/Scalar/CallSiteSplitting.cpp index b9e8e3424cc..208dd5a5a83 100644 --- a/lib/Transforms/Scalar/CallSiteSplitting.cpp +++ b/lib/Transforms/Scalar/CallSiteSplitting.cpp @@ -149,14 +149,14 @@ static void recordCondition(CallSite CS, BasicBlock *From, BasicBlock *To, /// Record ICmp conditions relevant to any argument in CS following Pred's /// single predecessors. If there are conflicting conditions along a path, like -/// x == 1 and x == 0, the first condition will be used. +/// x == 1 and x == 0, the first condition will be used. We stop once we reach +/// an edge to StopAt. static void recordConditions(CallSite CS, BasicBlock *Pred, - ConditionsTy &Conditions) { - recordCondition(CS, Pred, CS.getInstruction()->getParent(), Conditions); + ConditionsTy &Conditions, BasicBlock *StopAt) { BasicBlock *From = Pred; BasicBlock *To = Pred; SmallPtrSet Visited; - while (!Visited.count(From->getSinglePredecessor()) && + while (To != StopAt && !Visited.count(From->getSinglePredecessor()) && (From = From->getSinglePredecessor())) { recordCondition(CS, From, To, Conditions); Visited.insert(From); @@ -302,7 +302,7 @@ static void copyMustTailReturn(BasicBlock *SplitBB, Instruction *CI, static void splitCallSite( CallSite CS, const SmallVectorImpl> &Preds, - DominatorTree *DT) { + DominatorTree &DT) { Instruction *Instr = CS.getInstruction(); BasicBlock *TailBB = Instr->getParent(); bool IsMustTailCall = CS.isMustTailCall(); @@ -327,7 +327,7 @@ static void splitCallSite( BasicBlock *PredBB = Preds[i].first; BasicBlock *SplitBlock = DuplicateInstructionsInSplitBetween( TailBB, PredBB, &*std::next(Instr->getIterator()), ValueToValueMaps[i], - DT); + &DT); assert(SplitBlock && "Unexpected new basic block split."); Instruction *NewCI = @@ -438,7 +438,7 @@ static bool isPredicatedOnPHI(CallSite CS) { return false; } -static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree *DT) { +static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree &DT) { if (!isPredicatedOnPHI(CS)) return false; @@ -449,15 +449,25 @@ static bool tryToSplitOnPHIPredicatedArgument(CallSite CS, DominatorTree *DT) { return true; } -static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree *DT) { +static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree &DT) { auto Preds = getTwoPredecessors(CS.getInstruction()->getParent()); if (Preds[0] == Preds[1]) return false; + // We can stop recording conditions once we reached the immediate dominator + // for the block containing the call site. Conditions in predecessors of the + // that node will be the same for all paths to the call site and splitting + // is not beneficial. + auto *CSDTNode = DT.getNode(CS.getInstruction()->getParent()); + BasicBlock *StopAt = CSDTNode ? CSDTNode->getIDom()->getBlock() : nullptr; + SmallVector, 2> PredsCS; for (auto *Pred : make_range(Preds.rbegin(), Preds.rend())) { ConditionsTy Conditions; - recordConditions(CS, Pred, Conditions); + // Record condition on edge BB(CS) <- Pred + recordCondition(CS, Pred, CS.getInstruction()->getParent(), Conditions); + // Record conditions followng Pred's single predecessors. + recordConditions(CS, Pred, Conditions, StopAt); PredsCS.push_back({Pred, Conditions}); } @@ -466,12 +476,24 @@ static bool tryToSplitOnPredicatedArgument(CallSite CS, DominatorTree *DT) { })) return false; + // Record common conditions starting from StopAt. Those conditions hold for + // all paths to CS. Adding them gives the inliner a better chance at inlining + // CS. + ConditionsTy CommonConditions; + if (StopAt) + recordConditions(CS, StopAt, CommonConditions, nullptr); + if (!CommonConditions.empty()) + for (auto &Pred : PredsCS) { + Pred.second.insert(Pred.second.end(), CommonConditions.begin(), + CommonConditions.end()); + } + splitCallSite(CS, PredsCS, DT); return true; } static bool tryToSplitCallSite(CallSite CS, TargetTransformInfo &TTI, - DominatorTree *DT) { + DominatorTree &DT) { if (!CS.arg_size() || !canSplitCallSite(CS, TTI)) return false; return tryToSplitOnPredicatedArgument(CS, DT) || @@ -479,7 +501,7 @@ static bool tryToSplitCallSite(CallSite CS, TargetTransformInfo &TTI, } static bool doCallSiteSplitting(Function &F, TargetLibraryInfo &TLI, - TargetTransformInfo &TTI, DominatorTree *DT) { + TargetTransformInfo &TTI, DominatorTree &DT) { bool Changed = false; for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE;) { BasicBlock &BB = *BI++; @@ -524,6 +546,7 @@ struct CallSiteSplittingLegacyPass : public FunctionPass { void getAnalysisUsage(AnalysisUsage &AU) const override { AU.addRequired(); AU.addRequired(); + AU.addRequired(); AU.addPreserved(); FunctionPass::getAnalysisUsage(AU); } @@ -534,9 +557,8 @@ struct CallSiteSplittingLegacyPass : public FunctionPass { auto &TLI = getAnalysis().getTLI(); auto &TTI = getAnalysis().getTTI(F); - auto *DTWP = getAnalysisIfAvailable(); - return doCallSiteSplitting(F, TLI, TTI, - DTWP ? &DTWP->getDomTree() : nullptr); + auto &DT = getAnalysis().getDomTree(); + return doCallSiteSplitting(F, TLI, TTI, DT); } }; } // namespace @@ -546,6 +568,7 @@ INITIALIZE_PASS_BEGIN(CallSiteSplittingLegacyPass, "callsite-splitting", "Call-site splitting", false, false) INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass) INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass) +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) INITIALIZE_PASS_END(CallSiteSplittingLegacyPass, "callsite-splitting", "Call-site splitting", false, false) FunctionPass *llvm::createCallSiteSplittingPass() { @@ -556,7 +579,7 @@ PreservedAnalyses CallSiteSplittingPass::run(Function &F, FunctionAnalysisManager &AM) { auto &TLI = AM.getResult(F); auto &TTI = AM.getResult(F); - auto *DT = AM.getCachedResult(F); + auto &DT = AM.getResult(F); if (!doCallSiteSplitting(F, TLI, TTI, DT)) return PreservedAnalyses::all(); diff --git a/test/Other/new-pm-lto-defaults.ll b/test/Other/new-pm-lto-defaults.ll index 03f4804a2ba..ee2f0414e62 100644 --- a/test/Other/new-pm-lto-defaults.ll +++ b/test/Other/new-pm-lto-defaults.ll @@ -38,13 +38,13 @@ ; CHECK-O2-NEXT: Running pass: CallSiteSplittingPass on foo ; CHECK-O2-NEXT: Running analysis: TargetLibraryAnalysis on foo ; CHECK-O2-NEXT: Running analysis: TargetIRAnalysis on foo +; CHECK-O2-NEXT: Running analysis: DominatorTreeAnalysis on foo ; CHECK-O2-NEXT: Finished llvm::Function pass manager run. ; CHECK-O2-NEXT: PGOIndirectCallPromotion ; CHECK-O2-NEXT: Running analysis: ProfileSummaryAnalysis ; CHECK-O2-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis ; CHECK-O2-NEXT: Running pass: IPSCCPPass -; CHECK-O2-DAG: Running analysis: AssumptionAnalysis on foo -; CHECK-O2-DAG: Running analysis: DominatorTreeAnalysis on foo +; CHECK-O2-NEXT: Running analysis: AssumptionAnalysis on foo ; CHECK-O2-NEXT: Running pass: CalledValuePropagationPass ; CHECK-O-NEXT: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}PostOrderFunctionAttrsPass> ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}SCC diff --git a/test/Other/opt-O3-pipeline.ll b/test/Other/opt-O3-pipeline.ll index 33033fef183..51b6a6d4386 100644 --- a/test/Other/opt-O3-pipeline.ll +++ b/test/Other/opt-O3-pipeline.ll @@ -28,6 +28,7 @@ ; CHECK-NEXT: Force set function attributes ; CHECK-NEXT: Infer set function attributes ; CHECK-NEXT: FunctionPass Manager +; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: Call-site splitting ; CHECK-NEXT: Interprocedural Sparse Conditional Constant Propagation ; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() diff --git a/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll b/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll index d5f31f9ac91..2a400c93ae2 100644 --- a/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll +++ b/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll @@ -123,14 +123,14 @@ End: ;CHECK-LABEL: Header2.split: ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 10) ;CHECK-LABEL: TBB.split: -;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p) +;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p) ;CHECK-LABEL: Tail ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ] ;CHECK: ret i32 %[[MERGED]] define i32 @test_ne_eq_ne(i32* %a, i32 %v, i32 %p) { Header: %tobool1 = icmp ne i32* %a, null - br i1 %tobool1, label %Header2, label %End + br i1 %tobool1, label %Header2, label %TBB Header2: %tobool2 = icmp eq i32 %p, 10 @@ -178,14 +178,14 @@ End: ;CHECK-LABEL: Header2.split: ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p) ;CHECK-LABEL: TBB.split: -;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 %p) +;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p) ;CHECK-LABEL: Tail ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ] ;CHECK: ret i32 %[[MERGED]] define i32 @test_ne_ne_ne_constrain_same_pointer_arg(i32* %a, i32 %v, i32 %p, i32* %a2, i32* %a3) { Header: %tobool1 = icmp ne i32* %a, null - br i1 %tobool1, label %Header2, label %End + br i1 %tobool1, label %Header2, label %TBB Header2: %tobool2 = icmp ne i32* %a, %a2 @@ -235,14 +235,14 @@ End: ;CHECK-LABEL: Header2.split: ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* nonnull %a, i32 %v, i32 10) ;CHECK-LABEL: TBB.split: -;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* nonnull %a, i32 1, i32 %p) +;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 1, i32 %p) ;CHECK-LABEL: Tail ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ] ;CHECK: ret i32 %[[MERGED]] define i32 @test_eq_eq_eq_untaken(i32* %a, i32 %v, i32 %p) { Header: %tobool1 = icmp eq i32* %a, null - br i1 %tobool1, label %End, label %Header2 + br i1 %tobool1, label %TBB, label %Header2 Header2: %tobool2 = icmp eq i32 %p, 10 @@ -290,14 +290,14 @@ End: ;CHECK-LABEL: Header2.split: ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* null, i32 %v, i32 10) ;CHECK-LABEL: TBB.split: -;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* null, i32 %v, i32 %p) +;CHECK: %[[CALL2:.*]] = call i32 @callee(i32* %a, i32 %v, i32 %p) ;CHECK-LABEL: Tail ;CHECK: %[[MERGED:.*]] = phi i32 [ %[[CALL1]], %Header2.split ], [ %[[CALL2]], %TBB.split ] ;CHECK: ret i32 %[[MERGED]] define i32 @test_ne_eq_ne_untaken(i32* %a, i32 %v, i32 %p) { Header: %tobool1 = icmp ne i32* %a, null - br i1 %tobool1, label %End, label %Header2 + br i1 %tobool1, label %TBB, label %Header2 Header2: %tobool2 = icmp eq i32 %p, 10 @@ -489,6 +489,31 @@ End: ret i32 %v } +;CHECK-LABEL: @test_cond_no_effect +;CHECK-NOT: Header.split: +;CHECK-NOT: TBB.split: +;CHECK-LABEL: Tail: +;CHECK: %r = call i32 @callee(i32* %a, i32 %v, i32 0) +;CHECK: ret i32 %r +define i32 @test_cond_no_effect(i32* %a, i32 %v) { +Entry: + %tobool1 = icmp eq i32* %a, null + br i1 %tobool1, label %Header, label %End + +Header: + br i1 undef, label %Tail, label %TBB + +TBB: + br i1 undef, label %Tail, label %End + +Tail: + %r = call i32 @callee(i32* %a, i32 %v, i32 0) + ret i32 %r + +End: + ret i32 %v +} + ;CHECK-LABEL: @test_unreachable ;CHECK-LABEL: Header.split: ;CHECK: %[[CALL1:.*]] = call i32 @callee(i32* %a, i32 %v, i32 10)