Skip to content

Commit

Permalink
[CallSiteSplitting] Only record conditions up to the IDom(call site).
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@fhahn
fhahn committed Nov 9, 2018
1 parent e6b727e commit 52578f9
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 25 deletions.
53 changes: 38 additions & 15 deletions llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
View file
Expand Up @@ -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<BasicBlock *, 4> Visited;
while (!Visited.count(From->getSinglePredecessor()) &&
while (To != StopAt && !Visited.count(From->getSinglePredecessor()) &&
(From = From->getSinglePredecessor())) {
recordCondition(CS, From, To, Conditions);
Visited.insert(From);
Expand Down Expand Up @@ -302,7 +302,7 @@ static void copyMustTailReturn(BasicBlock *SplitBB, Instruction *CI,
static void splitCallSite(
CallSite CS,
const SmallVectorImpl<std::pair<BasicBlock *, ConditionsTy>> &Preds,
DominatorTree *DT) {
DominatorTree &DT) {
Instruction *Instr = CS.getInstruction();
BasicBlock *TailBB = Instr->getParent();
bool IsMustTailCall = CS.isMustTailCall();
Expand All @@ -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 =
Expand Down Expand Up @@ -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;

Expand All @@ -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<std::pair<BasicBlock *, ConditionsTy>, 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});
}

Expand All @@ -466,20 +476,32 @@ 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) ||
tryToSplitOnPHIPredicatedArgument(CS, DT);
}

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++;
Expand Down Expand Up @@ -524,6 +546,7 @@ struct CallSiteSplittingLegacyPass : public FunctionPass {
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<TargetLibraryInfoWrapperPass>();
AU.addRequired<TargetTransformInfoWrapperPass>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
FunctionPass::getAnalysisUsage(AU);
}
Expand All @@ -534,9 +557,8 @@ struct CallSiteSplittingLegacyPass : public FunctionPass {

auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();
return doCallSiteSplitting(F, TLI, TTI,
DTWP ? &DTWP->getDomTree() : nullptr);
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
return doCallSiteSplitting(F, TLI, TTI, DT);
}
};
} // namespace
Expand All @@ -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() {
Expand All @@ -556,7 +579,7 @@ PreservedAnalyses CallSiteSplittingPass::run(Function &F,
FunctionAnalysisManager &AM) {
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &TTI = AM.getResult<TargetIRAnalysis>(F);
auto *DT = AM.getCachedResult<DominatorTreeAnalysis>(F);
auto &DT = AM.getResult<DominatorTreeAnalysis>(F);

if (!doCallSiteSplitting(F, TLI, TTI, DT))
return PreservedAnalyses::all();
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Other/new-pm-lto-defaults.ll
View file
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions llvm/test/Other/opt-O3-pipeline.ll
View file
Expand Up @@ -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()
Expand Down
41 changes: 33 additions & 8 deletions llvm/test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 52578f9

Please sign in to comment.