On Mon, Jun 1, 2026 at 11:06 PM Jakub Jelinek <[email protected]> wrote: > > Hi! > > The following testcase shows wi::bitreverse_large is buggy. In particular, > it uses the len argument (i.e. number of limbs the xval wide_int has) > to decide how much to clear, but it should be using BLOCKS_NEEDED > (precision) for that and for canonize argument similarly to wi::bswap_large, > because it behaves similarly to that. If xval is say len 1 but precision > 512, we want to clear all 512 bits in the result limbs and the loop then > sets the most significant one in there and canonize with len (512 + 63) / > 64, not initialize just the least significant limb, set various bits in the > uninitialized ones and then only care about the least significant 64 bits > anyway. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok. > > 2026-06-02 Jakub Jelinek <[email protected]> > > PR middle-end/125517 > * wide-int.cc (wi::bitreverse_large): Rename LEN argument to XLEN and > use it as safe_uhwi argument. Add LEN automatic variable set to > BLOCKS_NEEDED (precision). Replace loop clearing val with memset. > Remove I automatic variable. > > * gcc.dg/builtin-bitreverse-9.c: New test. > > --- gcc/wide-int.cc.jj 2026-05-30 17:45:09.497108356 +0200 > +++ gcc/wide-int.cc 2026-06-01 09:26:50.076605916 +0200 > @@ -763,22 +763,21 @@ wi::bswap_large (HOST_WIDE_INT *val, con > return canonize (val, len, precision); > } > > -/* Bitreverse the integer represented by XVAL and LEN into VAL. Return > +/* Bitreverse the integer represented by XVAL and XLEN into VAL. Return > the number of blocks in VAL. Both XVAL and VAL have PRECISION bits. */ > unsigned int > wi::bitreverse_large (HOST_WIDE_INT *val, const HOST_WIDE_INT *xval, > - unsigned int len, unsigned int precision) > + unsigned int xlen, unsigned int precision) > { > - unsigned int i, s; > + unsigned int s, len = BLOCKS_NEEDED (precision); > > - for (i = 0; i < len; i++) > - val[i] = 0; > + memset (val, 0, sizeof (unsigned HOST_WIDE_INT) * len); > > for (s = 0; s < precision; s++) > { > unsigned int block = s / HOST_BITS_PER_WIDE_INT; > unsigned int offset = s & (HOST_BITS_PER_WIDE_INT - 1); > - if (((safe_uhwi (xval, len, block) >> offset) & 1) != 0) > + if (((safe_uhwi (xval, xlen, block) >> offset) & 1) != 0) > { > unsigned int d = (precision - 1) - s; > block = d / HOST_BITS_PER_WIDE_INT; > --- gcc/testsuite/gcc.dg/builtin-bitreverse-9.c.jj 2026-06-01 > 09:34:25.977580064 +0200 > +++ gcc/testsuite/gcc.dg/builtin-bitreverse-9.c 2026-06-01 09:39:01.738935197 > +0200 > @@ -0,0 +1,17 @@ > +/* PR middle-end/125517 */ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-O2" } */ > + > +#if __SIZEOF_INT128__ == 16 > +static_assert (__builtin_bitreverse128 (((unsigned __int128) > + 0x6132fc885cb8cf0cULL << 64) > + | 0xeaa7649b043ae77bULL) > + == (((unsigned __int128) 0xdee75c20d926e557ULL << 64) > + | 0x30f31d3a113f4c86ULL), "bitreverse128 1"); > +static_assert (__builtin_bitreverse128 (0xeaa7649b043ae77bULL) > + == ((unsigned __int128) 0xdee75c20d926e557ULL << 64), > + "bitreverse128 2"); > +static_assert (__builtin_bitreverse128 (0xaa7649b043ae77bULL) > + == ((unsigned __int128) 0xdee75c20d926e550ULL << 64), > + "bitreverse128 3"); > +#endif > > Jakub >
