Re: [PATCH] middle-end: If supported, use __builtin_bitreverse64 on reflect_hwi. [PR126625]

On Fri, Aug 7, 2026 at 12:26 PM Kael Andrew Franco
<[email protected]> wrote:
>
> Likewise for ira-color.cc?

Oh I didn't know there was an use of __has_builtin there; it is in
code that I don't read that often.

So it should be HAS_* after all. But `Conditional Compilation` section
of the GNU Coding standards suggest always 1/0 instead of
define/undefine.
Meaning the code in ira-color.cc should be changed too.  Maybe even
putting both defines in system.h might be a good idea too.

Thanks,
Andrea

>
> On Fri, Aug 7, 2026 at 1:46 AM Andrea Pinski <[email protected]> wrote:
>>
>> On Thu, Aug 6, 2026 at 3:24 AM Kael Andrew Franco <[email protected]> wrote:
>> >
>> > From f3df22990341e3ff247160509c41e8b123aff0bd Mon Sep 17 00:00:00 2001
>> > From: Kael Andrew Alonzo Franco <[email protected]>
>> > Date: Wed, 5 Aug 2026 12:50:20 -0400
>> > Subject: [PATCH] middle-end: If supported, use __builtin_bitreverse64 on reflect_hwi. [PR126625]
>> >
>> > reflect_hwi uses a naive for loop approach to emulate __builtin_bitreverse64 ().
>> > This is slow compared to using __builtin_bitreverse64 () plus bitshift.
>> > This is useful for bootstrapping GCC since r17-523.
>> > Also add assert on BITWIDTH <= 64.
>> >
>> > Bootstrapped and regtested on x86_64-pc-linux-gnu.
>> >
>> > PR middle-end/126625
>> >
>> > gcc/ChangeLog:
>> >
>> > * hwint.cc (reflect_hwi): If supported, use __builtin_bitreverse64.
>> >
>> > Signed-off-by: Kael Andrew Franco <[email protected]>
>> > ---
>> >  gcc/hwint.cc | 19 ++++++++++++++++++-
>> >  1 file changed, 18 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/gcc/hwint.cc b/gcc/hwint.cc
>> > index f3b3e7b8408..e87956ed3f5 100644
>> > --- a/gcc/hwint.cc
>> > +++ b/gcc/hwint.cc
>> > @@ -189,12 +189,27 @@ least_common_multiple (HOST_WIDE_INT a, HOST_WIDE_INT b)
>> >    return mul_hwi (abs_hwi (a) / gcd (a, b), abs_hwi (b));
>> >  }
>> >
>> > -/* Reflect (reverse) the bits of a given VALUE within a specified BITWIDTH.  */
>> > +#ifdef __has_builtin
>> > +#if __has_builtin(__builtin_bitreverse64)
>> > +#define HAVE_BITREVERSE64
>> > +#endif
>> > +#endif
>>
>> I think it is better to do:
>> #define HAVE_BITREVERSE64 0
>> #ifdef __has_builtin
>> # if __has_builtin(__builtin_bitreverse64)
>> #  define HAVE_BITREVERSE64 1
>> #endif
>> #endif
>>
>> > +
>> > +/* Reflect (reverse) the bits of a given VALUE within a specified BITWIDTH <= 64.  */
>> >
>> >  unsigned HOST_WIDE_INT
>> >  reflect_hwi (unsigned HOST_WIDE_INT value, unsigned bitwidth)
>> >  {
>> > +  if (bitwidth == 0)
>> > +    return 0;
>> > +
>> > +  gcc_checking_assert (bitwidth <= 64);
>> > +
>> > +#ifdef HAS_BITREVERSE64
>> #if HAVE_BITREVERSE64
>> > +  return __builtin_bitreverse64 (value) >> (64 - bitwidth);
>> > +#else
>> >    unsigned HOST_WIDE_INT reflected_value = 0;
>> > +
>> >    /* Loop through each bit in the specified BITWIDTH.  */
>> >    for (size_t i = 0; i < bitwidth; i++)
>> >      {
>> > @@ -204,5 +219,7 @@ reflect_hwi (unsigned HOST_WIDE_INT value, unsigned bitwidth)
>> >        reflected_value |= (value & 1);
>> >        value >>= 1;
>> >      }
>> > +
>> >    return reflected_value;
>> > +#endif
>> >  }
>> > --
>> > 2.55.0
>> >
>> >