[PATCH v2] match.pd: popcount(X & -X) -> (X & -X) != 0 [PR102486]

[Jeffrey Law]

On 1/4/2026 3:26 PM, Andrew Pinski wrote:
> On Sun, Jan 4, 2026 at 2:13 PM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>>
>>
>> On 1/4/2026 2:54 PM, Daniel Henrique Barboza wrote:
>>>
>>> On 1/4/26 4:56 PM, Andrew Pinski wrote:
>>>> On Sun, Jan 4, 2026 at 11:32 AM Jeffrey Law
>>>> <jeffrey.law@oss.qualcomm.com> wrote:
>>>>>
>>>>>
>>>>> On 12/30/2025 2:34 PM, Daniel Barboza wrote:
>>>>>>            PR 102486
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>>             * match.pd (`popcount (X & -X) -> (X & -X) != 0`): New
>>>>>> pattern.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>>         * gcc.dg/tree-ssa/pr102486.c: New test.
>>>>> So isn't it transformed into "X != 0" rather than "(X & -X) != 0)"?
>>>>> Which means the subject line and ChangeLog entry need minor updates.
>>>>> I'm not sure why Andrew suggested optimizing to (X & -X) != 0 since the
>>>>> X & -X is zero when X == 0 and nonzero when X != 0.
>>>> I don't remember why I suggested optimizing it to `(X & -X) != 0`
>>>> instead of just `X != 0`. The bug report was filed 4 years ago and I
>>>> can't find my notes for what I was looking at that time or why 2 years
>>>> later mentioned `(X&-X) != 0` either. But yes it should be to `X !=
>>>> 0`.
>>> Do we still care about the extra checks we introduced in v2 w.r.t the
>>> result
>>> of the optional conversion? We have this:
>>>
>>> +  (POPCOUNT (convert?@1 (bit_and:c @0 (negate @0))))
>>> +    (with { tree type0 = TREE_TYPE (@0);
>>> +        tree type1 = TREE_TYPE (@1); }
>>> +      (if (INTEGRAL_TYPE_P (type0)
>>> +       && (TYPE_UNSIGNED (type0)
>>> +           || TYPE_PRECISION (type1) <= TYPE_PRECISION (type0)))
>>> +    /* Use an explicit bool conversion to avoid errors when the
>>> +       POPCOUNT result is assigned to a type (e.g. int) that
>>> +       will generate a "bogus comparison result type" error.  */
>>> +    (convert:type
>>> +      (ne:boolean_type_node @1 { build_zero_cst (type1); })))))
>>>
>>>
>>> I assume that these checks
>>>
>>> +       && (TYPE_UNSIGNED (type0)
>>> +           || TYPE_PRECISION (type1) <= TYPE_PRECISION (type0)))
>>>
>>> Are still relevant because the conversion can still add additional
>>> ones aside from
>>> the signal bit if we happen to go from a signed type that is larger.
>> I can't offhand think of a case were the extension would be
>> problematical.  The only case that's close in my mind would be
>> 0x80000000 (assuming a 32bit type), where its negation would also be
>> 0x80000000.  But I think the property still holds in that case.
>>
>> Other thoughts Andrew?
> The case where I am worried about is:
> ```
> int f(signed char a) // Take a = -128
> {
>    signed char b = a & -a; // -128
>    int c = b; // -128 or rather 0xffffff80
>    int d = __builtin_popcount(c); // 24+1
>    return d;
> }
> ```
> Notice how d is not 1  but rather 25 when a is -128.
Right, I guess with the convert? in the match.pd pattern that would be 
possible.  I wasn't considering type changes within the pattern.  So 
yea, we need to keep those tests or a variant thereof.

Jeff