From 107b24a43f484d43c8ee7e020f315337ff33da7a Mon Sep 17 00:00:00 2001 From: Matt Stephanson Date: Thu, 20 Jan 2022 22:24:19 -0800 Subject: [PATCH 1/6] constexpr all the generate_canonical paramters ...up to 64 bits of entropy. Leave the original implementation for more bits and naughty generators (I'm looking at you, tr1) whose min and max functions aren't static. Fixes #1964. Alternative to #2452. --- stl/inc/random | 76 +++++++++++++++++-- .../env.lst | 4 + .../test.cpp | 58 ++++++++++++++ 3 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst create mode 100644 tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp diff --git a/stl/inc/random b/stl/inc/random index 4d684d36c1f..37b887ebc7d 100644 --- a/stl/inc/random +++ b/stl/inc/random @@ -238,19 +238,48 @@ private: vector _Myvec; }; +constexpr int _Generate_canonical_iterations(const int _Bits, const uint64_t _Gmin, const uint64_t _Gmax) { + // For a URBG type `G` with range == `(G::max() - G::min()) + 1`, returns the number of calls to generate at least + // _Bits bits of entropy. Specifically, max(1, ceil(_Bits / log2(range))). + + int _Ceil = 0; + if (_Bits == 0) { + ; + } else if (_Gmax == UINT64_MAX && _Gmin == 0) { + _Ceil = ((_Bits - 1) / 64) + 1; + } else { + const auto _Range = (_Gmax - _Gmin) + 1; + if ((_Range & (_Range - 1)) == 0) { + _Ceil = (_Bits - 1) / _Countr_zero(_Range) + 1; + } else { + const auto _Target = ~uint64_t{0} >> (64 - _Bits); + uint64_t _Prod = 1; + while (_Prod <= _Target) { + if (_Prod > UINT64_MAX / _Range) { + ++_Ceil; + break; + } + _Prod *= _Range; + ++_Ceil; + } + } + } + + return _Ceil < 1 ? 1 : _Ceil; +} + template _NODISCARD _Real generate_canonical(_Gen& _Gx) { // build a floating-point value from random sequence _RNG_REQUIRE_REALTYPE(generate_canonical, _Real); - const size_t _Digits = static_cast(numeric_limits<_Real>::digits); - const size_t _Minbits = _Digits < _Bits ? _Digits : _Bits; + constexpr auto _Digits = static_cast(numeric_limits<_Real>::digits); + constexpr auto _Minbits = static_cast(_Digits < _Bits ? _Digits : _Bits); - const _Real _Gxmin = static_cast<_Real>((_Gx.min)()); - const _Real _Gxmax = static_cast<_Real>((_Gx.max)()); - const _Real _Rx = (_Gxmax - _Gxmin) + _Real{1}; + constexpr auto _Gxmin = static_cast<_Real>((_Gen::min)()); + constexpr auto _Gxmax = static_cast<_Real>((_Gen::max)()); + constexpr auto _Rx = (_Gxmax - _Gxmin) + _Real{1}; - const int _Ceil = static_cast(_STD ceil(static_cast<_Real>(_Minbits) / _STD log2(_Rx))); - const int _Kx = _Ceil < 1 ? 1 : _Ceil; + constexpr int _Kx = _Generate_canonical_iterations(_Minbits, (_Gen::min)(), (_Gen::max)()); _Real _Ans{0}; _Real _Factor{1}; @@ -263,7 +292,38 @@ _NODISCARD _Real generate_canonical(_Gen& _Gx) { // build a floating-point value return _Ans / _Factor; } -#define _NRAND(eng, resty) (_STD generate_canonical(-1)>(eng)) +template +_NODISCARD _Real _Nrand_impl(_Gen& _Gx) { // build a floating-point value from random sequence + _RNG_REQUIRE_REALTYPE(_Nrand_impl, _Real); + + constexpr auto _Digits = static_cast(numeric_limits<_Real>::digits); + constexpr auto _Bits = ~size_t{0}; + constexpr auto _Minbits = _Digits < _Bits ? _Digits : _Bits; + + if constexpr (_Select_invoke_traits::_Is_invocable::value + && _Select_invoke_traits::_Is_invocable::value && _Minbits <= 64) { + return _STD generate_canonical<_Real, _Minbits>(_Gx); + } else { + const _Real _Gxmin = static_cast<_Real>((_Gx.min)()); + const _Real _Gxmax = static_cast<_Real>((_Gx.max)()); + const _Real _Rx = (_Gxmax - _Gxmin) + _Real{1}; + + const int _Ceil = static_cast(_STD ceil(static_cast<_Real>(_Minbits) / _STD log2(_Rx))); + const int _Kx = _Ceil < 1 ? 1 : _Ceil; + + _Real _Ans{0}; + _Real _Factor{1}; + + for (int _Idx = 0; _Idx < _Kx; ++_Idx) { // add in another set of bits + _Ans += (static_cast<_Real>(_Gx()) - _Gxmin) * _Factor; + _Factor *= _Rx; + } + + return _Ans / _Factor; + } +} + +#define _NRAND(eng, resty) (_Nrand_impl(eng)) _INLINE_VAR constexpr int _MP_len = 5; using _MP_arr = uint64_t[_MP_len]; diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst b/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst new file mode 100644 index 00000000000..642f530ffad --- /dev/null +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_matrix.lst diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp new file mode 100644 index 00000000000..e15d66a5188 --- /dev/null +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -0,0 +1,58 @@ +#include +#include +#include +#include +#include + +using namespace std; + +int naive_iterations(const int bits, const size_t gmin, const size_t gmax) { + // Naive implementation of [rand.util.canonical]. Note that for large values of range, it's possible that + // log2(range) == bits when range < 2^bits. This can lead to incorrect results, so we can't use this function as + // a reference for all values. + + const double range = static_cast(gmax) - static_cast(gmin) + 1.0; + return static_cast(ceil(static_cast(bits) / log2(static_cast(range)))); +} + +bool test(const int target_bits) { + // Increase the range until the number of iterations repeats. + size_t range = 2; + int k = 0; + int prev_k = -1; + while (k != prev_k) { + prev_k = exchange(k, naive_iterations(target_bits, 1, range)); + const int k1 = _Generate_canonical_iterations(target_bits, 1, range); + assert(k == k1); + ++range; + } + + // Now only check the crossover points, where incrementing the range actually causes the number of iterations to + // increase. + --k; + for (; k > 0; --k) { + // The largest range such that k iterations generating [1,range] produces less than target_bits bits. + if (k == 1) { + range = ~size_t{0} >> (64 - target_bits); + } else { + range = static_cast(ceil(pow(2.0, static_cast(target_bits) / k))) - 1; + } + + int k0 = (k == 1) ? 2 : naive_iterations(target_bits, 1, range); + int k1 = _Generate_canonical_iterations(target_bits, 1, range); + assert(k0 == k1 && k1 == k + 1); + + k0 = (k == 1) ? 1 : naive_iterations(target_bits, 0, range); + k1 = _Generate_canonical_iterations(target_bits, 0, range); + assert(k0 == k1 && k1 == k); + } +} + +int main() { + static_assert(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2); + static_assert(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1); + + for (int bits = 1; bits <= 64; ++bits) { + test(bits); + } +} From bc48cced6d554d85f2dd8fc760ae4c23a4d4f57e Mon Sep 17 00:00:00 2001 From: Matt Stephanson Date: Fri, 21 Jan 2022 09:51:38 -0800 Subject: [PATCH 2/6] fix test --- .../GH_001964_constexpr_generate_canonical/test.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp index e15d66a5188..526903b9170 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -6,7 +6,7 @@ using namespace std; -int naive_iterations(const int bits, const size_t gmin, const size_t gmax) { +int naive_iterations(const int bits, const uint64_t gmin, const uint64_t gmax) { // Naive implementation of [rand.util.canonical]. Note that for large values of range, it's possible that // log2(range) == bits when range < 2^bits. This can lead to incorrect results, so we can't use this function as // a reference for all values. @@ -15,9 +15,9 @@ int naive_iterations(const int bits, const size_t gmin, const size_t gmax) { return static_cast(ceil(static_cast(bits) / log2(static_cast(range)))); } -bool test(const int target_bits) { +void test(const int target_bits) { // Increase the range until the number of iterations repeats. - size_t range = 2; + uint64_t range = 2; int k = 0; int prev_k = -1; while (k != prev_k) { @@ -33,9 +33,9 @@ bool test(const int target_bits) { for (; k > 0; --k) { // The largest range such that k iterations generating [1,range] produces less than target_bits bits. if (k == 1) { - range = ~size_t{0} >> (64 - target_bits); + range = ~uint64_t{0} >> (64 - target_bits); } else { - range = static_cast(ceil(pow(2.0, static_cast(target_bits) / k))) - 1; + range = static_cast(ceil(pow(2.0, static_cast(target_bits) / k))) - 1; } int k0 = (k == 1) ? 2 : naive_iterations(target_bits, 1, range); From 45239609e1e15e2bba7691f3dd6e6692521a1978 Mon Sep 17 00:00:00 2001 From: Matt Stephanson Date: Fri, 21 Jan 2022 09:53:32 -0800 Subject: [PATCH 3/6] clang-format --- .../std/tests/GH_001964_constexpr_generate_canonical/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp index 526903b9170..0d15da475da 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -18,8 +18,8 @@ int naive_iterations(const int bits, const uint64_t gmin, const uint64_t gmax) { void test(const int target_bits) { // Increase the range until the number of iterations repeats. uint64_t range = 2; - int k = 0; - int prev_k = -1; + int k = 0; + int prev_k = -1; while (k != prev_k) { prev_k = exchange(k, naive_iterations(target_bits, 1, range)); const int k1 = _Generate_canonical_iterations(target_bits, 1, range); From fb630e3931f1a8ffde868b7d4f4b9de64942cc8f Mon Sep 17 00:00:00 2001 From: Matt Stephanson Date: Mon, 24 Jan 2022 10:23:02 -0800 Subject: [PATCH 4/6] code review feedback --- stl/inc/random | 38 +++++++++---------- .../env.lst | 2 +- .../test.cpp | 7 ++-- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/stl/inc/random b/stl/inc/random index 37b887ebc7d..771d66a0189 100644 --- a/stl/inc/random +++ b/stl/inc/random @@ -242,30 +242,30 @@ constexpr int _Generate_canonical_iterations(const int _Bits, const uint64_t _Gm // For a URBG type `G` with range == `(G::max() - G::min()) + 1`, returns the number of calls to generate at least // _Bits bits of entropy. Specifically, max(1, ceil(_Bits / log2(range))). - int _Ceil = 0; + _STL_INTERNAL_CHECK(0 <= _Bits && _Bits <= 64); + if (_Bits == 0) { - ; - } else if (_Gmax == UINT64_MAX && _Gmin == 0) { - _Ceil = ((_Bits - 1) / 64) + 1; + return 1; + } + + const auto _Range = (_Gmax - _Gmin) + 1; + if ((_Range & (_Range - 1)) == 0) { + return (_Bits - 1) / _Countr_zero(_Range) + 1; } else { - const auto _Range = (_Gmax - _Gmin) + 1; - if ((_Range & (_Range - 1)) == 0) { - _Ceil = (_Bits - 1) / _Countr_zero(_Range) + 1; - } else { - const auto _Target = ~uint64_t{0} >> (64 - _Bits); - uint64_t _Prod = 1; - while (_Prod <= _Target) { - if (_Prod > UINT64_MAX / _Range) { - ++_Ceil; - break; - } - _Prod *= _Range; - ++_Ceil; + const auto _Target = ~uint64_t{0} >> (64 - _Bits); + uint64_t _Prod = 1; + int _Ceil = 0; + while (_Prod <= _Target) { + ++_Ceil; + if (_Prod > UINT64_MAX / _Range) { + break; } + + _Prod *= _Range; } - } - return _Ceil < 1 ? 1 : _Ceil; + return _Ceil; + } } template diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst b/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst index 642f530ffad..19f025bd0e6 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_latest_matrix.lst +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp index 0d15da475da..1638272eccf 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -29,8 +29,7 @@ void test(const int target_bits) { // Now only check the crossover points, where incrementing the range actually causes the number of iterations to // increase. - --k; - for (; k > 0; --k) { + while (--k) { // The largest range such that k iterations generating [1,range] produces less than target_bits bits. if (k == 1) { range = ~uint64_t{0} >> (64 - target_bits); @@ -49,8 +48,8 @@ void test(const int target_bits) { } int main() { - static_assert(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2); - static_assert(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1); + static_assert(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2, ""); + static_assert(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1, ""); for (int bits = 1; bits <= 64; ++bits) { test(bits); From 0386512d730789f1913dfae94f88975d6023a8d3 Mon Sep 17 00:00:00 2001 From: Matt Stephanson Date: Mon, 31 Jan 2022 20:09:00 -0800 Subject: [PATCH 5/6] remove non-constexpr call, expand testing to 0 bits --- stl/inc/random | 28 ++++++++----------- .../test.cpp | 4 +-- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/stl/inc/random b/stl/inc/random index 771d66a0189..ed7a76a3389 100644 --- a/stl/inc/random +++ b/stl/inc/random @@ -244,28 +244,24 @@ constexpr int _Generate_canonical_iterations(const int _Bits, const uint64_t _Gm _STL_INTERNAL_CHECK(0 <= _Bits && _Bits <= 64); - if (_Bits == 0) { + if (_Bits == 0 || (_Gmax == UINT64_MAX && _Gmin == 0)) { return 1; } - const auto _Range = (_Gmax - _Gmin) + 1; - if ((_Range & (_Range - 1)) == 0) { - return (_Bits - 1) / _Countr_zero(_Range) + 1; - } else { - const auto _Target = ~uint64_t{0} >> (64 - _Bits); - uint64_t _Prod = 1; - int _Ceil = 0; - while (_Prod <= _Target) { - ++_Ceil; - if (_Prod > UINT64_MAX / _Range) { - break; - } - - _Prod *= _Range; + const auto _Range = (_Gmax - _Gmin) + 1; + const auto _Target = ~uint64_t{0} >> (64 - _Bits); + uint64_t _Prod = 1; + int _Ceil = 0; + while (_Prod <= _Target) { + ++_Ceil; + if (_Prod > UINT64_MAX / _Range) { + break; } - return _Ceil; + _Prod *= _Range; } + + return _Ceil; } template diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp index 1638272eccf..ce8b9248bab 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -12,7 +12,7 @@ int naive_iterations(const int bits, const uint64_t gmin, const uint64_t gmax) { // a reference for all values. const double range = static_cast(gmax) - static_cast(gmin) + 1.0; - return static_cast(ceil(static_cast(bits) / log2(static_cast(range)))); + return max(1, static_cast(ceil(static_cast(bits) / log2(static_cast(range))))); } void test(const int target_bits) { @@ -51,7 +51,7 @@ int main() { static_assert(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2, ""); static_assert(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1, ""); - for (int bits = 1; bits <= 64; ++bits) { + for (int bits = 0; bits <= 64; ++bits) { test(bits); } } From 2aed18f2195a6714de626baa5238de0c96810a38 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 3 Feb 2022 18:01:17 -0800 Subject: [PATCH 6/6] Code review feedback. --- stl/inc/random | 15 +++++++++++---- tests/std/test.lst | 1 + .../test.cpp | 19 +++++++++++++------ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/stl/inc/random b/stl/inc/random index ed7a76a3389..b832a9bc193 100644 --- a/stl/inc/random +++ b/stl/inc/random @@ -238,7 +238,7 @@ private: vector _Myvec; }; -constexpr int _Generate_canonical_iterations(const int _Bits, const uint64_t _Gmin, const uint64_t _Gmax) { +_NODISCARD constexpr int _Generate_canonical_iterations(const int _Bits, const uint64_t _Gmin, const uint64_t _Gmax) { // For a URBG type `G` with range == `(G::max() - G::min()) + 1`, returns the number of calls to generate at least // _Bits bits of entropy. Specifically, max(1, ceil(_Bits / log2(range))). @@ -288,6 +288,14 @@ _NODISCARD _Real generate_canonical(_Gen& _Gx) { // build a floating-point value return _Ans / _Factor; } +template +struct _Has_static_min_max : false_type {}; + +// This checks a requirement of N4901 [rand.req.urng] `concept uniform_random_bit_generator` but doesn't attempt +// to implement the whole concept - we just need to distinguish Standard machinery from tr1 machinery. +template +struct _Has_static_min_max<_Gen, void_t::value)>> : true_type {}; + template _NODISCARD _Real _Nrand_impl(_Gen& _Gx) { // build a floating-point value from random sequence _RNG_REQUIRE_REALTYPE(_Nrand_impl, _Real); @@ -296,10 +304,9 @@ _NODISCARD _Real _Nrand_impl(_Gen& _Gx) { // build a floating-point value from r constexpr auto _Bits = ~size_t{0}; constexpr auto _Minbits = _Digits < _Bits ? _Digits : _Bits; - if constexpr (_Select_invoke_traits::_Is_invocable::value - && _Select_invoke_traits::_Is_invocable::value && _Minbits <= 64) { + if constexpr (_Has_static_min_max<_Gen>::value && _Minbits <= 64) { return _STD generate_canonical<_Real, _Minbits>(_Gx); - } else { + } else { // TRANSITION, for tr1 machinery only; Standard machinery can call generate_canonical directly const _Real _Gxmin = static_cast<_Real>((_Gx.min)()); const _Real _Gxmax = static_cast<_Real>((_Gx.max)()); const _Real _Rx = (_Gxmax - _Gxmin) + _Real{1}; diff --git a/tests/std/test.lst b/tests/std/test.lst index 9296abdddd9..152e1e4ff00 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -186,6 +186,7 @@ tests\GH_001638_dllexport_derived_classes tests\GH_001850_clog_tied_to_cout tests\GH_001858_iostream_exception tests\GH_001914_cached_position +tests\GH_001964_constexpr_generate_canonical tests\GH_002030_asan_annotate_vector tests\GH_002039_byte_is_not_trivially_swappable tests\GH_002058_debug_iterator_race diff --git a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp index ce8b9248bab..b8d40cb9f11 100644 --- a/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp +++ b/tests/std/tests/GH_001964_constexpr_generate_canonical/test.cpp @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + #include #include #include @@ -6,13 +9,15 @@ using namespace std; +#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__) + int naive_iterations(const int bits, const uint64_t gmin, const uint64_t gmax) { // Naive implementation of [rand.util.canonical]. Note that for large values of range, it's possible that // log2(range) == bits when range < 2^bits. This can lead to incorrect results, so we can't use this function as // a reference for all values. const double range = static_cast(gmax) - static_cast(gmin) + 1.0; - return max(1, static_cast(ceil(static_cast(bits) / log2(static_cast(range))))); + return max(1, static_cast(ceil(static_cast(bits) / log2(range)))); } void test(const int target_bits) { @@ -29,7 +34,7 @@ void test(const int target_bits) { // Now only check the crossover points, where incrementing the range actually causes the number of iterations to // increase. - while (--k) { + while (--k != 0) { // The largest range such that k iterations generating [1,range] produces less than target_bits bits. if (k == 1) { range = ~uint64_t{0} >> (64 - target_bits); @@ -39,17 +44,19 @@ void test(const int target_bits) { int k0 = (k == 1) ? 2 : naive_iterations(target_bits, 1, range); int k1 = _Generate_canonical_iterations(target_bits, 1, range); - assert(k0 == k1 && k1 == k + 1); + assert(k0 == k1); + assert(k1 == k + 1); k0 = (k == 1) ? 1 : naive_iterations(target_bits, 0, range); k1 = _Generate_canonical_iterations(target_bits, 0, range); - assert(k0 == k1 && k1 == k); + assert(k0 == k1); + assert(k1 == k); } } int main() { - static_assert(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2, ""); - static_assert(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1, ""); + STATIC_ASSERT(_Generate_canonical_iterations(53, 1, uint64_t{1} << 32) == 2); + STATIC_ASSERT(_Generate_canonical_iterations(64, 0, ~uint64_t{0}) == 1); for (int bits = 0; bits <= 64; ++bits) { test(bits);