[PATCH] D45173: [InstCombine] Recognize idioms for ctpop and ctlz

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45173#1059970, @kparzysz wrote:

> In https://reviews.llvm.org/D45173#1059947, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45173#1059921, @kparzysz wrote:
> >
> > > Moving aggressive instcombine pass to before the regular instcombine, just to demonstrate the idea.
> >
> >
> > I'm curious to know, while this does sidestep the issue of having to adapt to the instcombine/earlier passes being smarter, how does this not either [...]
>
>
> The patterns that we're trying to match correspond to some "typical implementations", so it's not intended to detect any possible computation that, for example, calculates population count.


Ah, i see. Disappointing. That design decision should really be specified in the documentation.

To point the obvious, if one takes one of these tests, runs those through instcombine
and then runs aggressiveinstcombine, chances are they will no longer be matched...
(Well, the main point being, any non-expected, slightly different pattern, that i would normally expected to be slightly canonicalized beforehand),

Also, while that may be a net positive tradeoff for this particular combine (`BitCountCombine`),
is that also true for all other possible future combines in this pass?

> Different popcount algorithms would obviously need different matching codes.  That's not a loss, since instcombine cannot now and likely will never be able to reorganize entire algorithms into a common way.  The big problem with instcombine now is that its ongoing development results in a continuous stream of small changes to the output which then require ongoing adjustments to the matching code.  Look at HexagonLoopIdiomRecognition.cpp to see the extent to which it goes to try and see through what instcombine has done.




Repository:
  rL LLVM

https://reviews.llvm.org/D45173