[llvm] r238066 - [InstCombine] Don't eagerly propagate nsw for A*B+A*C => A*(B+C)

[David Majnemer]

Author: majnemer
Date: Fri May 22 18:02:11 2015
New Revision: 238066

URL: http://llvm.org/viewvc/llvm-project?rev=238066&view=rev
Log:
[InstCombine] Don't eagerly propagate nsw for A*B+A*C => A*(B+C)

InstCombine transforms A *nsw B +nsw A *nsw C to A *nsw (B + C).
This is incorrect -- e.g. if A = -1, B = 1, C = INT_SMAX. Then
nothing in the LHS overflows, but the multiplication in RHS overflows.

We need to first make sure that we won't multiple by INT_SMAX + 1.

Test case `add_of_mul` contributed by Sanjoy Das.

This fixes PR23635.

Differential Revision: http://reviews.llvm.org/D9629

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/trunk/test/Transforms/InstCombine/add2.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=238066&r1=238065&r2=238066&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri May 22 18:02:11 2015
@@ -452,6 +452,7 @@ static Value *tryFactorization(InstCombi
   if (!A || !C || !B || !D)
     return nullptr;
 
+  Value *V = nullptr;
   Value *SimplifiedInst = nullptr;
   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
   Instruction::BinaryOps TopLevelOpcode = I.getOpcode();
@@ -468,7 +469,7 @@ static Value *tryFactorization(InstCombi
         std::swap(C, D);
       // Consider forming "A op' (B op D)".
       // If "B op D" simplifies then it can be formed with no cost.
-      Value *V = SimplifyBinOp(TopLevelOpcode, B, D, DL);
+      V = SimplifyBinOp(TopLevelOpcode, B, D, DL);
       // If "B op D" doesn't simplify then only go on if both of the existing
       // operations "A op' B" and "C op' D" will be zapped as no longer used.
       if (!V && LHS->hasOneUse() && RHS->hasOneUse())
@@ -487,7 +488,7 @@ static Value *tryFactorization(InstCombi
         std::swap(C, D);
       // Consider forming "(A op C) op' B".
       // If "A op C" simplifies then it can be formed with no cost.
-      Value *V = SimplifyBinOp(TopLevelOpcode, A, C, DL);
+      V = SimplifyBinOp(TopLevelOpcode, A, C, DL);
 
       // If "A op C" doesn't simplify then only go on if both of the existing
       // operations "A op' B" and "C op' D" will be zapped as no longer used.
@@ -517,7 +518,19 @@ static Value *tryFactorization(InstCombi
         if (BinaryOperator *Op1 = dyn_cast<BinaryOperator>(RHS))
           if (isa<OverflowingBinaryOperator>(Op1))
             HasNSW &= Op1->hasNoSignedWrap();
-        BO->setHasNoSignedWrap(HasNSW);
+
+        // We can propogate 'nsw' if we know that
+        //  %Y = mul nsw i16 %X, C
+        //  %Z = add nsw i16 %Y, %X
+        // =>
+        //  %Z = mul nsw i16 %X, C+1
+        //
+        // iff C+1 isn't INT_MIN
+        const APInt *CInt;
+        if (TopLevelOpcode == Instruction::Add &&
+            InnerOpcode == Instruction::Mul)
+          if (match(V, m_APInt(CInt)) && !CInt->isMinSignedValue())
+            BO->setHasNoSignedWrap(HasNSW);
       }
     }
   }

Modified: llvm/trunk/test/Transforms/InstCombine/add2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/add2.ll?rev=238066&r1=238065&r2=238066&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/add2.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/add2.ll Fri May 22 18:02:11 2015
@@ -273,6 +273,35 @@ define i32 @mul_add_to_mul_6(i32 %x, i32
 ; CHECK-NEXT: ret i32 %add
 }
 
+define i16 @mul_add_to_mul_7(i16 %x) {
+  %mul1 = mul nsw i16 %x, 32767
+  %add2 = add nsw i16 %x, %mul1
+  ret i16 %add2
+; CHECK-LABEL: @mul_add_to_mul_7(
+; CHECK-NEXT: %add2 = shl i16 %x, 15
+; CHECK-NEXT: ret i16 %add2
+}
+
+define i16 @mul_add_to_mul_8(i16 %a) {
+  %mul1 = mul nsw i16 %a, 16383
+  %mul2 = mul nsw i16 %a, 16384
+  %add = add nsw i16 %mul1, %mul2
+  ret i16 %add
+; CHECK-LABEL: @mul_add_to_mul_8(
+; CHECK-NEXT: %add = mul nsw i16 %a, 32767
+; CHECK-NEXT: ret i16 %add
+}
+
+define i16 @mul_add_to_mul_9(i16 %a) {
+  %mul1 = mul nsw i16 %a, 16384
+  %mul2 = mul nsw i16 %a, 16384
+  %add = add nsw i16 %mul1, %mul2
+  ret i16 %add
+; CHECK-LABEL: @mul_add_to_mul_9(
+; CHECK-NEXT: %add = shl i16 %a, 15
+; CHECK-NEXT: ret i16 %add
+}
+
 ; This test and the next test verify that when a range metadata is attached to
 ; llvm.cttz, ValueTracking correctly intersects the range specified by the
 ; metadata and the range implied by the intrinsic.
@@ -353,3 +382,16 @@ define i32 @add_nuw_nsw_or_and(i32 %x, i
 ; CHECK-NEXT: add nuw nsw i32 %x, %y
 ; CHECK-NEXT: ret i32
 }
+
+; A *nsw B + A *nsw C != A *nsw (B + C)
+; e.g. A = -1, B = 1, C = INT_SMAX
+
+define i8 @add_of_mul(i8 %x, i8 %y, i8 %z) {
+; CHECK-LABEL: @add_of_mul(
+ entry:
+  %mA = mul nsw i8 %x, %y
+  %mB = mul nsw i8 %x, %z
+; CHECK: %sum = mul i8
+  %sum = add nsw i8 %mA, %mB
+  ret i8 %sum
+}