[PATCH] D101423: [InstCombine] Fold overflow bit of [u|s]mul.with.overflow in a poison-safe way

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2587
 }
 
+/// The @llvm.[us]mul.with.overflow intrinsic could have been folded from some
----------------
aqjune wrote:
> spatel wrote:
> > aqjune wrote:
> > > The function below is based on InstSimplify's version.
> > Duplicating so much code feels wrong. Could we just call the simplify code from here with something like:
> > 
> > ```
> >     // Simulate a bitwise logic op for a select with a constant operand. 
> >     if (match(FalseVal, m_Zero())) {
> >       if (Value *Simplified =
> >               SimplifyAndInst(CondVal, TrueVal, SQ.getWithInstruction(&SI))) {
> >         // create freeze and return
> >       }
> >     }
> >     if (match(TrueVal, m_One())) {
> >       if (Value *Simplified =
> >               SimplifyOrInst(CondVal, FalseVal, SQ.getWithInstruction(&SI))) {
> >         // create freeze and return
> >       }
> >     }
> > 
> > ```
> I think we still need to look into the expression because we should determine which variable to freeze.
> 
> If there is a header file that InstSimplify and InstCombine can share would be a great place for this function?
Don't we only need to freeze the operand that is not in the icmp expression? That is, CondVal of the select must match as an icmp with a non-zero operand, and it's the other value that must be frozen.

I don't know of a shared header in Analysis for general logic. We do have things like VectorUtils and CmpInstAnalysis, so we could invent some new place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101423