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?

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

Reply via email to