Re: [PATCH] libstdc++: Fix up std::generate_canonical for 32-bit arches

[Jonathan Wakely]

On Wed, 17 Dec 2025 at 14:49, Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> Actually, the
> FAIL: 26_numerics/random/uniform_real_distribution/operators/64351.cc  
> -std=gnu++20 execution test
> FAIL: 26_numerics/random/uniform_real_distribution/operators/gencanon.cc  
> -std=gnu++20 execution test
> errors are gone when testing with --target_board=unix/-m32/-msse2/-mfpmath=sse
> or when the tests are compiled with -O0, which means that it isn't buggy
> unsigned __int128 emulation in that case, but rather either those tests or
> something in random.tcc not being extended precision clean.
>
> So, here is the full patch.  It uses what Tomasz was asking for, i.e. for
> #ifdef __SIZEOF_INT128__ uses __extension__ using __rng_uint128 = unsigned 
> __int128;
> and otherwise the new class.  Tested on x86_64-linux and i686-linux, ok for
> trunk?

We already have a type that does part of this, see
_Select_uint_least_t::type at the top of <bits/random.h>

I've just finished a patch to extend that with the missing operations
needed to support std::philox_engine on 32-bit targets, so my work on
that will also be usable here. I will review what you've done below to
see if I can improve the code I have. But I'll push that work first
for philox, then we can extend it and use it for generate_canonical.



> +
> +    constexpr __rng_uint128
> +    operator+(const __rng_uint128& __x, const __rng_uint128& __y) noexcept
> +    {
> +      return __rng_uint128(__x._M_h + __y._M_h
> +                          + (__x._M_l + __y._M_l < __x._M_l),
> +                          __x._M_l + __y._M_l);

Our existing code uses the overflow checking built-in for addition:

      friend constexpr type
      operator+(type __l, uint64_t __c) noexcept
      {
        __l._M_hi += __builtin_add_overflow(__l._M_lo, __c, &__l._M_lo);
        return __l;
      }

I would expect that to be equivalent, right?

      friend constexpr type
      operator-(type __l, uint64_t __c) noexcept
      {
        __l._M_hi -= __builtin_sub_overflow(__l._M_lo, __c, &__l._M_lo);
        return __l;
      }


> +    }
> +
> +    constexpr __rng_uint128
> +    operator<<(const __rng_uint128& __x, size_t __y) noexcept
> +    {
> +      return (__y >= 64
> +             ? __rng_uint128(__x._M_l << (__y - 64), 0)
> +             : __y
> +             ? __rng_uint128((__x._M_h << __y) | (__x._M_l >> (64 - __y)),
> +                             __x._M_l << __y)
> +             : __x);
> +    }
> +
> +    constexpr __rng_uint128
> +    operator>>(const __rng_uint128& __x, size_t __y) noexcept
> +    {
> +      return (__y >= 64
> +             ? __rng_uint128(0, __x._M_h >> (__y - 64))
> +             : __y
> +             ? __rng_uint128(__x._M_h >> __y,
> +                             (__x._M_l >> __y) | (__x._M_h << (64 - __y)))
> +             : __x);
> +    }

This is what I have (the [[likely]] attributes are because for philox
it usually shifts by exactly 64, but maybe for the general case we
shouldn't assume that):

      friend constexpr type
      operator>>(type __l, unsigned __m) noexcept
      {
        if (__m >= 64) [[__likely__]]
          {
            __l._M_lo = __l._M_hi >> (__m - 64);
            __l._M_hi = 0;
            return __l;
          }

        uint64_t __hi = __l._M_hi >> __m;
        uint64_t __lo = (__l._M_lo >> __m) | (__l._M_hi << (64 - __m));
        type __res(__lo);
        __res._M_hi = __hi;
        return __res;
      }

      friend constexpr type
      operator<<(type __l, unsigned __m) noexcept
      {
        if (__m >= 64) [[__likely__]]
          {
            __l._M_hi = __l._M_lo << (__m - 64);
            __l._M_lo = 0;
            return __l;
          }

        uint64_t __hi = (__l._M_hi << __m) | (__l._M_hi >> (64 - __m));
        uint64_t __lo = (__l._M_lo << __m);
        type __res(__lo);
        __res._M_hi = __hi;
        return __res;
      }