Skip to content

Commit 35fa7b8

Browse files
LebedevRILebedevRI
committed
Reland "[InstCombine] Recognize ((x * y) s/ x) !=/== y as an signed multiplication overflow check (PR48769)"
This reverts commit 91f7a4f, relanding commit 13ec913. The original commit was reverted because of (essentially) https://bugs.llvm.org/show_bug.cgi?id=35922 which has now been addressed by d0eeb64.
1 parent 0a5ebc6 commit 35fa7b8

File tree

4 files changed

+71
-66
lines changed

4 files changed

+71
-66
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3829,19 +3829,22 @@ foldShiftIntoShiftInAnotherHandOfAndInICmp(ICmpInst &I, const SimplifyQuery SQ,
38293829

38303830
/// Fold
38313831
/// (-1 u/ x) u< y
3832-
/// ((x * y) u/ x) != y
3832+
/// ((x * y) ?/ x) != y
38333833
/// to
3834-
/// @llvm.umul.with.overflow(x, y) plus extraction of overflow bit
3834+
/// @llvm.?mul.with.overflow(x, y) plus extraction of overflow bit
38353835
/// Note that the comparison is commutative, while inverted (u>=, ==) predicate
38363836
/// will mean that we are looking for the opposite answer.
3837-
Value *InstCombinerImpl::foldUnsignedMultiplicationOverflowCheck(ICmpInst &I) {
3837+
Value *InstCombinerImpl::foldMultiplicationOverflowCheck(ICmpInst &I) {
38383838
ICmpInst::Predicate Pred;
38393839
Value *X, *Y;
38403840
Instruction *Mul;
3841+
Instruction *Div;
38413842
bool NeedNegation;
38423843
// Look for: (-1 u/ x) u</u>= y
38433844
if (!I.isEquality() &&
3844-
match(&I, m_c_ICmp(Pred, m_OneUse(m_UDiv(m_AllOnes(), m_Value(X))),
3845+
match(&I, m_c_ICmp(Pred,
3846+
m_CombineAnd(m_OneUse(m_UDiv(m_AllOnes(), m_Value(X))),
3847+
m_Instruction(Div)),
38453848
m_Value(Y)))) {
38463849
Mul = nullptr;
38473850

@@ -3856,13 +3859,16 @@ Value *InstCombinerImpl::foldUnsignedMultiplicationOverflowCheck(ICmpInst &I) {
38563859
default:
38573860
return nullptr; // Wrong predicate.
38583861
}
3859-
} else // Look for: ((x * y) u/ x) !=/== y
3862+
} else // Look for: ((x * y) / x) !=/== y
38603863
if (I.isEquality() &&
3861-
match(&I, m_c_ICmp(Pred, m_Value(Y),
3862-
m_OneUse(m_UDiv(m_CombineAnd(m_c_Mul(m_Deferred(Y),
3864+
match(&I,
3865+
m_c_ICmp(Pred, m_Value(Y),
3866+
m_CombineAnd(
3867+
m_OneUse(m_IDiv(m_CombineAnd(m_c_Mul(m_Deferred(Y),
38633868
m_Value(X)),
38643869
m_Instruction(Mul)),
3865-
m_Deferred(X)))))) {
3870+
m_Deferred(X))),
3871+
m_Instruction(Div))))) {
38663872
NeedNegation = Pred == ICmpInst::Predicate::ICMP_EQ;
38673873
} else
38683874
return nullptr;
@@ -3874,19 +3880,22 @@ Value *InstCombinerImpl::foldUnsignedMultiplicationOverflowCheck(ICmpInst &I) {
38743880
if (MulHadOtherUses)
38753881
Builder.SetInsertPoint(Mul);
38763882

3877-
Function *F = Intrinsic::getDeclaration(
3878-
I.getModule(), Intrinsic::umul_with_overflow, X->getType());
3879-
CallInst *Call = Builder.CreateCall(F, {X, Y}, "umul");
3883+
Function *F = Intrinsic::getDeclaration(I.getModule(),
3884+
Div->getOpcode() == Instruction::UDiv
3885+
? Intrinsic::umul_with_overflow
3886+
: Intrinsic::smul_with_overflow,
3887+
X->getType());
3888+
CallInst *Call = Builder.CreateCall(F, {X, Y}, "mul");
38803889

38813890
// If the multiplication was used elsewhere, to ensure that we don't leave
38823891
// "duplicate" instructions, replace uses of that original multiplication
38833892
// with the multiplication result from the with.overflow intrinsic.
38843893
if (MulHadOtherUses)
3885-
replaceInstUsesWith(*Mul, Builder.CreateExtractValue(Call, 0, "umul.val"));
3894+
replaceInstUsesWith(*Mul, Builder.CreateExtractValue(Call, 0, "mul.val"));
38863895

3887-
Value *Res = Builder.CreateExtractValue(Call, 1, "umul.ov");
3896+
Value *Res = Builder.CreateExtractValue(Call, 1, "mul.ov");
38883897
if (NeedNegation) // This technically increases instruction count.
3889-
Res = Builder.CreateNot(Res, "umul.not.ov");
3898+
Res = Builder.CreateNot(Res, "mul.not.ov");
38903899

38913900
// If we replaced the mul, erase it. Do this after all uses of Builder,
38923901
// as the mul is used as insertion point.
@@ -4283,7 +4292,7 @@ Instruction *InstCombinerImpl::foldICmpBinOp(ICmpInst &I,
42834292
}
42844293
}
42854294

4286-
if (Value *V = foldUnsignedMultiplicationOverflowCheck(I))
4295+
if (Value *V = foldMultiplicationOverflowCheck(I))
42874296
return replaceInstUsesWith(I, V);
42884297

42894298
if (Value *V = foldICmpWithLowBitMaskedVal(I, Builder))

‎llvm/lib/Transforms/InstCombine/InstCombineInternal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
661661
Instruction *foldSignBitTest(ICmpInst &I);
662662
Instruction *foldICmpWithZero(ICmpInst &Cmp);
663663

664-
Value *foldUnsignedMultiplicationOverflowCheck(ICmpInst &Cmp);
664+
Value *foldMultiplicationOverflowCheck(ICmpInst &Cmp);
665665

666666
Instruction *foldICmpSelectConstant(ICmpInst &Cmp, SelectInst *Select,
667667
ConstantInt *C);

‎llvm/test/Transforms/InstCombine/signed-mul-lack-of-overflow-check-via-mul-sdiv.ll

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88

99
define i1 @t0_basic(i8 %x, i8 %y) {
1010
; CHECK-LABEL: @t0_basic(
11-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[X:%.*]], [[Y:%.*]]
12-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
13-
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[T1]], [[Y]]
14-
; CHECK-NEXT: ret i1 [[R]]
11+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
12+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
13+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor i1 [[MUL_OV]], true
14+
; CHECK-NEXT: ret i1 [[MUL_NOT_OV]]
1515
;
1616
%t0 = mul i8 %x, %y
1717
%t1 = sdiv i8 %t0, %x
@@ -21,10 +21,10 @@ define i1 @t0_basic(i8 %x, i8 %y) {
2121

2222
define <2 x i1> @t1_vec(<2 x i8> %x, <2 x i8> %y) {
2323
; CHECK-LABEL: @t1_vec(
24-
; CHECK-NEXT: [[T0:%.*]] = mul <2 x i8> [[X:%.*]], [[Y:%.*]]
25-
; CHECK-NEXT: [[T1:%.*]] = sdiv <2 x i8> [[T0]], [[X]]
26-
; CHECK-NEXT: [[R:%.*]] = icmp eq <2 x i8> [[T1]], [[Y]]
27-
; CHECK-NEXT: ret <2 x i1> [[R]]
24+
; CHECK-NEXT: [[MUL:%.*]] = call { <2 x i8>, <2 x i1> } @llvm.smul.with.overflow.v2i8(<2 x i8> [[X:%.*]], <2 x i8> [[Y:%.*]])
25+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { <2 x i8>, <2 x i1> } [[MUL]], 1
26+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor <2 x i1> [[MUL_OV]], <i1 true, i1 true>
27+
; CHECK-NEXT: ret <2 x i1> [[MUL_NOT_OV]]
2828
;
2929
%t0 = mul <2 x i8> %x, %y
3030
%t1 = sdiv <2 x i8> %t0, %x
@@ -37,10 +37,10 @@ declare i8 @gen8()
3737
define i1 @t2_commutative(i8 %x) {
3838
; CHECK-LABEL: @t2_commutative(
3939
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
40-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
41-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
42-
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[T1]], [[Y]]
43-
; CHECK-NEXT: ret i1 [[R]]
40+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
41+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
42+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor i1 [[MUL_OV]], true
43+
; CHECK-NEXT: ret i1 [[MUL_NOT_OV]]
4444
;
4545
%y = call i8 @gen8()
4646
%t0 = mul i8 %y, %x ; swapped
@@ -52,10 +52,10 @@ define i1 @t2_commutative(i8 %x) {
5252
define i1 @t3_commutative(i8 %x) {
5353
; CHECK-LABEL: @t3_commutative(
5454
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
55-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
56-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
57-
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[T1]], [[Y]]
58-
; CHECK-NEXT: ret i1 [[R]]
55+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
56+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
57+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor i1 [[MUL_OV]], true
58+
; CHECK-NEXT: ret i1 [[MUL_NOT_OV]]
5959
;
6060
%y = call i8 @gen8()
6161
%t0 = mul i8 %y, %x ; swapped
@@ -67,10 +67,10 @@ define i1 @t3_commutative(i8 %x) {
6767
define i1 @t4_commutative(i8 %x) {
6868
; CHECK-LABEL: @t4_commutative(
6969
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
70-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
71-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
72-
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[Y]], [[T1]]
73-
; CHECK-NEXT: ret i1 [[R]]
70+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
71+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
72+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor i1 [[MUL_OV]], true
73+
; CHECK-NEXT: ret i1 [[MUL_NOT_OV]]
7474
;
7575
%y = call i8 @gen8()
7676
%t0 = mul i8 %y, %x ; swapped
@@ -85,11 +85,12 @@ declare void @use8(i8)
8585

8686
define i1 @t5_extrause0(i8 %x, i8 %y) {
8787
; CHECK-LABEL: @t5_extrause0(
88-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[X:%.*]], [[Y:%.*]]
89-
; CHECK-NEXT: call void @use8(i8 [[T0]])
90-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
91-
; CHECK-NEXT: [[R:%.*]] = icmp eq i8 [[T1]], [[Y]]
92-
; CHECK-NEXT: ret i1 [[R]]
88+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
89+
; CHECK-NEXT: [[MUL_VAL:%.*]] = extractvalue { i8, i1 } [[MUL]], 0
90+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
91+
; CHECK-NEXT: [[MUL_NOT_OV:%.*]] = xor i1 [[MUL_OV]], true
92+
; CHECK-NEXT: call void @use8(i8 [[MUL_VAL]])
93+
; CHECK-NEXT: ret i1 [[MUL_NOT_OV]]
9394
;
9495
%t0 = mul i8 %x, %y
9596
call void @use8(i8 %t0)

‎llvm/test/Transforms/InstCombine/signed-mul-overflow-check-via-mul-sdiv.ll

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@
88

99
define i1 @t0_basic(i8 %x, i8 %y) {
1010
; CHECK-LABEL: @t0_basic(
11-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[X:%.*]], [[Y:%.*]]
12-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
13-
; CHECK-NEXT: [[R:%.*]] = icmp ne i8 [[T1]], [[Y]]
14-
; CHECK-NEXT: ret i1 [[R]]
11+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
12+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
13+
; CHECK-NEXT: ret i1 [[MUL_OV]]
1514
;
1615
%t0 = mul i8 %x, %y
1716
%t1 = sdiv i8 %t0, %x
@@ -21,10 +20,9 @@ define i1 @t0_basic(i8 %x, i8 %y) {
2120

2221
define <2 x i1> @t1_vec(<2 x i8> %x, <2 x i8> %y) {
2322
; CHECK-LABEL: @t1_vec(
24-
; CHECK-NEXT: [[T0:%.*]] = mul <2 x i8> [[X:%.*]], [[Y:%.*]]
25-
; CHECK-NEXT: [[T1:%.*]] = sdiv <2 x i8> [[T0]], [[X]]
26-
; CHECK-NEXT: [[R:%.*]] = icmp ne <2 x i8> [[T1]], [[Y]]
27-
; CHECK-NEXT: ret <2 x i1> [[R]]
23+
; CHECK-NEXT: [[MUL:%.*]] = call { <2 x i8>, <2 x i1> } @llvm.smul.with.overflow.v2i8(<2 x i8> [[X:%.*]], <2 x i8> [[Y:%.*]])
24+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { <2 x i8>, <2 x i1> } [[MUL]], 1
25+
; CHECK-NEXT: ret <2 x i1> [[MUL_OV]]
2826
;
2927
%t0 = mul <2 x i8> %x, %y
3028
%t1 = sdiv <2 x i8> %t0, %x
@@ -37,10 +35,9 @@ declare i8 @gen8()
3735
define i1 @t2_commutative(i8 %x) {
3836
; CHECK-LABEL: @t2_commutative(
3937
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
40-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
41-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
42-
; CHECK-NEXT: [[R:%.*]] = icmp ne i8 [[T1]], [[Y]]
43-
; CHECK-NEXT: ret i1 [[R]]
38+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
39+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
40+
; CHECK-NEXT: ret i1 [[MUL_OV]]
4441
;
4542
%y = call i8 @gen8()
4643
%t0 = mul i8 %y, %x ; swapped
@@ -52,10 +49,9 @@ define i1 @t2_commutative(i8 %x) {
5249
define i1 @t3_commutative(i8 %x) {
5350
; CHECK-LABEL: @t3_commutative(
5451
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
55-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
56-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
57-
; CHECK-NEXT: [[R:%.*]] = icmp ne i8 [[T1]], [[Y]]
58-
; CHECK-NEXT: ret i1 [[R]]
52+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
53+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
54+
; CHECK-NEXT: ret i1 [[MUL_OV]]
5955
;
6056
%y = call i8 @gen8()
6157
%t0 = mul i8 %y, %x ; swapped
@@ -67,10 +63,9 @@ define i1 @t3_commutative(i8 %x) {
6763
define i1 @t4_commutative(i8 %x) {
6864
; CHECK-LABEL: @t4_commutative(
6965
; CHECK-NEXT: [[Y:%.*]] = call i8 @gen8()
70-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[Y]], [[X:%.*]]
71-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
72-
; CHECK-NEXT: [[R:%.*]] = icmp ne i8 [[Y]], [[T1]]
73-
; CHECK-NEXT: ret i1 [[R]]
66+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y]])
67+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
68+
; CHECK-NEXT: ret i1 [[MUL_OV]]
7469
;
7570
%y = call i8 @gen8()
7671
%t0 = mul i8 %y, %x ; swapped
@@ -85,11 +80,11 @@ declare void @use8(i8)
8580

8681
define i1 @t5_extrause0(i8 %x, i8 %y) {
8782
; CHECK-LABEL: @t5_extrause0(
88-
; CHECK-NEXT: [[T0:%.*]] = mul i8 [[X:%.*]], [[Y:%.*]]
89-
; CHECK-NEXT: call void @use8(i8 [[T0]])
90-
; CHECK-NEXT: [[T1:%.*]] = sdiv i8 [[T0]], [[X]]
91-
; CHECK-NEXT: [[R:%.*]] = icmp ne i8 [[T1]], [[Y]]
92-
; CHECK-NEXT: ret i1 [[R]]
83+
; CHECK-NEXT: [[MUL:%.*]] = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
84+
; CHECK-NEXT: [[MUL_VAL:%.*]] = extractvalue { i8, i1 } [[MUL]], 0
85+
; CHECK-NEXT: [[MUL_OV:%.*]] = extractvalue { i8, i1 } [[MUL]], 1
86+
; CHECK-NEXT: call void @use8(i8 [[MUL_VAL]])
87+
; CHECK-NEXT: ret i1 [[MUL_OV]]
9388
;
9489
%t0 = mul i8 %x, %y
9590
call void @use8(i8 %t0)

0 commit comments

Comments
 (0)