<random>
: Use _Unsigned128
for linear_congruential_engine
#5436
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that we had a single remaining usage of separately compiled multiprecision arithmetic machinery. I believe that we can replace this with the modern
_Unsigned128
type, which is far better tested and more widely exercised. As @AlexGuteniev observed,_Unsigned128
uses intrinsics, so it should also be faster than multprec.cpp. And as a final bonus, it makes this codepath strongly resemble the 32-bit and 64-bit codepaths above.👨🔬 Proof that 128 bits are sufficient
The function comment says:
STL/stl/inc/random
Lines 474 to 482 in b5df16a
The first part, _Cx <= (2^128 - 1), is obviously true.
The next part is (_Mx - 1) <= ((2^128 - 1) - _Cx) / _Ax. This is the hardest to achieve for the largest LHS (caused by the largest _Mx) and the smallest RHS (caused by the largest _Ax and the largest _Cx). So ((2^64 - 1) - 1) <= ((2^128 - 1) - (2^64 - 1)) / (2^64 - 1) is the worst case.
⚙️ Commits
_Uint _Cx
always satisfies_Cx <= ULLONG_MAX
._Unsigned128
, preserve multprec.cpp for bincompat._INLINE_VAR
from_MP_len
, giving it internal linkage within multprec.cpp.extern "C++"
on the declarations - that was for modules._CRTIMP2_PURE
to the definitions. I thought we had harmonized all declarations and definitions, but we missed these.64
instead of including<limits>
fornumeric_limits<unsigned long long>::digits
.uint64_t
andunsigned long long
are synonyms. In fact, this file was already usingunsigned long long
for the definitions, even though the declarations originally saiduint64_t
.<random>
so we don't need it anymore. All we need is<yvals.h>
.✅ Notes
static_assert(false)
) that this codepath is exercised by our test suites.dumpbin /exports
) that this doesn't affect the dllexport surface.