[llvm] r219639 - InstCombine: Fix miscompile in X % -Y -> X % Y transform

[David Majnemer]

Author: majnemer
Date: Mon Oct 13 17:37:51 2014
New Revision: 219639

URL: http://llvm.org/viewvc/llvm-project?rev=219639&view=rev
Log:
InstCombine: Fix miscompile in X % -Y -> X % Y transform

We assumed that negation operations of the form (0 - %Z) resulted in a
negative number.  This isn't true if %Z was originally negative.
Substituting the negative number into the remainder operation may result
in undefined behavior because the dividend might be INT_MIN.

This fixes PR21256.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
    llvm/trunk/test/Transforms/InstCombine/apint-sub.ll
    llvm/trunk/test/Transforms/InstCombine/sub.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=219639&r1=219638&r2=219639&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Mon Oct 13 17:37:51 2014
@@ -1324,15 +1324,15 @@ Instruction *InstCombiner::visitSRem(Bin
   if (Instruction *Common = commonIRemTransforms(I))
     return Common;
 
-  if (Value *RHSNeg = dyn_castNegVal(Op1))
-    if (!isa<Constant>(RHSNeg) ||
-        (isa<ConstantInt>(RHSNeg) &&
-         cast<ConstantInt>(RHSNeg)->getValue().isStrictlyPositive())) {
-      // X % -Y -> X % Y
+  {
+    const APInt *Y;
+    // X % -Y -> X % Y
+    if (match(Op1, m_APInt(Y)) && Y->isNegative() && !Y->isMinSignedValue()) {
       Worklist.AddValue(I.getOperand(1));
-      I.setOperand(1, RHSNeg);
+      I.setOperand(1, ConstantInt::get(I.getType(), -*Y));
       return &I;
     }
+  }
 
   // If the sign bits of both operands are zero (i.e. we can prove they are
   // unsigned inputs), turn this into a urem.

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=219639&r1=219638&r2=219639&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/apint-sub.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/apint-sub.ll Mon Oct 13 17:37:51 2014
@@ -95,12 +95,6 @@ define i1024 @test14(i1024 %A) {
 	ret i1024 %D
 }
 
-define i14 @test15(i14 %A, i14 %B) {
-	%C = sub i14 0, %A		; <i14> [#uses=1]
-	%D = srem i14 %B, %C		; <i14> [#uses=1]
-	ret i14 %D
-}
-
 define i51 @test16(i51 %A) {
 	%X = sdiv i51 %A, 1123		; <i51> [#uses=1]
 	%Y = sub i51 0, %X		; <i51> [#uses=1]

Modified: llvm/trunk/test/Transforms/InstCombine/sub.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/sub.ll?rev=219639&r1=219638&r2=219639&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/sub.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/sub.ll Mon Oct 13 17:37:51 2014
@@ -142,8 +142,9 @@ define i32 @test15(i32 %A, i32 %B) {
 	%D = srem i32 %B, %C
 	ret i32 %D
 ; CHECK-LABEL: @test15(
-; CHECK: %D = srem i32 %B, %A
-; CHECK: ret i32 %D
+; CHECK:      %[[sub:.*]] = sub i32 0, %A
+; CHECK-NEXT: %[[rem:.*]] = srem i32 %B, %[[sub]]
+; CHECK: ret i32 %[[rem]]
 }
 
 define i32 @test16(i32 %A) {