On Wed, 17 Dec 2025 at 20:32, Tomasz Kaminski <[email protected]> wrote:
>
>
>
> 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)

Yeah, this only solves the problem for some targets (e.g. i686 and arm).

> 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