[PATCH] D74141: [InstCombine] Simplify a umul overflow check to a != 0 && b != 0.
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 14 03:33:12 PST 2020
lebedev.ri removed reviewers: RKSimon, dexonsmith, Bigcheese, aemerson, majnemer.
lebedev.ri added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2726-2743
+ CmpInst::Predicate Pred;
+ Value *UMul1, *UMul2;
+ // 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.
+ auto P = m_ICmp(Pred, m_ExtractValue<0>(m_Value(UMul2)), m_SpecificInt(0));
----------------
Consider doing the following instead:
```
// 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.
CmpInst::Predicate Pred;
Value *MulWithOv;
if (match(&I,
m_c_Or(m_ICmp(Pred, m_OneUse(m_ExtractValue<0>(m_Value(MulWithOv))),
m_ZeroInt()),
m_OneUse(m_ExtractValue<1>(m_Deferred(MulWithOv))))) &&
Pred == CmpInst::ICMP_NE) {
Value *A, *B;
if (match(MulWithOv, m_Intrinsic<Intrinsic::umul_with_overflow>(
m_Value(A), m_Value(B))))
return BinaryOperator::CreateAnd(
Builder.CreateICmpNE(A, ConstantInt::get(A->getType(), 0)),
Builder.CreateICmpNE(A, ConstantInt::get(B->getType(), 0)));
}
```
================
Comment at: llvm/test/Transforms/InstCombine/umul-signed.ll:22
;
%res = tail call { i64, i1 } @llvm.umul.with.overflow.i64(i64 %a, i64 -4775807)
%overflow = extractvalue { i64, i1 } %res, 1
----------------
There shouldn't be any `-4775807` here, please replace it with `%b`
================
Comment at: llvm/test/Transforms/InstCombine/umul-signed.ll:67
; Do not simplify if %overflow has multiple uses.
define i1 @test3_multiple_overflow_users(i64 %a, i64 %b, i64* %ptr) {
----------------
Please add the same test for `%mul`, and `%res`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74141/new/
https://reviews.llvm.org/D74141
More information about the llvm-commits
mailing list