On Wed, Dec 17, 2025 at 8:51 PM Jonathan Wakely <[email protected]> wrote:

> Make use of __detail::_Select_uint_least_t<d>::type for
> std::generate_canonical, so that we have a 128-bit integer type even on
> 32-bit targets.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/random.tcc (__generate_canonical_pow2): Elide
>         variable into poopcount call. Cast floating-point literal to
>         _RealT.
>         (__generate_canonical_any): Use direct-initialization for _UInt
>         variables. Cast floating-point literal to _RealT.
>         (generate_canonical): Remove unused typedef. Use
>         _Select_uint_least_t to select the unsigned integer type to use.
>
> Co-authored-by: Jakub Jelinek <[email protected]>
> ---
>
> Tested x86_64-linux
> Also checked -m32 and -D_GLIBCXX_USE_OLD_GENERATE_CANONICAL
>
>  libstdc++-v3/include/bits/random.tcc | 51 +++++++---------------------
>  1 file changed, 12 insertions(+), 39 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/random.tcc
> b/libstdc++-v3/include/bits/random.tcc
> index 68eae419b721..6d9ce7d6a623 100644
> --- a/libstdc++-v3/include/bits/random.tcc
> +++ b/libstdc++-v3/include/bits/random.tcc
> @@ -3522,10 +3522,9 @@ namespace __detail
>        // r = 2;  // Redundant, we only support radix 2.
>        using _Rng = decltype(_Urbg::max());
>        const _Rng __rng_range_less_1 = _Urbg::max() - _Urbg::min();
> -      const _UInt __uint_range_less_1 = ~_UInt(0);
>        // R = _UInt(__rng_range_less_1) + 1;  // May wrap to 0.
>        const auto __log2_R = __builtin_popcountg(__rng_range_less_1);
> -      const auto __log2_uint_max =
> __builtin_popcountg(__uint_range_less_1);
> +      const auto __log2_uint_max = __builtin_popcountg(~_UInt(0));
>        // rd = _UInt(1) << __d;  // Could overflow, UB.
>        const unsigned __k = (__d + __log2_R - 1) / __log2_R;
>        const unsigned __log2_Rk_max = __k * __log2_R;
> @@ -3533,7 +3532,7 @@ namespace __detail
>         __log2_uint_max < __log2_Rk_max ? __log2_uint_max : __log2_Rk_max;
>        static_assert(__log2_Rk >= __d);
>        // Rk = _UInt(1) << __log2_Rk;  // Likely overflows, UB.
> -      constexpr _RealT __Rk = _RealT(_UInt(1) << (__log2_Rk - 1)) * 2.0;
> +      constexpr _RealT __Rk = _RealT(_UInt(1) << (__log2_Rk - 1)) *
> _RealT(2.0);
>  #if defined(_GLIBCXX_GENERATE_CANONICAL_STRICT)
>        const unsigned __log2_x = __log2_Rk - __d; // # of spare entropy
> bits.
>  #else
> @@ -3586,25 +3585,25 @@ namespace __detail
>      _RealT
>      __generate_canonical_any(_Urbg& __urng)
>      {
> -      static_assert(__d < __builtin_popcountg(~_UInt(0)));
>        // Names below are chosen to match the description in the Standard.
>        // Parameter d is the actual target number of bits.
> -      const _UInt __r = 2;
> +      const _UInt __r{2};
>        const _UInt __R = _UInt(_Urbg::max() - _Urbg::min()) + 1;
>        const _UInt __rd = _UInt(1) << __d;
>        const unsigned __k = __gen_can_rng_calls_needed(__R, __rd);
>        const _UInt __Rk = __gen_can_pow(__R, __k);
> -      const _UInt __x =  __Rk/__rd;
> +      const _UInt __x =  __Rk / __rd;
>
>        while (true)
>         {
> -         _UInt __Ri = 1, __sum = __urng() - _Urbg::min();
> +         _UInt __Ri{1};
> +         _UInt __sum{__urng() - _Urbg::min()};
>           for (int __i = __k - 1; __i > 0; --__i)
>             {
>               __Ri *= __R;
> -             __sum += (__urng() - _Urbg::min()) * __Ri;
> +             __sum += _UInt{__urng() - _Urbg::min()} * __Ri;
>             }
> -         const _RealT __ret = _RealT(__sum / __x) / __rd;
> +         const _RealT __ret = _RealT(__sum / __x) / _RealT(__rd);
>           if (__ret < _RealT(1.0))
>             return __ret;
>         }
> @@ -3660,7 +3659,6 @@ _GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
>         "float16_t type is not supported, consider using bfloat16_t");
>  #endif
>
> -      using _Rng = decltype(_Urbg::max());
>        const unsigned __d_max = std::numeric_limits<_RealT>::digits;
>        const unsigned __d = __digits > __d_max ? __d_max : __digits;
>
> @@ -3668,38 +3666,13 @@ _GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
>        // is enough bits. If not, we need more.
>        if constexpr (__is_power_of_2_less_1(_Urbg::max() - _Urbg::min()))
>         {
> -         if constexpr (__d <= 32)
> -           return __generate_canonical_pow2<_RealT, uint32_t,
> __d>(__urng);
> -         else if constexpr (__d <= 64)
> -           return __generate_canonical_pow2<_RealT, uint64_t,
> __d>(__urng);
> -         else
> -           {
> -#if defined(__SIZEOF_INT128__)
> -             // Accommodate double double or float128.
> -             return __extension__ __generate_canonical_pow2<
> -               _RealT, unsigned __int128, __d>(__urng);
> -#else
> -             static_assert(false,
> -               "float precision >64 requires __int128 support");
> -#endif
> -           }
> +         using type = typename __detail::_Select_uint_least_t<__d>::type;
> +         return __generate_canonical_pow2<_RealT, type, __d>(__urng);
>         }
>        else // Need up to 2x bits.
>         {
> -         if constexpr (__d <= 32)
> -           return __generate_canonical_any<_RealT, uint64_t, __d>(__urng);
> -         else
> -           {
> -#if defined(__SIZEOF_INT128__)
> -             static_assert(__d <= 64,
> -               "irregular RNG with float precision >64 is not supported");
> -             return __extension__ __generate_canonical_any<
> -               _RealT, unsigned __int128, __d>(__urng);
> -#else
> -             static_assert(false, "irregular RNG with float precision"
> -                " >32 requires __int128 support");
> -#endif
> -           }
> +         using type = typename __detail::_Select_uint_least_t<__d *
> 2>::type;
>
There are two problems here:
a) for any system where long double has mantissa larger than 64 (i.e. uses
some version of __float128)
this will be ill-formed, as we do not provide integers longer than 128
(regardless of emulation)
b) for some combinations of the URGB and _FloatT, R^k would still fit in
32,64 bits even
   if __float128 is available.
Doing b) help to partially address the problem a, as assuming URBG
producing less <= 64bits
on entropy at some time, we may be able to fit between long double digits,
and 128.
I will update my patch that I posted before to be based on that (I think I
can implement it a lot nicer).
But this is an improvement, so LGTM.


> +         return __generate_canonical_any<_RealT, type, __d>(__urng);
>         }
>      }
>  _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
> --
> 2.52.0
>
>

Reply via email to