[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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@238066 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
David Majnemer 2015-05-22 23:02:11 +00:00
parent 464deacf05
commit 12baade498
2 changed files with 58 additions and 3 deletions

View File

@ -452,6 +452,7 @@ static Value *tryFactorization(InstCombiner::BuilderTy *Builder,
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(InstCombiner::BuilderTy *Builder,
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(InstCombiner::BuilderTy *Builder,
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(InstCombiner::BuilderTy *Builder,
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);
}
}
}

View File

@ -273,6 +273,35 @@ define i32 @mul_add_to_mul_6(i32 %x, i32 %y) {
; 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, i32 %y) {
; 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
}