From 329b590e6e2be8f181fb09231d6c871564a725cf Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Sun, 27 Jan 2013 06:42:03 +0000 Subject: [PATCH] Re-revert r173342, without losing the compile time improvements, flat out bug fixes, or functionality preserving refactorings. llvm-svn: 173610 --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 39 +++---- .../test/Transforms/SimplifyCFG/SpeculativeExec.ll | 115 --------------------- 2 files changed, 12 insertions(+), 142 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 3cf3984..a63d31d 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -1369,8 +1369,7 @@ static bool SinkThenElseCodeToEnd(BranchInst *BI1) { /// \endcode /// /// \returns true if the conditional block is removed. -static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB, - const TargetTransformInfo &TTI) { +static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB) { // Be conservative for now. FP select instruction can often be expensive. Value *BrCond = BI->getCondition(); if (isa(BrCond)) @@ -1406,24 +1405,15 @@ static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB, // Only speculatively execution a single instruction (not counting the // terminator) for now. - SpeculationCost += TTI.getUserCost(I); - if (SpeculationCost > TargetTransformInfo::TCC_Basic) + ++SpeculationCost; + if (SpeculationCost > 1) return false; // Don't hoist the instruction if it's unsafe or expensive. if (!isSafeToSpeculativelyExecute(I)) return false; - // FIXME: These should really be cost metrics, but our cost model doesn't - // accurately model the expense of selects and floating point operations. - // FIXME: Is it really safe to speculate floating point operations? - // Signaling NaNs break with this, but we shouldn't care, right? - if (isa(I) || I->getType()->isFPOrFPVectorTy()) + if (ComputeSpeculationCost(I) > PHINodeFoldingThreshold) return false; - // FIXME: The cost metric currently doesn't reason accurately about simple - // versus complex GEPs, take a conservative approach here. - if (GEPOperator *GEP = dyn_cast(I)) - if (!GEP->hasAllConstantIndices()) - return false; // Do not hoist the instruction if any of its operands are defined but not // used in this BB. The transformation will prevent the operand from @@ -1446,8 +1436,8 @@ static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB, SinkCandidateUseCounts.begin(), E = SinkCandidateUseCounts.end(); I != E; ++I) if (I->first->getNumUses() == I->second) { - SpeculationCost += TTI.getUserCost(I->first); - if (SpeculationCost > TargetTransformInfo::TCC_Basic) + ++SpeculationCost; + if (SpeculationCost > 1) return false; } @@ -1469,20 +1459,15 @@ static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB, if (!isSafeToSpeculativelyExecute(CE)) return false; - - // Don't speculate into a select with a constant select expression operand. - // FIXME: This should really be a cost metric, but our cost model doesn't - // accurately model the expense of select. - if (Operator::getOpcode(CE) == Instruction::Select) + if (ComputeSpeculationCost(CE) > PHINodeFoldingThreshold) return false; // Account for the cost of an unfolded ConstantExpr which could end up // getting expanded into Instructions. // FIXME: This doesn't account for how many operations are combined in the - // constant expression. The cost functions in TTI don't yet correctly model - // constant expression costs. - SpeculationCost += TargetTransformInfo::TCC_Basic; - if (SpeculationCost > TargetTransformInfo::TCC_Basic) + // constant expression. + ++SpeculationCost; + if (SpeculationCost > 1) return false; } @@ -3899,7 +3884,7 @@ bool SimplifyCFGOpt::SimplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) { TerminatorInst *Succ0TI = BI->getSuccessor(0)->getTerminator(); if (Succ0TI->getNumSuccessors() == 1 && Succ0TI->getSuccessor(0) == BI->getSuccessor(1)) - if (SpeculativelyExecuteBB(BI, BI->getSuccessor(0), TTI)) + if (SpeculativelyExecuteBB(BI, BI->getSuccessor(0))) return SimplifyCFG(BB, TTI, TD) | true; } } else if (BI->getSuccessor(1)->getSinglePredecessor() != 0) { @@ -3908,7 +3893,7 @@ bool SimplifyCFGOpt::SimplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) { TerminatorInst *Succ1TI = BI->getSuccessor(1)->getTerminator(); if (Succ1TI->getNumSuccessors() == 1 && Succ1TI->getSuccessor(0) == BI->getSuccessor(0)) - if (SpeculativelyExecuteBB(BI, BI->getSuccessor(1), TTI)) + if (SpeculativelyExecuteBB(BI, BI->getSuccessor(1))) return SimplifyCFG(BB, TTI, TD) | true; } diff --git a/llvm/test/Transforms/SimplifyCFG/SpeculativeExec.ll b/llvm/test/Transforms/SimplifyCFG/SpeculativeExec.ll index b60e5dc5..dd2e5d1 100644 --- a/llvm/test/Transforms/SimplifyCFG/SpeculativeExec.ll +++ b/llvm/test/Transforms/SimplifyCFG/SpeculativeExec.ll @@ -44,29 +44,6 @@ join: ret i8 %c } -define i8* @test3(i1* %dummy, i8* %a, i8* %b) { -; Test that a weird, unfolded constant cast in the PHI don't block speculation. -; CHECK: @test3 - -entry: - %cond1 = load volatile i1* %dummy - br i1 %cond1, label %if, label %end - -if: - %cond2 = load volatile i1* %dummy - br i1 %cond2, label %then, label %end - -then: - br label %end - -end: - %x = phi i8* [ %a, %entry ], [ %b, %if ], [ inttoptr (i64 42 to i8*), %then ] -; CHECK-NOT: phi -; CHECK: select i1 %cond2, i8* inttoptr - - ret i8* %x -} - define i8* @test4(i1* %dummy, i8* %a, i8* %b) { ; Test that we don't speculate an arbitrarily large number of unfolded constant ; expressions. @@ -108,95 +85,3 @@ end: ret i8* %x10 } - -define i16 @test5(i1* %dummy, i16 %a, i16 %b) { -; Test that we speculate no-op instructions. -; CHECK: @test5 - -entry: - %cond1 = load volatile i1* %dummy - br i1 %cond1, label %if, label %end - -if: - %cond2 = load volatile i1* %dummy - %a.conv = sext i16 %a to i32 - %b.conv = sext i16 %b to i32 - %cmp = icmp ult i32 %a.conv, %b.conv - br i1 %cond2, label %then, label %end - -then: - %sub = sub i32 %a.conv, %b.conv - %sub.conv = trunc i32 %sub to i16 - br label %end - -end: - %x = phi i16 [ %a, %entry ], [ %b, %if ], [ %sub.conv, %then ] -; CHECK-NOT: phi -; CHECK: select i1 - - ret i16 %x -} - -define i16 @test6(i1* %dummy, i64 %a, i64 %b) { -; Test that we speculate no-op instructions when those instructions are in the -; predecessor but could potentially be sunk. -; CHECK: @test6 - -entry: - %cond1 = load volatile i1* %dummy - %a.conv = trunc i64 %a to i16 - %b.conv = trunc i64 %b to i16 - br i1 %cond1, label %if, label %end - -if: - %cond2 = load volatile i1* %dummy - %cond3 = load volatile i1* %dummy - %cond4 = load volatile i1* %dummy - %cmp = icmp ult i16 %a.conv, %b.conv - %a.conv2 = trunc i64 %a to i32 - %b.conv2 = trunc i64 %b to i32 - br i1 %cond2, label %then, label %end - -then: - %sub = sub i32 %a.conv2, %b.conv2 - %sub.conv = trunc i32 %sub to i16 - br label %end - -end: - %x = phi i16 [ %a.conv, %entry ], [ %b.conv, %if ], [ %sub.conv, %then ] -; CHECK-NOT: phi -; CHECK: select i1 - - ret i16 %x -} - -define i16 @test7(i1* %dummy, i16 %a, i16 %b, i32 %x) { -; Test that we don't speculate when there are instructions that could -; potentially sink into the conditional block. -; CHECK: @test7 - -entry: - %cond1 = load volatile i1* %dummy - br i1 %cond1, label %if, label %end - -if: - %cond2 = load volatile i1* %dummy - %a.conv = sext i16 %a to i32 - %b.conv = sext i16 %b to i32 - %cmp = icmp ult i32 %a.conv, %b.conv - %a.conv2 = add i32 %a.conv, %x - br i1 %cond2, label %then, label %end - -then: - %sub = sub i32 %a.conv2, %b.conv - %sub.conv = trunc i32 %sub to i16 - br label %end - -end: - %y = phi i16 [ %a, %entry ], [ %b, %if ], [ %sub.conv, %then ] -; CHECK-NOT: select -; CHECK: phi i16 - - ret i16 %y -} - -- 2.7.4