
This fixes two major mistakes in the implementation of `linear_congruential_engine` that allowed it to produce incorrect output. Specifically, these mistakes are in `__lce_alg_picker`, which is used to determine whether Schrage's algorithm is valid and needed. The first mistake is in the definition of `_OverflowOK`. The code comment and the description of [D65041](https://reviews.llvm.org/D65041) both indicate that it's supposed to be true iff `m` is a power of two. However, the definition used does not work out to that, and instead is true whenever `m` is even. This could result in `linear_congruential_engine` using an invalid implementation, as it would incorrectly assume that any integer overflow can't change the result. I changed the implementation to one that accurately checks if `m` is a power of two. Technically, this implementation has an edge case where it considers `0` to be a power of two, but in this case this is actually accurate behavior, as `m = 0` indicates a modulus of 2^w where w is the size of `result_type` in bits, which *is* a power of two. The second mistake is in the static assert. The original static assert erroneously included an unnecessary `a != 0 || m != 0`. Combined with the `|| !_MightOverflow`, this actually resulted in the static assert being impossible to fail. Applying De Morgan's law and expanding `_MightOverflow` gives that the only way this static assert can be triggered is if `a == 0 && m == 0 && a != 0 && m != 0 && ...`, which clearly cannot be true. I simply removed the explicit checks against `a` and `m`, as the intended checks are already included in `_MightOverflow` and `_SchrageOK`, and their inclusion doesn't provide any obvious semantic benefit. This should fix all the current instances where `linear_congruential_engine` uses an invalid implementation. This technically isn't a complete implementation, though, since the static assert will cause some instantiations of `linear_congruential_engine` not disallowed by the standard from compiling. However, this should still be an improvement, as all compiling instantiations of `linear_congruential_engine` should use a valid implementation. Fixing the cases where the static assert triggers will require adding additional implementations, some of which will be fairly non-trivial, so I'd rather leave those for another PR so they don't hold up these more important fixes. Fixes #33554
84 lines
2.9 KiB
C++
84 lines
2.9 KiB
C++
//===----------------------------------------------------------------------===//
|
|
//
|
|
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
// See https://llvm.org/LICENSE.txt for license information.
|
|
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
// <random>
|
|
|
|
// template <class UIntType, UIntType a, UIntType c, UIntType m>
|
|
// class linear_congruential_engine;
|
|
|
|
// result_type operator()();
|
|
|
|
#include <random>
|
|
#include <cassert>
|
|
|
|
#include "test_macros.h"
|
|
|
|
int main(int, char**)
|
|
{
|
|
typedef unsigned long long T;
|
|
|
|
// m might overflow, but the overflow is OK so it shouldn't use Schrage's algorithm
|
|
typedef std::linear_congruential_engine<T, 25214903917ull, 1, (1ull << 48)> E1;
|
|
E1 e1;
|
|
// make sure the right algorithm was used
|
|
assert(e1() == 25214903918ull);
|
|
assert(e1() == 205774354444503ull);
|
|
assert(e1() == 158051849450892ull);
|
|
// make sure result is in bounds
|
|
assert(e1() < (1ull << 48));
|
|
assert(e1() < (1ull << 48));
|
|
assert(e1() < (1ull << 48));
|
|
assert(e1() < (1ull << 48));
|
|
assert(e1() < (1ull << 48));
|
|
|
|
// m might overflow. The overflow is not OK and result will be in bounds
|
|
// so we should use Schrage's algorithm
|
|
typedef std::linear_congruential_engine<T, (1ull << 32), 0, (1ull << 63) + 1> E2;
|
|
E2 e2;
|
|
// make sure Schrage's algorithm is used (it would be 0s after the first otherwise)
|
|
assert(e2() == (1ull << 32));
|
|
assert(e2() == (1ull << 63) - 1ull);
|
|
assert(e2() == (1ull << 63) - (1ull << 33) + 1ull);
|
|
// make sure result is in bounds
|
|
assert(e2() < (1ull << 63) + 1);
|
|
assert(e2() < (1ull << 63) + 1);
|
|
assert(e2() < (1ull << 63) + 1);
|
|
assert(e2() < (1ull << 63) + 1);
|
|
assert(e2() < (1ull << 63) + 1);
|
|
|
|
// m might overflow. The overflow is not OK and result will be in bounds
|
|
// so we should use Schrage's algorithm. m is even
|
|
typedef std::linear_congruential_engine<T, 0x18000001ull, 0x12347ull, (3ull << 56)> E3;
|
|
E3 e3;
|
|
// make sure Schrage's algorithm is used
|
|
assert(e3() == 402727752ull);
|
|
assert(e3() == 162159612030764687ull);
|
|
assert(e3() == 108176466184989142ull);
|
|
// make sure result is in bounds
|
|
assert(e3() < (3ull << 56));
|
|
assert(e3() < (3ull << 56));
|
|
assert(e3() < (3ull << 56));
|
|
assert(e3() < (3ull << 56));
|
|
assert(e3() < (3ull << 56));
|
|
|
|
// m will not overflow so we should not use Schrage's algorithm
|
|
typedef std::linear_congruential_engine<T, 1ull, 1, (1ull << 48)> E4;
|
|
E4 e4;
|
|
// make sure the correct algorithm was used
|
|
assert(e4() == 2ull);
|
|
assert(e4() == 3ull);
|
|
assert(e4() == 4ull);
|
|
// make sure result is in bounds
|
|
assert(e4() < (1ull << 48));
|
|
assert(e4() < (1ull << 48));
|
|
assert(e4() < (1ull << 48));
|
|
assert(e4() < (1ull << 48));
|
|
assert(e4() < (1ull << 48));
|
|
|
|
return 0;
|
|
} |