[llvm-commits] [llvm] r60275 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/apint-sub.ll test/Transforms/InstCombine/sdiv-1.ll test/Transforms/InstCombine/sub.ll

[Bill Wendling]

Author: void
Date: Sat Nov 29 21:42:12 2008
New Revision: 60275

URL: http://llvm.org/viewvc/llvm-project?rev=60275&view=rev
Log:
Instcombine was illegally transforming -X/C into X/-C when either X or C
overflowed on negation. This commit checks to make sure that neithe C nor X
overflows. This requires that the RHS of X (a subtract instruction) be a
constant integer.

Added:
    llvm/trunk/test/Transforms/InstCombine/sdiv-1.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
    llvm/trunk/test/Transforms/InstCombine/apint-sub.ll
    llvm/trunk/test/Transforms/InstCombine/sub.ll

Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=60275&r1=60274&r2=60275&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Sat Nov 29 21:42:12 2008
@@ -2956,9 +2956,26 @@
     if (RHS->isAllOnesValue())
       return BinaryOperator::CreateNeg(Op0);
 
-    // -X/C -> X/-C
-    if (Value *LHSNeg = dyn_castNegVal(Op0))
-      return BinaryOperator::CreateSDiv(LHSNeg, ConstantExpr::getNeg(RHS));
+    ConstantInt *RHSNeg = cast<ConstantInt>(ConstantExpr::getNeg(RHS));
+
+    // -X/C -> X/-C, if and only if negation doesn't overflow.
+    if ((RHS->getSExtValue() < 0 &&
+         RHS->getSExtValue() < RHSNeg->getSExtValue()) ||
+        (RHS->getSExtValue() > 0 &&
+         RHS->getSExtValue() > RHSNeg->getSExtValue())) {
+      if (Value *LHSNeg = dyn_castNegVal(Op0)) {
+        if (ConstantInt *CI = dyn_cast<ConstantInt>(LHSNeg)) {
+          ConstantInt *CINeg = cast<ConstantInt>(ConstantExpr::getNeg(CI));
+
+          if ((CI->getSExtValue() < 0 &&
+               CI->getSExtValue() < CINeg->getSExtValue()) ||
+              (CI->getSExtValue() > 0 &&
+               CI->getSExtValue() > CINeg->getSExtValue()))
+            return BinaryOperator::CreateSDiv(LHSNeg,
+                                              ConstantExpr::getNeg(RHS));
+        }
+      }
+    }
   }
 
   // If the sign bits of both operands are zero (i.e. we can prove they are

Modified: llvm/trunk/test/Transforms/InstCombine/apint-sub.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/apint-sub.ll?rev=60275&r1=60274&r2=60275&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/apint-sub.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/apint-sub.ll Sat Nov 29 21:42:12 2008
@@ -3,7 +3,7 @@
 ;
 
 ; RUN: llvm-as < %s | opt -instcombine | llvm-dis | \
-; RUN:   grep -v {sub i19 %Cok, %Bok} | not grep sub
+; RUN:   grep -v {sub i19 %Cok, %Bok} | grep -v {sub i25 0, %Aok} | not grep sub
 ; END.
 
 define i23 @test1(i23 %A) {
@@ -107,8 +107,10 @@
 	ret i51 %Y
 }
 
-define i25 @test17(i25 %A) {
-	%B = sub i25 0, %A		; <i25> [#uses=1]
+; Can't fold subtract here because negation it might oveflow.
+; PR3142
+define i25 @test17(i25 %Aok) {
+	%B = sub i25 0, %Aok		; <i25> [#uses=1]
 	%C = sdiv i25 %B, 1234		; <i25> [#uses=1]
 	ret i25 %C
 }

Added: llvm/trunk/test/Transforms/InstCombine/sdiv-1.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/sdiv-1.ll?rev=60275&view=auto

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/sdiv-1.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/sdiv-1.ll Sat Nov 29 21:42:12 2008
@@ -0,0 +1,22 @@
+; RUN: llvm-as < %s | opt -instcombine -inline | llvm-dis | grep {715827882} | count 2
+; PR3142
+
+define i32 @a(i32 %X) nounwind readnone {
+entry:
+       %0 = sub i32 0, %X
+       %1 = sdiv i32 %0, -3
+       ret i32 %1
+}
+
+define i32 @b(i32 %X) nounwind readnone {
+entry:
+       %0 = call i32 @a(i32 -2147483648)
+       ret i32 %0
+}
+
+define i32 @c(i32 %X) nounwind readnone {
+entry:
+       %0 = sub i32 0, -2147483648
+       %1 = sdiv i32 %0, -3
+       ret i32 %1
+}

Modified: llvm/trunk/test/Transforms/InstCombine/sub.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/sub.ll?rev=60275&r1=60274&r2=60275&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/sub.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/sub.ll Sat Nov 29 21:42:12 2008
@@ -1,7 +1,7 @@
 ; This test makes sure that these instructions are properly eliminated.
 ;
 ; RUN: llvm-as < %s | opt -instcombine | llvm-dis | \
-; RUN:   grep -v {sub i32 %Cok, %Bok} | not grep sub
+; RUN:   grep -v {sub i32 %Cok, %Bok} | grep -v {sub i32 0, %Aok} | not grep sub
 
 define i32 @test1(i32 %A) {
 	%B = sub i32 %A, %A		; <i32> [#uses=1]
@@ -104,8 +104,10 @@
 	ret i32 %Y
 }
 
-define i32 @test17(i32 %A) {
-	%B = sub i32 0, %A		; <i32> [#uses=1]
+; Can't fold subtract here because negation it might oveflow.
+; PR3142
+define i32 @test17(i32 %Aok) {
+	%B = sub i32 0, %Aok		; <i32> [#uses=1]
 	%C = sdiv i32 %B, 1234		; <i32> [#uses=1]
 	ret i32 %C
 }