[PATCH] D74141: [InstCombine] Simplify a umul overflow check to a != 0 && b != 0.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: Bigcheese, dexonsmith, aemerson.
fhahn added a comment.

(re-adding dropped reviewers)



================
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:
> 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!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74141/new/

https://reviews.llvm.org/D74141