Fix PR2506 by being a bit more careful about reverse fact propagation when
authorChris Lattner <sabre@nondot.org>
Mon, 14 Jul 2008 00:15:52 +0000 (00:15 +0000)
committerChris Lattner <sabre@nondot.org>
Mon, 14 Jul 2008 00:15:52 +0000 (00:15 +0000)
disproving a condition.  This actually compiles the existing testcase
(udiv_select_to_select_shift) to:

define i64 @test(i64 %X, i1 %Cond) {
entry:
%divisor1.t = lshr i64 %X, 3 ; <i64> [#uses=1]
%quotient2 = lshr i64 %X, 3 ; <i64> [#uses=1]
%sum = add i64 %divisor1.t, %quotient2 ; <i64> [#uses=1]
ret i64 %sum
}

instead of:

define i64 @test(i64 %X, i1 %Cond) {
entry:
%quotient1.v = select i1 %Cond, i64 3, i64 4 ; <i64> [#uses=1]
%quotient1 = lshr i64 %X, %quotient1.v ; <i64> [#uses=1]
%quotient2 = lshr i64 %X, 3 ; <i64> [#uses=1]
%sum = add i64 %quotient1, %quotient2 ; <i64> [#uses=1]
ret i64 %sum
}

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@53534 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Transforms/Scalar/InstructionCombining.cpp
test/Transforms/InstCombine/2008-07-13-DivZero.ll [new file with mode: 0644]
test/Transforms/InstCombine/udiv_select_to_select_shift.ll

index 20580e30ab47d1b4aeb641f14dbb2566769afc00..743c0c1a302fbcc40b0178b57e969c95c07290a6 100644 (file)
@@ -172,6 +172,7 @@ namespace {
     Instruction *visitURem(BinaryOperator &I);
     Instruction *visitSRem(BinaryOperator &I);
     Instruction *visitFRem(BinaryOperator &I);
+    bool SimplifyDivRemOfSelect(BinaryOperator &I);
     Instruction *commonRemTransforms(BinaryOperator &I);
     Instruction *commonIRemTransforms(BinaryOperator &I);
     Instruction *commonDivTransforms(BinaryOperator &I);
@@ -2566,6 +2567,78 @@ Instruction *InstCombiner::visitMul(BinaryOperator &I) {
   return Changed ? &I : 0;
 }
 
+/// SimplifyDivRemOfSelect - Try to fold a divide or remainder of a select
+/// instruction.
+bool InstCombiner::SimplifyDivRemOfSelect(BinaryOperator &I) {
+  SelectInst *SI = cast<SelectInst>(I.getOperand(1));
+  
+  // div/rem X, (Cond ? 0 : Y) -> div/rem X, Y
+  int NonNullOperand = -1;
+  if (Constant *ST = dyn_cast<Constant>(SI->getOperand(1)))
+    if (ST->isNullValue())
+      NonNullOperand = 2;
+  // div/rem X, (Cond ? Y : 0) -> div/rem X, Y
+  if (Constant *ST = dyn_cast<Constant>(SI->getOperand(2)))
+    if (ST->isNullValue())
+      NonNullOperand = 1;
+  
+  if (NonNullOperand == -1)
+    return false;
+  
+  Value *SelectCond = SI->getOperand(0);
+  
+  // Change the div/rem to use 'Y' instead of the select.
+  I.setOperand(1, SI->getOperand(NonNullOperand));
+  
+  // Okay, we know we replace the operand of the div/rem with 'Y' with no
+  // problem.  However, the select, or the condition of the select may have
+  // multiple uses.  Based on our knowledge that the operand must be non-zero,
+  // propagate the known value for the select into other uses of it, and
+  // propagate a known value of the condition into its other users.
+  
+  // If the select and condition only have a single use, don't bother with this,
+  // early exit.
+  if (SI->use_empty() && SelectCond->hasOneUse())
+    return true;
+  
+  // Scan the current block backward, looking for other uses of SI.
+  BasicBlock::iterator BBI = &I, BBFront = I.getParent()->begin();
+  
+  while (BBI != BBFront) {
+    --BBI;
+    // If we found a call to a function, we can't assume it will return, so
+    // information from below it cannot be propagated above it.
+    if (isa<CallInst>(BBI) && !isa<IntrinsicInst>(BBI))
+      break;
+    
+    // Replace uses of the select or its condition with the known values.
+    for (Instruction::op_iterator I = BBI->op_begin(), E = BBI->op_end();
+         I != E; ++I) {
+      if (*I == SI) {
+        *I = SI->getOperand(NonNullOperand);
+        AddToWorkList(BBI);
+      } else if (*I == SelectCond) {
+        *I = NonNullOperand == 1 ? ConstantInt::getTrue() :
+                                   ConstantInt::getFalse();
+        AddToWorkList(BBI);
+      }
+    }
+    
+    // If we past the instruction, quit looking for it.
+    if (&*BBI == SI)
+      SI = 0;
+    if (&*BBI == SelectCond)
+      SelectCond = 0;
+    
+    // If we ran out of things to eliminate, break out of the loop.
+    if (SelectCond == 0 && SI == 0)
+      break;
+    
+  }
+  return true;
+}
+
+
 /// This function implements the transforms on div instructions that work
 /// regardless of the kind of div instruction it is (udiv, sdiv, or fdiv). It is
 /// used by the visitors to those instructions.
@@ -2585,40 +2658,6 @@ Instruction *InstCombiner::commonDivTransforms(BinaryOperator &I) {
   if (isa<UndefValue>(Op1))
     return ReplaceInstUsesWith(I, Op1);
 
-  // Handle cases involving: [su]div X, (select Cond, Y, Z)
-  // This does not apply for fdiv.
-  if (SelectInst *SI = dyn_cast<SelectInst>(Op1)) {
-    // [su]div X, (Cond ? 0 : Y) -> div X, Y.  If the div and the select are in
-    // the same basic block, then we replace the select with Y, and the
-    // condition of the select with false (if the cond value is in the same BB).
-    // If the select has uses other than the div, this allows them to be
-    // simplified also. Note that div X, Y is just as good as div X, 0 (undef)
-    if (ConstantInt *ST = dyn_cast<ConstantInt>(SI->getOperand(1)))
-      if (ST->isNullValue()) {
-        Instruction *CondI = dyn_cast<Instruction>(SI->getOperand(0));
-        if (CondI && CondI->getParent() == I.getParent())
-          UpdateValueUsesWith(CondI, ConstantInt::getFalse());
-        else if (I.getParent() != SI->getParent() || SI->hasOneUse())
-          I.setOperand(1, SI->getOperand(2));
-        else
-          UpdateValueUsesWith(SI, SI->getOperand(2));
-        return &I;
-      }
-
-    // Likewise for: [su]div X, (Cond ? Y : 0) -> div X, Y
-    if (ConstantInt *ST = dyn_cast<ConstantInt>(SI->getOperand(2)))
-      if (ST->isNullValue()) {
-        Instruction *CondI = dyn_cast<Instruction>(SI->getOperand(0));
-        if (CondI && CondI->getParent() == I.getParent())
-          UpdateValueUsesWith(CondI, ConstantInt::getTrue());
-        else if (I.getParent() != SI->getParent() || SI->hasOneUse())
-          I.setOperand(1, SI->getOperand(1));
-        else
-          UpdateValueUsesWith(SI, SI->getOperand(1));
-        return &I;
-      }
-  }
-
   return 0;
 }
 
@@ -2643,6 +2682,11 @@ Instruction *InstCombiner::commonIDivTransforms(BinaryOperator &I) {
   
   if (Instruction *Common = commonDivTransforms(I))
     return Common;
+  
+  // Handle cases involving: [su]div X, (select Cond, Y, Z)
+  // This does not apply for fdiv.
+  if (isa<SelectInst>(Op1) && SimplifyDivRemOfSelect(I))
+    return &I;
 
   if (ConstantInt *RHS = dyn_cast<ConstantInt>(Op1)) {
     // div X, 1 == X
@@ -2798,36 +2842,8 @@ Instruction *InstCombiner::commonRemTransforms(BinaryOperator &I) {
     return ReplaceInstUsesWith(I, Op1);  // X % undef -> undef
 
   // Handle cases involving: rem X, (select Cond, Y, Z)
-  if (SelectInst *SI = dyn_cast<SelectInst>(Op1)) {
-    // rem X, (Cond ? 0 : Y) -> rem X, Y.  If the rem and the select are in
-    // the same basic block, then we replace the select with Y, and the
-    // condition of the select with false (if the cond value is in the same
-    // BB).  If the select has uses other than the div, this allows them to be
-    // simplified also.
-    if (Constant *ST = dyn_cast<Constant>(SI->getOperand(1)))
-      if (ST->isNullValue()) {
-        Instruction *CondI = dyn_cast<Instruction>(SI->getOperand(0));
-        if (CondI && CondI->getParent() == I.getParent())
-          UpdateValueUsesWith(CondI, ConstantInt::getFalse());
-        else if (I.getParent() != SI->getParent() || SI->hasOneUse())
-          I.setOperand(1, SI->getOperand(2));
-        else
-          UpdateValueUsesWith(SI, SI->getOperand(2));
-        return &I;
-      }
-    // Likewise for: rem X, (Cond ? Y : 0) -> rem X, Y
-    if (Constant *ST = dyn_cast<Constant>(SI->getOperand(2)))
-      if (ST->isNullValue()) {
-        Instruction *CondI = dyn_cast<Instruction>(SI->getOperand(0));
-        if (CondI && CondI->getParent() == I.getParent())
-          UpdateValueUsesWith(CondI, ConstantInt::getTrue());
-        else if (I.getParent() != SI->getParent() || SI->hasOneUse())
-          I.setOperand(1, SI->getOperand(1));
-        else
-          UpdateValueUsesWith(SI, SI->getOperand(1));
-        return &I;
-      }
-  }
+  if (isa<SelectInst>(Op1) && SimplifyDivRemOfSelect(I))
+    return &I;
 
   return 0;
 }
diff --git a/test/Transforms/InstCombine/2008-07-13-DivZero.ll b/test/Transforms/InstCombine/2008-07-13-DivZero.ll
new file mode 100644 (file)
index 0000000..85c3dbc
--- /dev/null
@@ -0,0 +1,16 @@
+; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {lshr.*3}
+; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {call .*%cond}
+; PR2506
+
+; We can simplify the operand of udiv to '8', but not the operand to the
+; call.  If the callee never returns, we can't assume the div is reachable.
+define i32 @a(i32 %x, i32 %y) {
+entry:
+        %tobool = icmp ne i32 %y, 0             ; <i1> [#uses=1]
+        %cond = select i1 %tobool, i32 8, i32 0         ; <i32> [#uses=2]
+        %call = call i32 @b( i32 %cond )                ; <i32> [#uses=0]
+        %div = udiv i32 %x, %cond               ; <i32> [#uses=1]
+        ret i32 %div
+}
+
+declare i32 @b(i32)
index 277592c983da06c2e5ebdba7e0b6d2b130aa3e36..614ae3dc975c7c5ca16032e1314b4225165a25cd 100644 (file)
@@ -2,13 +2,13 @@
 ; udiv X, (Select Cond, C1, C2) --> Select Cond, (shr X, C1), (shr X, C2)
 ;
 ; RUN: llvm-as < %s | opt -instcombine | llvm-dis -f -o %t
-; RUN:   grep select %t | count 1
+; RUN:   not grep select %t
 ; RUN:   grep lshr %t | count 2
-; RUN:   ignore grep udiv %t | count 0
+; RUN:   not grep udiv %t
 
 define i64 @test(i64 %X, i1 %Cond ) {
 entry:
-        %divisor1 = select i1 %Cond, i64 8, i64 16
+        %divisor1 = select i1 %Cond, i64 16, i64 8
         %quotient1 = udiv i64 %X, %divisor1
         %divisor2 = select i1 %Cond, i64 8, i64 0
         %quotient2 = udiv i64 %X, %divisor2