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

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
+  // remember the BB it is in.  If there is more than one or if *it* is a PHI,
-  // out.  We don't do arbitrary constant expressions here because moving their
+  // bail out.  We don't do arbitrary constant expressions here because moving
-  // computation can be expensive without a cost model.
+  // their computation can be expensive without a cost model.
   BasicBlock *NonConstBB = 0;
   for (unsigned i = 0; i != NumPHIValues; ++i)
     if (!isa<Constant>(PN->getIncomingValue(i)) ||
@@ -1985,21 +1985,23 @@ Instruction *InstCombiner::FoldOpIntoPhi(Instruction &I) {
   if (SelectInst *SI = dyn_cast<SelectInst>(&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<Constant>(PN->getIncomingValue(i))) {
-        if (InC->isNullValue())
+        InV = InC->isNullValue() ? FalseVInPred : TrueVInPred;
-          InV = SI->getFalseValue();
-        else
-          InV = SI->getTrueValue();
       } else {
         assert(PN->getIncomingBlock(i) == NonConstBB);
-        InV = SelectInst::Create(PN->getIncomingValue(i), 
+        InV = SelectInst::Create(PN->getIncomingValue(i), TrueVInPred,
-                                 SI->getTrueValue(), SI->getFalseValue(),
+                                 FalseVInPred,
                                  "phitmp", NonConstBB->getTerminator());
         Worklist.Add(cast<Instruction>(InV));
       }
-      NewPN->addIncoming(InV, PN->getIncomingBlock(i));
+      NewPN->addIncoming(InV, ThisBB);
     }
   } else if (I.getNumOperands() == 2) {
     Constant *C = cast<Constant>(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<Instruction>(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<PHINode>(SI.getCondition()) &&
-      isa<Constant>(SI.getTrueValue()) &&
+      (!isDefinedInBB(SI.getTrueValue(), SI.getParent()) ||
-      isa<Constant>(SI.getFalseValue()))
+       isa<PHINode>(SI.getTrueValue())) &&
+      (!isDefinedInBB(SI.getFalseValue(), SI.getParent()) ||
+       isa<PHINode>(SI.getFalseValue())))
     if (Instruction *NV = FoldOpIntoPhi(SI))
       return NV;
 

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
+}