refactor generate_canonical #2452
Conversation
... to calculate parameters at compile-time for random engines with power-of-two range. This greatly improves debug codegen, and release codegen for compilers that do not constant fold `std::ceil` and/or `std::log2`. Fixes microsoft#1964.
| @@ -238,32 +238,60 @@ private: | |||
| vector<result_type> _Myvec; | |||
| }; | |||
|
|
|||
| template <class _Real, size_t _Minbits, class _Result, _Result _Ix> | |||
| _NODISCARD constexpr int _Generate_canonical_helper() { | |||
There was a problem hiding this comment.
"Helper" names aren't actually helpful. Maybe _Generate_canonical_times_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; |
There was a problem hiding this comment.
This repeats the above block, can it be fused?
| _RNG_REQUIRE_REALTYPE(generate_canonical, _Real); | ||
|
|
||
| const size_t _Digits = static_cast<size_t>(numeric_limits<_Real>::digits); | ||
| const size_t _Minbits = _Digits < _Bits ? _Digits : _Bits; | ||
| constexpr auto _Digits = static_cast<size_t>(numeric_limits<_Real>::digits); |
There was a problem hiding this comment.
(Pre existing) convert with {} as you changed _NRAND to do so.
|
A while ago I was playing with making this |
Yes, please! Note that |
This comment has been minimized.
This comment has been minimized.
Conflict with formatting changes on main in `<random>` - pick mine and reformat.
...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 microsoft#1964. Alternative to microsoft#2452.
|
Abandoning in favor of #2498. |
... to calculate parameters at compile-time for random engines with power-of-two range. This greatly improves debug codegen, and release codegen for compilers that do not constant fold
std::ceiland/orstd::log2.Fixes #1964.
Release codegen diff for:
is
assembly diff
Note the predominance of red at the end of the diff (it's 535 lines before and 200 after). If people aren't convinced by the assembly diff alone, shout and I'll benchmark something.