[PATCH] D71568: [InstCombine] `select + mul` -> `select + shl` with power of twos.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D71568#1812498 <https://reviews.llvm.org/D71568#1812498>, @danlark wrote:

> In D71568#1812463 <https://reviews.llvm.org/D71568#1812463>, @danlark wrote:
>
> > In D71568#1812426 <https://reviews.llvm.org/D71568#1812426>, @spatel wrote:
> >
> > > In D71568#1812355 <https://reviews.llvm.org/D71568#1812355>, @spatel wrote:
> > >
> > > > There's a lot going on here, and the patch doesn't apply cleanly to current source. Can you pre-commit any NFC changes/tests to make this patch smaller? For example, the udiv change/tests are not affected by the mul code?
> > >
> > >
> > > Sorry - I didn't see earlier that this patch is part of a sequence. 
> > >  I'm a bit skeptical about the need to extend the "udiv action" machine. Are the motivating cases really that complicated or could we get away with a simpler pattern match of mul(select...)?
> >
> >
> > I am not sure but I just reused the code for both division and multiplication -- from my perspective the idea is the same.
> >
> > Also, I rebased a bit and made the patch smaller as you asked.
>
>
> So, I basically don't see any reason division machine is doing a lot of things but as abstractions and semantics are mostly the same, I decided to reuse it. And to generalize power 2 multiplication then.
>
> That's true that most of patterns are just one `mul(select)` but I saw at least one where the depth level was two.


Yes, I understand the similarity, I'm just not sold on the idea that we ever needed the div complexity in the first place. If @lebedev.ri is comfortable with all of the poison-related corner cases, then we can proceed. AFAIK, there are no active contributers on this code, so if you've looked it over - then you're the expert. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71568