Re: [PATCH] tree-ssa-math-opts: popcount (X) == 1 to (X ^ (X - 1)) > (X - 1) optimization [PR90693]

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/20/23 01:39, Richard Biener wrote:
> On Mon, 20 Nov 2023, Jakub Jelinek wrote:
> 
>> On Mon, Nov 20, 2023 at 07:54:54AM +0000, Richard Biener wrote:
>>> On Fri, 17 Nov 2023, Jakub Jelinek wrote:
>>>> Per the earlier discussions on this PR, the following patch folds
>>>> popcount (x) == 1 (and != 1) into (x ^ (x - 1)) > x - 1 (or <=)
>>>> if the corresponding popcount optab isn't implemented (I think any
>>>> double-word popcount or call will be necessarily slower than the
>>>> above cheap 3 op check and even for -Os larger or same size).
>>>>
>>>> I've noticed e.g. C++ aligned new starts with std::has_single_bit
>>>> which does popcount (x) == 1.
>>>>
>>>> As a follow-up, I'm considering changing in this routine the popcount
>>>> call to IFN_POPCOUNT with 2 arguments and during expansion test costs.
>>>>
>>>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>>
>>> Classically this would have been an RTL expansion alternative, given
>>> we want to do less of those the next place would have been the ISEL
>>> pass.  Any particular reason you chose widening-mul for this (guess
>>> that pass just has a bad name and it's the effective "optimize" ISEL pass
>>> we have).
>>
>> I think the ssa-math-opts pass does far more of this staff than the isel
>> pass which only deals with vector stuff right now and you've even mentioned
>> that pass for that in the PR90693 thread.
>> That said, I can move it into the isel pass as well if you prefer that.
> 
> I think it's fine as you posted (and Jeff approved), I'm just wondering
> if we should rename that pass somehow ;)  Note ISEL is more for
> required pre-expansion stuff and widen-mul is for expansion related but
> optimization parts.
Yea, a rename seems appropriate given how much stuff in there is no 
longer math-ops related :-)
jeff