[PATCH] D74141: [InstCombine] Simplify a umul overflow check to a != 0 && b != 0.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 14 07:09:57 PST 2020
fhahn marked an inline comment as done.
fhahn 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));
----------------
lebedev.ri wrote:
> fhahn wrote:
> > lebedev.ri wrote:
> > > lebedev.ri wrote:
> > > > 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)));
> > > > }
> > > > ```
> > > Also, use `Builder.CreateIsNotNull()`
> > Nice, m_c_Or and m_Deferred are very helpful!
> Please adjust one-use checks as they were in the snippet,
> both operands of `or` must be single-use, and we don't care if the intrinsic goes away or not.
> Please adjust one-use checks as they were in the snippet, both operands of or must be single-use
Do you mean both `extractvalues`? I am not sure, why do we need to restrict the number of uses for the multiplication result?
> and we don't care if the intrinsic goes away or not.
Will this still be profitable if the simplification does not result in the umul.with.overflow call to go away?
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