[PATCH] D110488: [InstCombine] Fold ctpop((x & -x ) - 1) -> cttz(x, false) (PR51784)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D110488#3022686 <https://reviews.llvm.org/D110488#3022686>, @xbolva00 wrote:

> In D110488#3022683 <https://reviews.llvm.org/D110488#3022683>, @RKSimon wrote:
>
>> An alternative would be to decide if (~x & (x - 1)) or ((x & -x ) - 1) should be the canonical pattern?
>>
>> https://alive2.llvm.org/ce/z/WA_cmX
>
> Yeah I was thinking about some more general pattern too. @spatel?

I'd go with the 'not' pattern since that is likely better for known bits - and that's what we were already expecting here. 
But if either of the 'and' or 'sub' ops has an extra use, then we can't do the 'not' canonicalization, so we'd want this fold too.

So this patch might be made less important with the 'not' canonicalization, but it could still fire. 
I expect this has no detectable compile-time cost and it's a one-line patch, so seems fine to keep it. 
But please add tests with extra uses on the preceding ops, so we have motivating (even if they are contrived) regression tests for this patch. That would still show the value of this patch whether we do the other transform or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110488