Skip to content

Commit

Permalink
[InstCombine] Simplify a umul overflow check to a != 0 && b != 0.
Browse files Browse the repository at this point in the history 
This patch adds a simplification if an OR weakens the overflow condition
for umul.with.overflow by treating any non-zero result as overflow. In that
case, we overflow if both umul.with.overflow operands are != 0, as in that
case the result can only be 0, iff the multiplication overflows.

Code like this is generated by code using __builtin_mul_overflow with
negative integer constants, e.g.
   bool test(unsigned long long v, unsigned long long *res) {
     return __builtin_mul_overflow(v, -4775807LL, res);
   }

```
----------------------------------------
Name: D74141
  %res = umul_overflow {i8, i1} %a, %b
  %mul = extractvalue {i8, i1} %res, 0
  %overflow = extractvalue {i8, i1} %res, 1
  %cmp = icmp ne %mul, 0
  %ret = or i1 %overflow, %cmp
  ret i1 %ret
=>
  %t0 = icmp ne i8 %a, 0
  %t1 = icmp ne i8 %b, 0
  %ret = and i1 %t0, %t1
  ret i1 %ret
  %res = umul_overflow {i8, i1} %a, %b
  %mul = extractvalue {i8, i1} %res, 0
  %cmp = icmp ne %mul, 0
  %overflow = extractvalue {i8, i1} %res, 1

Done: 1
Optimization is correct!

```

Reviewers: nikic, lebedev.ri, spatel, Bigcheese, dexonsmith, aemerson

Reviewed By: lebedev.ri

Differential Revision: https://reviews.llvm.org/D74141

(Cherry-picked from 6c85e92)
  • Loading branch information
@fhahn
fhahn committed Feb 20, 2020
1 parent 5ab84b6 commit 379c69d
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 26 deletions.
25 changes: 25 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
View file
Expand Up @@ -2729,6 +2729,31 @@ Instruction *InstCombiner::visitOr(BinaryOperator &I) {
canonicalizeCondSignextOfHighBitExtractToSignextHighBitExtract(I))
return V;

CmpInst::Predicate Pred;
Value *Mul, *Ov, *MulIsNotZero, *UMulWithOv;
// Check if the OR weakens the overflow condition for umul.with.overflow by
// treating any non-zero result as overflow. In that case, we overflow if both
// umul.with.overflow operands are != 0, as in that case the result can only
// be 0, iff the multiplication overflows.
if (match(&I,
m_c_Or(m_CombineAnd(m_ExtractValue<1>(m_Value(UMulWithOv)),
m_Value(Ov)),
m_CombineAnd(m_ICmp(Pred,
m_CombineAnd(m_ExtractValue<0>(
m_Deferred(UMulWithOv)),
m_Value(Mul)),
m_ZeroInt()),
m_Value(MulIsNotZero)))) &&
(Ov->hasOneUse() || (MulIsNotZero->hasOneUse() && Mul->hasOneUse())) &&
Pred == CmpInst::ICMP_NE) {
Value *A, *B;
if (match(UMulWithOv, m_Intrinsic<Intrinsic::umul_with_overflow>(
m_Value(A), m_Value(B))))

return BinaryOperator::CreateAnd(Builder.CreateIsNotNull(A),
Builder.CreateIsNotNull(B));
}

return nullptr;
}

Expand Down
48 changes: 22 additions & 26 deletions llvm/test/Transforms/InstCombine/umul-sign-check.ll
View file
Expand Up @@ -13,11 +13,10 @@ declare { i64, i1 } @llvm.umul.with.overflow.i64(i64, i64) #0

define i1 @test1(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test1(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[OVERFLOW]], [[CMP]]
; CHECK-NEXT: [[MUL:%.*]] = mul i64 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: store i64 [[MUL]], i64* [[PTR:%.*]], align 8
; CHECK-NEXT: ret i1 [[OVERFLOW_1]]
;
Expand All @@ -33,11 +32,10 @@ define i1 @test1(i64 %a, i64 %b, i64* %ptr) {

define i1 @test1_or_ops_swapped(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test1_or_ops_swapped(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[CMP]], [[OVERFLOW]]
; CHECK-NEXT: [[MUL:%.*]] = mul i64 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: store i64 [[MUL]], i64* [[PTR:%.*]], align 8
; CHECK-NEXT: ret i1 [[OVERFLOW_1]]
;
Expand All @@ -54,11 +52,10 @@ define i1 @test1_or_ops_swapped(i64 %a, i64 %b, i64* %ptr) {

define i1 @test2(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test2(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[OVERFLOW]], [[CMP]]
; CHECK-NEXT: [[MUL:%.*]] = mul i64 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[MUL]]
; CHECK-NEXT: store i64 [[NEG]], i64* [[PTR:%.*]], align 8
; CHECK-NEXT: ret i1 [[OVERFLOW_1]]
Expand All @@ -80,9 +77,9 @@ define i1 @test3_multiple_overflow_users(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test3_multiple_overflow_users(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[OVERFLOW]], [[CMP]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: call void @use(i1 [[OVERFLOW]])
; CHECK-NEXT: ret i1 [[OVERFLOW_1]]
;
Expand Down Expand Up @@ -124,10 +121,10 @@ declare void @use.2({ i64, i1 })
define i1 @test3_multiple_res_users(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test3_multiple_res_users(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[OVERFLOW]], [[CMP]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[MUL]]
; CHECK-NEXT: store i64 [[NEG]], i64* [[PTR:%.*]], align 8
; CHECK-NEXT: call void @use.2({ i64, i1 } [[RES]])
Expand All @@ -149,11 +146,10 @@ declare void @use.3(i64)
; Simplify if %mul has multiple uses.
define i1 @test3_multiple_mul_users(i64 %a, i64 %b, i64* %ptr) {
; CHECK-LABEL: @test3_multiple_mul_users(
; CHECK-NEXT: [[RES:%.*]] = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 [[A:%.*]], i64 [[B:%.*]])
; CHECK-NEXT: [[OVERFLOW:%.*]] = extractvalue { i64, i1 } [[RES]], 1
; CHECK-NEXT: [[MUL:%.*]] = extractvalue { i64, i1 } [[RES]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64 [[MUL]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = or i1 [[OVERFLOW]], [[CMP]]
; CHECK-NEXT: [[MUL:%.*]] = mul i64 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[A]], 0
; CHECK-NEXT: [[TMP2:%.*]] = icmp ne i64 [[B]], 0
; CHECK-NEXT: [[OVERFLOW_1:%.*]] = and i1 [[TMP1]], [[TMP2]]
; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[MUL]]
; CHECK-NEXT: store i64 [[NEG]], i64* [[PTR:%.*]], align 8
; CHECK-NEXT: call void @use.3(i64 [[MUL]])
Expand Down

0 comments on commit 379c69d

Please sign in to comment.