<div dir="ltr"><div>A better title for this commit:<br>[CGP] transform select instructions into branches and sink expensive operands<br><br></div>I missed the headline when copying the text from the phab review.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 16, 2015 at 10:54 AM, Sanjay Patel via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: spatel<br>
Date: Fri Oct 16 11:54:30 2015<br>
New Revision: 250527<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250527&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=250527&view=rev</a><br>
Log:<br>
This is a follow-up to the discussion in D12882.<br>
<br>
Ideally, we would like SimplifyCFG to be able to form select instructions even when the operands<br>
are expensive (as defined by the TTI cost model) because that may expose further optimizations.<br>
However, we would then like a later pass like CodeGenPrepare to undo that transformation if the<br>
target would likely benefit from not speculatively executing an expensive op (this patch).<br>
<br>
Once we have this safety mechanism in place, we can adjust SimplifyCFG to restore its<br>
select-formation behavior that changed with r248439.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D13297" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13297</a><br>
<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/CodeGenPrepare/select.ll<br>
Modified:<br>
llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=250527&r1=250526&r2=250527&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=250527&r1=250526&r2=250527&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Fri Oct 16 11:54:30 2015<br>
@@ -3846,8 +3846,20 @@ bool CodeGenPrepare::optimizeExtUses(Ins<br>
return MadeChange;<br>
}<br>
<br>
+/// Check if V (an operand of a select instruction) is an expensive instruction<br>
+/// that is only used once.<br>
+static bool sinkSelectOperand(const TargetTransformInfo *TTI, Value *V) {<br>
+ auto *I = dyn_cast<Instruction>(V);<br>
+ if (I && I->hasOneUse() &&<br>
+ TTI->getUserCost(I) == TargetTransformInfo::TCC_Expensive)<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
/// Returns true if a SelectInst should be turned into an explicit branch.<br>
-static bool isFormingBranchFromSelectProfitable(SelectInst *SI) {<br>
+static bool isFormingBranchFromSelectProfitable(const TargetTransformInfo *TTI,<br>
+ SelectInst *SI) {<br>
// FIXME: This should use the same heuristics as IfConversion to determine<br>
// whether a select is better represented as a branch. This requires that<br>
// branch probability metadata is preserved for the select, which is not the<br>
@@ -3868,8 +3880,17 @@ static bool isFormingBranchFromSelectPro<br>
// on a load from memory. But if the load is used more than once, do not<br>
// change the select to a branch because the load is probably needed<br>
// regardless of whether the branch is taken or not.<br>
- return ((isa<LoadInst>(CmpOp0) && CmpOp0->hasOneUse()) ||<br>
- (isa<LoadInst>(CmpOp1) && CmpOp1->hasOneUse()));<br>
+ if ((isa<LoadInst>(CmpOp0) && CmpOp0->hasOneUse()) ||<br>
+ (isa<LoadInst>(CmpOp1) && CmpOp1->hasOneUse()))<br>
+ return true;<br>
+<br>
+ // If either operand of the select is expensive and only needed on one side<br>
+ // of the select, we should form a branch.<br>
+ if (sinkSelectOperand(TTI, SI->getTrueValue()) ||<br>
+ sinkSelectOperand(TTI, SI->getFalseValue()))<br>
+ return true;<br>
+<br>
+ return false;<br>
}<br>
<br>
<br>
@@ -3895,34 +3916,97 @@ bool CodeGenPrepare::optimizeSelectInst(<br>
// We have efficient codegen support for the select instruction.<br>
// Check if it is profitable to keep this 'select'.<br>
if (!TLI->isPredictableSelectExpensive() ||<br>
- !isFormingBranchFromSelectProfitable(SI))<br>
+ !isFormingBranchFromSelectProfitable(TTI, SI))<br>
return false;<br>
}<br>
<br>
ModifiedDT = true;<br>
<br>
+ // Transform a sequence like this:<br>
+ // start:<br>
+ // %cmp = cmp uge i32 %a, %b<br>
+ // %sel = select i1 %cmp, i32 %c, i32 %d<br>
+ //<br>
+ // Into:<br>
+ // start:<br>
+ // %cmp = cmp uge i32 %a, %b<br>
+ // br i1 %cmp, label %select.true, label %select.false<br>
+ // select.true:<br>
+ // br label %select.end<br>
+ // select.false:<br>
+ // br label %select.end<br>
+ // select.end:<br>
+ // %sel = phi i32 [ %c, %select.true ], [ %d, %select.false ]<br>
+ //<br>
+ // In addition, we may sink instructions that produce %c or %d from<br>
+ // the entry block into the destination(s) of the new branch.<br>
+ // If the true or false blocks do not contain a sunken instruction, that<br>
+ // block and its branch may be optimized away. In that case, one side of the<br>
+ // first branch will point directly to select.end, and the corresponding PHI<br>
+ // predecessor block will be the start block.<br>
+<br>
// First, we split the block containing the select into 2 blocks.<br>
BasicBlock *StartBlock = SI->getParent();<br>
BasicBlock::iterator SplitPt = ++(BasicBlock::iterator(SI));<br>
- BasicBlock *NextBlock = StartBlock->splitBasicBlock(SplitPt, "select.end");<br>
-<br>
- // Create a new block serving as the landing pad for the branch.<br>
- BasicBlock *SmallBlock = BasicBlock::Create(SI->getContext(), "select.mid",<br>
- NextBlock->getParent(), NextBlock);<br>
+ BasicBlock *EndBlock = StartBlock->splitBasicBlock(SplitPt, "select.end");<br>
<br>
- // Move the unconditional branch from the block with the select in it into our<br>
- // landing pad block.<br>
+ // Delete the unconditional branch that was just created by the split.<br>
StartBlock->getTerminator()->eraseFromParent();<br>
- BranchInst::Create(NextBlock, SmallBlock);<br>
+<br>
+ // These are the new basic blocks for the conditional branch.<br>
+ // At least one will become an actual new basic block.<br>
+ BasicBlock *TrueBlock = nullptr;<br>
+ BasicBlock *FalseBlock = nullptr;<br>
+<br>
+ // Sink expensive instructions into the conditional blocks to avoid executing<br>
+ // them speculatively.<br>
+ if (sinkSelectOperand(TTI, SI->getTrueValue())) {<br>
+ TrueBlock = BasicBlock::Create(SI->getContext(), "select.true.sink",<br>
+ EndBlock->getParent(), EndBlock);<br>
+ auto *TrueBranch = BranchInst::Create(EndBlock, TrueBlock);<br>
+ auto *TrueInst = cast<Instruction>(SI->getTrueValue());<br>
+ TrueInst->moveBefore(TrueBranch);<br>
+ }<br>
+ if (sinkSelectOperand(TTI, SI->getFalseValue())) {<br>
+ FalseBlock = BasicBlock::Create(SI->getContext(), "select.false.sink",<br>
+ EndBlock->getParent(), EndBlock);<br>
+ auto *FalseBranch = BranchInst::Create(EndBlock, FalseBlock);<br>
+ auto *FalseInst = cast<Instruction>(SI->getFalseValue());<br>
+ FalseInst->moveBefore(FalseBranch);<br>
+ }<br>
+<br>
+ // If there was nothing to sink, then arbitrarily choose the 'false' side<br>
+ // for a new input value to the PHI.<br>
+ if (TrueBlock == FalseBlock) {<br>
+ assert(TrueBlock == nullptr &&<br>
+ "Unexpected basic block transform while optimizing select");<br>
+<br>
+ FalseBlock = BasicBlock::Create(SI->getContext(), "select.false",<br>
+ EndBlock->getParent(), EndBlock);<br>
+ BranchInst::Create(EndBlock, FalseBlock);<br>
+ }<br>
<br>
// Insert the real conditional branch based on the original condition.<br>
- BranchInst::Create(NextBlock, SmallBlock, SI->getCondition(), SI);<br>
+ // If we did not create a new block for one of the 'true' or 'false' paths<br>
+ // of the condition, it means that side of the branch goes to the end block<br>
+ // directly and the path originates from the start block from the point of<br>
+ // view of the new PHI.<br>
+ if (TrueBlock == nullptr) {<br>
+ BranchInst::Create(EndBlock, FalseBlock, SI->getCondition(), SI);<br>
+ TrueBlock = StartBlock;<br>
+ } else if (FalseBlock == nullptr) {<br>
+ BranchInst::Create(TrueBlock, EndBlock, SI->getCondition(), SI);<br>
+ FalseBlock = StartBlock;<br>
+ } else {<br>
+ BranchInst::Create(TrueBlock, FalseBlock, SI->getCondition(), SI);<br>
+ }<br>
<br>
// The select itself is replaced with a PHI Node.<br>
- PHINode *PN = PHINode::Create(SI->getType(), 2, "", &NextBlock->front());<br>
+ PHINode *PN = PHINode::Create(SI->getType(), 2, "", &EndBlock->front());<br>
PN->takeName(SI);<br>
- PN->addIncoming(SI->getTrueValue(), StartBlock);<br>
- PN->addIncoming(SI->getFalseValue(), SmallBlock);<br>
+ PN->addIncoming(SI->getTrueValue(), TrueBlock);<br>
+ PN->addIncoming(SI->getFalseValue(), FalseBlock);<br>
+<br>
SI->replaceAllUsesWith(PN);<br>
SI->eraseFromParent();<br>
<br>
<br>
Added: llvm/trunk/test/Transforms/CodeGenPrepare/select.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/select.ll?rev=250527&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/select.ll?rev=250527&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/CodeGenPrepare/select.ll (added)<br>
+++ llvm/trunk/test/Transforms/CodeGenPrepare/select.ll Fri Oct 16 11:54:30 2015<br>
@@ -0,0 +1,114 @@<br>
+; RUN: opt -codegenprepare -S < %s | FileCheck %s<br>
+<br>
+target triple = "x86_64-unknown-unknown"<br>
+<br>
+; Nothing to sink here, but this gets converted to a branch to<br>
+; avoid stalling an out-of-order CPU on a predictable branch.<br>
+<br>
+define i32 @no_sink(double %a, double* %b, i32 %x, i32 %y) {<br>
+entry:<br>
+ %load = load double, double* %b, align 8<br>
+ %cmp = fcmp olt double %load, %a<br>
+ %sel = select i1 %cmp, i32 %x, i32 %y<br>
+ ret i32 %sel<br>
+<br>
+; CHECK-LABEL: @no_sink(<br>
+; CHECK: %load = load double, double* %b, align 8<br>
+; CHECK: %cmp = fcmp olt double %load, %a<br>
+; CHECK: br i1 %cmp, label %select.end, label %select.false<br>
+; CHECK: select.false:<br>
+; CHECK: br label %select.end<br>
+; CHECK: select.end:<br>
+; CHECK: %sel = phi i32 [ %x, %entry ], [ %y, %select.false ]<br>
+; CHECK: ret i32 %sel<br>
+}<br>
+<br>
+<br>
+; An 'fdiv' is expensive, so sink it rather than speculatively execute it.<br>
+<br>
+define float @fdiv_true_sink(float %a, float %b) {<br>
+entry:<br>
+ %div = fdiv float %a, %b<br>
+ %cmp = fcmp ogt float %a, 1.0<br>
+ %sel = select i1 %cmp, float %div, float 2.0<br>
+ ret float %sel<br>
+<br>
+; CHECK-LABEL: @fdiv_true_sink(<br>
+; CHECK: %cmp = fcmp ogt float %a, 1.0<br>
+; CHECK: br i1 %cmp, label %select.true.sink, label %select.end<br>
+; CHECK: select.true.sink:<br>
+; CHECK: %div = fdiv float %a, %b<br>
+; CHECK: br label %select.end<br>
+; CHECK: select.end:<br>
+; CHECK: %sel = phi float [ %div, %select.true.sink ], [ 2.000000e+00, %entry ]<br>
+; CHECK: ret float %sel<br>
+}<br>
+<br>
+define float @fdiv_false_sink(float %a, float %b) {<br>
+entry:<br>
+ %div = fdiv float %a, %b<br>
+ %cmp = fcmp ogt float %a, 3.0<br>
+ %sel = select i1 %cmp, float 4.0, float %div<br>
+ ret float %sel<br>
+<br>
+; CHECK-LABEL: @fdiv_false_sink(<br>
+; CHECK: %cmp = fcmp ogt float %a, 3.0<br>
+; CHECK: br i1 %cmp, label %select.end, label %select.false.sink<br>
+; CHECK: select.false.sink:<br>
+; CHECK: %div = fdiv float %a, %b<br>
+; CHECK: br label %select.end<br>
+; CHECK: select.end:<br>
+; CHECK: %sel = phi float [ 4.000000e+00, %entry ], [ %div, %select.false.sink ]<br>
+; CHECK: ret float %sel<br>
+}<br>
+<br>
+define float @fdiv_both_sink(float %a, float %b) {<br>
+entry:<br>
+ %div1 = fdiv float %a, %b<br>
+ %div2 = fdiv float %b, %a<br>
+ %cmp = fcmp ogt float %a, 5.0<br>
+ %sel = select i1 %cmp, float %div1, float %div2<br>
+ ret float %sel<br>
+<br>
+; CHECK-LABEL: @fdiv_both_sink(<br>
+; CHECK: %cmp = fcmp ogt float %a, 5.0<br>
+; CHECK: br i1 %cmp, label %select.true.sink, label %select.false.sink<br>
+; CHECK: select.true.sink:<br>
+; CHECK: %div1 = fdiv float %a, %b<br>
+; CHECK: br label %select.end<br>
+; CHECK: select.false.sink:<br>
+; CHECK: %div2 = fdiv float %b, %a<br>
+; CHECK: br label %select.end<br>
+; CHECK: select.end:<br>
+; CHECK: %sel = phi float [ %div1, %select.true.sink ], [ %div2, %select.false.sink ]<br>
+; CHECK: ret float %sel<br>
+}<br>
+<br>
+; An 'fadd' is not too expensive, so it's ok to speculate.<br>
+<br>
+define float @fadd_no_sink(float %a, float %b) {<br>
+ %add = fadd float %a, %b<br>
+ %cmp = fcmp ogt float 6.0, %a<br>
+ %sel = select i1 %cmp, float %add, float 7.0<br>
+ ret float %sel<br>
+<br>
+; CHECK-LABEL: @fadd_no_sink(<br>
+; CHECK: %sel = select i1 %cmp, float %add, float 7.0<br>
+}<br>
+<br>
+; Possible enhancement: sinkability is only calculated with the direct<br>
+; operand of the select, so we don't try to sink this. The fdiv cost is not<br>
+; taken into account.<br>
+<br>
+define float @fdiv_no_sink(float %a, float %b) {<br>
+entry:<br>
+ %div = fdiv float %a, %b<br>
+ %add = fadd float %div, %b<br>
+ %cmp = fcmp ogt float %a, 1.0<br>
+ %sel = select i1 %cmp, float %add, float 8.0<br>
+ ret float %sel<br>
+<br>
+; CHECK-LABEL: @fdiv_no_sink(<br>
+; CHECK: %sel = select i1 %cmp, float %add, float 8.0<br>
+}<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>