[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX339969: [libc++] Use correct rand.eng.mers all-zeroes seed
sequence fallback (authored by hubert.reinterpretcast, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50736?vs=160923=161145#toc
Repository:
rCXX libc++
https://reviews.llvm.org/D50736
Files:
include/random
test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
@@ -2363,7 +2363,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
Index: test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
===
--- test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
+++ test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
@@ -0,0 +1,81 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// class mersenne_twister_engine;
+
+// template explicit mersenne_twister_engine(Sseq );
+//
+// [ ... ] Finally, if the most significant $w-r$ bits of $X_{-n}$ are zero,
+// and if each of the other resulting $X_i$ is $0$, changes $X_{-n}$ to
+// $ 2^{w-1} $.
+
+#include
+
+#include
+#include
+#include
+#if TEST_STD_VER >= 11
+#include
+#endif
+
+struct all_zero_seed_seq {
+ typedef unsigned int result_type;
+
+ all_zero_seed_seq() {}
+
+ template
+ all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+ all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+ template
+ void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+ }
+
+ std::size_t size() const { return 0u; }
+ template void param(OutputIterator) const {}
+};
+
+template
+void test(void) {
+ const std::size_t state_size = 1u;
+ const std::size_t shift_size = 1u;
+ const std::size_t tempering_l = word_size;
+
+ all_zero_seed_seq q;
+ std::mersenne_twister_engine
+ e(q);
+
+ const result_type Xneg1 = result_type(1) << (word_size - 1);
+ const result_type Y = Xneg1;
+ const result_type X0 = Xneg1 ^ (Y >> 1);
+ assert(e() == X0);
+}
+
+int main() {
+ // Test for k == 1: word_size <= 32.
+ test();
+
+ // Test for k == 2: (32 < word_size <= 64).
+ test();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
I can commit sometime today; thanks. -- HT On Thu, Aug 16, 2018 at 1:24 PM, Marshall Clow via Phabricator < revi...@reviews.llvm.org> wrote: > mclow.lists accepted this revision. > mclow.lists added a comment. > This revision is now accepted and ready to land. > > This LGTM. Do you want me to commit it for you? > > > Repository: > rCXX libc++ > > https://reviews.llvm.org/D50736 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This LGTM. Do you want me to commit it for you? Repository: rCXX libc++ https://reviews.llvm.org/D50736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
mclow.lists added a comment. Thanks for doing this. This looks good to me; but I want to play with it a bit before committing. I'll do that tonight. Repository: rCXX libc++ https://reviews.llvm.org/D50736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
hubert.reinterpretcast updated this revision to Diff 160923.
hubert.reinterpretcast added a comment.
Address review comments by Marshall
Add a line break after the template-head. Add a comment with the requested
quote from the Standard. Move test from the `test/libcxx/` tree to the
`test/std/` tree, and guard against inclusion of `` under
C++03.
Repository:
rCXX libc++
https://reviews.llvm.org/D50736
Files:
include/random
test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
Index: test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
===
--- /dev/null
+++ test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
@@ -0,0 +1,81 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// class mersenne_twister_engine;
+
+// template explicit mersenne_twister_engine(Sseq );
+//
+// [ ... ] Finally, if the most significant $w-r$ bits of $X_{-n}$ are zero,
+// and if each of the other resulting $X_i$ is $0$, changes $X_{-n}$ to
+// $ 2^{w-1} $.
+
+#include
+
+#include
+#include
+#include
+#if TEST_STD_VER >= 11
+#include
+#endif
+
+struct all_zero_seed_seq {
+ typedef unsigned int result_type;
+
+ all_zero_seed_seq() {}
+
+ template
+ all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+ all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+ template
+ void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+ }
+
+ std::size_t size() const { return 0u; }
+ template void param(OutputIterator) const {}
+};
+
+template
+void test(void) {
+ const std::size_t state_size = 1u;
+ const std::size_t shift_size = 1u;
+ const std::size_t tempering_l = word_size;
+
+ all_zero_seed_seq q;
+ std::mersenne_twister_engine
+ e(q);
+
+ const result_type Xneg1 = result_type(1) << (word_size - 1);
+ const result_type Y = Xneg1;
+ const result_type X0 = Xneg1 ^ (Y >> 1);
+ assert(e() == X0);
+}
+
+int main() {
+ // Test for k == 1: word_size <= 32.
+ test();
+
+ // Test for k == 2: (32 < word_size <= 64).
+ test();
+}
Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
@@ -2363,7 +2363,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
hubert.reinterpretcast added a comment. In https://reviews.llvm.org/D50736#1200774, @hubert.reinterpretcast wrote: > In https://reviews.llvm.org/D50736#1200761, @mclow.lists wrote: > > > Is this test that's being added libc++ specific, or would it apply to other > > implementations as well? > > > The test can apply to other implementations as well (although I am not sure > how the `initializer_list` include behaves under C++03 on other > implementations). Is there some other place to put such tests? It seems that `test/std/numerics/rand/rand.eng/rand.eng.mers/` may be the place to add the test. I am looking into it. Comment at: test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:18 + +// template explicit mersenne_twister_engine(Sseq ); + mclow.lists wrote: > Please add a comment here about what's being tested, because when I look at > this a year from now, I won't know what's going on. > > Something like: > `// Finally, if the most significant w − r bits of X−n are zero, and if each > of the other resulting Xi is 0, changes X−n to 2w−1.` > I will add a comment to this effect. In an effort to be clear in the formatting, I will use the LaTeX snippet for the text. Repository: rCXX libc++ https://reviews.llvm.org/D50736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
hubert.reinterpretcast added a comment.
In https://reviews.llvm.org/D50736#1200761, @mclow.lists wrote:
> Is this test that's being added libc++ specific, or would it apply to other
> implementations as well?
The test can apply to other implementations as well (although I am not sure how
the `initializer_list` include behaves under C++03 on other implementations).
Is there some other place to put such tests?
Comment at:
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:47
+
+template void test(void) {
+ const std::size_t state_size = 1u;
mclow.lists wrote:
> Need a line break before `void`
I have no objection to adding a line break, but I would like to clarify that
this line came straight out of a recent build of `clang-format` with
`--style=llvm`. I would like to know whether there is something else here that
I should be doing with `clang-format`, including if it is deemed that
`clang-format` does not do the right thing here.
Repository:
rCXX libc++
https://reviews.llvm.org/D50736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
mclow.lists added a comment.
Is this test that's being added libc++ specific, or would it apply to other
implementations as well?
Comment at:
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:18
+
+// template explicit mersenne_twister_engine(Sseq );
+
Please add a comment here about what's being tested, because when I look at
this a year from now, I won't know what's going on.
Something like:
`// Finally, if the most significant w − r bits of X−n are zero, and if each
of the other resulting Xi is 0, changes X−n to 2w−1.`
Comment at:
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:47
+
+template void test(void) {
+ const std::size_t state_size = 1u;
Need a line break before `void`
Repository:
rCXX libc++
https://reviews.llvm.org/D50736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: mclow.lists, EricWF, jasonliu.
Herald added subscribers: ldionne, christof.
When a seed sequence would lead to having no non-zero significant bits in the
initial state of a `mersenne_twister_engine`, the fallback is to flip the most
significant bit of the first value that appears in the textual representation
of the initial state.
rand.eng.mers describes this as setting the value to be 2 to the power of one
less than w; the previous value encoded in the implementation, namely one less
than "2 to the power of w", is replaced by the correct value in this patch.
Repository:
rCXX libc++
https://reviews.llvm.org/D50736
Files:
include/random
test/libcxx/numerics/rand/rand.eng.mers/
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp
Index: test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp
===
--- /dev/null
+++ test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp
@@ -0,0 +1,74 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// class mersenne_twister_engine;
+
+// template explicit mersenne_twister_engine(Sseq );
+
+#include
+
+#include
+#include
+#include
+#include
+
+struct all_zero_seed_seq {
+ typedef unsigned int result_type;
+
+ all_zero_seed_seq() {}
+
+ template
+ all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+ all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+ template
+ void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+ }
+
+ std::size_t size() const { return 0u; }
+ template void param(OutputIterator) const {}
+};
+
+template void test(void) {
+ const std::size_t state_size = 1u;
+ const std::size_t shift_size = 1u;
+ const std::size_t tempering_l = word_size;
+
+ all_zero_seed_seq q;
+ std::mersenne_twister_engine
+ e(q);
+
+ const result_type Xneg1 = result_type(1) << (word_size - 1);
+ const result_type Y = Xneg1;
+ const result_type X0 = Xneg1 ^ (Y >> 1);
+ assert(e() == X0);
+}
+
+int main() {
+ // Test for k == 1: word_size <= 32.
+ test();
+
+ // Test for k == 2: (32 < word_size <= 64).
+ test();
+}
Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
@@ -2363,7 +2363,7 @@
for (size_t __i = 1; __i < __n; ++__i)
if (__x_[__i] != 0)
return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
