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 >> > >> >
