From ae289632efd31192da8c93d653338cd34f87f85c Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 27 Sep 2009 20:18:49 +0000 Subject: [PATCH] Enhance the previous fix for PR4895 to allow more values than just simple constants for the true/false value of the select. We now do phi translation etc. This really fixes PR4895 :) llvm-svn: 82917 --- .../Scalar/InstructionCombining.cpp | 39 ++++++++++++------- llvm/test/Transforms/InstCombine/select.ll | 31 +++++++++++++-- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp index be88d299ded0..aec9027db021 100644 --- a/llvm/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/llvm/lib/Transforms/Scalar/InstructionCombining.cpp @@ -1949,9 +1949,9 @@ Instruction *InstCombiner::FoldOpIntoPhi(Instruction &I) { // Check to see if all of the operands of the PHI are simple constants // (constantint/constantfp/undef). If there is one non-constant value, - // remember the BB it is. If there is more than one or if *it* is a PHI, bail - // out. We don't do arbitrary constant expressions here because moving their - // computation can be expensive without a cost model. + // remember the BB it is in. If there is more than one or if *it* is a PHI, + // bail out. We don't do arbitrary constant expressions here because moving + // their computation can be expensive without a cost model. BasicBlock *NonConstBB = 0; for (unsigned i = 0; i != NumPHIValues; ++i) if (!isa(PN->getIncomingValue(i)) || @@ -1985,21 +1985,23 @@ Instruction *InstCombiner::FoldOpIntoPhi(Instruction &I) { if (SelectInst *SI = dyn_cast(&I)) { // We only currently try to fold the condition of a select when it is a phi, // not the true/false values. + Value *TrueV = SI->getTrueValue(); + Value *FalseV = SI->getFalseValue(); for (unsigned i = 0; i != NumPHIValues; ++i) { + BasicBlock *ThisBB = PN->getIncomingBlock(i); + Value *TrueVInPred = TrueV->DoPHITranslation(I.getParent(), ThisBB); + Value *FalseVInPred = FalseV->DoPHITranslation(I.getParent(), ThisBB); Value *InV = 0; if (Constant *InC = dyn_cast(PN->getIncomingValue(i))) { - if (InC->isNullValue()) - InV = SI->getFalseValue(); - else - InV = SI->getTrueValue(); + InV = InC->isNullValue() ? FalseVInPred : TrueVInPred; } else { assert(PN->getIncomingBlock(i) == NonConstBB); - InV = SelectInst::Create(PN->getIncomingValue(i), - SI->getTrueValue(), SI->getFalseValue(), + InV = SelectInst::Create(PN->getIncomingValue(i), TrueVInPred, + FalseVInPred, "phitmp", NonConstBB->getTerminator()); Worklist.Add(cast(InV)); } - NewPN->addIncoming(InV, PN->getIncomingBlock(i)); + NewPN->addIncoming(InV, ThisBB); } } else if (I.getNumOperands() == 2) { Constant *C = cast(I.getOperand(1)); @@ -9234,6 +9236,14 @@ Instruction *InstCombiner::visitSelectInstWithICmp(SelectInst &SI, return Changed ? &SI : 0; } +/// isDefinedInBB - Return true if the value is an instruction defined in the +/// specified basicblock. +static bool isDefinedInBB(const Value *V, const BasicBlock *BB) { + const Instruction *I = dyn_cast(V); + return I != 0 && I->getParent() == BB; +} + + Instruction *InstCombiner::visitSelectInst(SelectInst &SI) { Value *CondVal = SI.getCondition(); Value *TrueVal = SI.getTrueValue(); @@ -9446,10 +9456,13 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) { } // See if we can fold the select into a phi node. The true/false values have - // to be live in the predecessor blocks. + // to be live in the predecessor blocks. If they are instructions in SI's + // block, we can't map to the predecessor. if (isa(SI.getCondition()) && - isa(SI.getTrueValue()) && - isa(SI.getFalseValue())) + (!isDefinedInBB(SI.getTrueValue(), SI.getParent()) || + isa(SI.getTrueValue())) && + (!isDefinedInBB(SI.getFalseValue(), SI.getParent()) || + isa(SI.getFalseValue()))) if (Instruction *NV = FoldOpIntoPhi(SI)) return NV; diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll index fc4b82002d30..701d5967a67c 100644 --- a/llvm/test/Transforms/InstCombine/select.ll +++ b/llvm/test/Transforms/InstCombine/select.ll @@ -202,9 +202,9 @@ define i1 @test24(i1 %a, i1 %b) { ret i1 %c } -define i32 @test25() { +define i32 @test25(i1 %c) { entry: - br i1 false, label %jump, label %ret + br i1 %c, label %jump, label %ret jump: br label %ret ret: @@ -213,9 +213,9 @@ ret: ret i32 %b } -define i32 @test26() { +define i32 @test26(i1 %cond) { entry: - br i1 false, label %jump, label %ret + br i1 %cond, label %jump, label %ret jump: %c = or i1 false, false br label %ret @@ -224,3 +224,26 @@ ret: %b = select i1 %a, i32 10, i32 20 ret i32 %b } + +define i32 @test27(i1 %c, i32 %A, i32 %B) { +entry: + br i1 %c, label %jump, label %ret +jump: + br label %ret +ret: + %a = phi i1 [true, %jump], [false, %entry] + %b = select i1 %a, i32 %A, i32 %B + ret i32 %b +} + +define i32 @test28(i1 %cond, i32 %A, i32 %B) { +entry: + br i1 %cond, label %jump, label %ret +jump: + br label %ret +ret: + %c = phi i32 [%A, %jump], [%B, %entry] + %a = phi i1 [true, %jump], [false, %entry] + %b = select i1 %a, i32 %A, i32 %c + ret i32 %b +}