Message ID | alpine.BSF.2.20.16.2312292023350.28105@arjuna.pair.com |
---|---|
State | New |
Headers | show |
Series | libstdc++ testsuite/std/ranges/iota/max_size_type.cc: Reduce /10 for simulators | expand |
On Sat, 30 Dec 2023, 01:41 Hans-Peter Nilsson, <hp@bitrange.com> wrote: > I'm not completely sure I got the intent of the "log2_limit", > or whether "limit" is sane to decrease like this; it just > looked like an obvious and safe reduction. Also, I verified > the 10+ minute runtime, on this same host (clocked at 11:43.61 > elapsed time) for a r12-2797-g307e0d40367996 build that I > happened to have kept around; likely the build that led up > to that commit. Now it's 58:45.78 elapsed time for a > successful run. Looks like a 5x performance regression. > Worrisome; PR mentioned below. > > Incidentally, a parallel build and a serial test-run takes 9 > hours on that laptop, so that's almost 2 hours just for one > test, if just updating the timeout to fit. IOW, currently 48 > minutes out of 9 hours for one test that just times out. > > (That was just mentioned for comparison purposed: when suitable, > I test with `nprocs`-1 in parallel.) > > I'll put it on the back-burner to investigate. I think I'll > try to graft that version of libstdc++-v3 to this version > Unfortunately that will probably be difficult. and see if I can shift the blame away from MMIX code > generation onto libstdc++-v3. ;) > Or perhaps the cause is known? > Not to me. It probably is a target codegen bug, since all this test really does is emulate a wide integer type using masks and shifts. > With this, the test successfully completes in ~34 seconds. > > Ok to commit? > Looks OK to me, but Patrick wrote this test so please wait for him to confirm. I think this just reduces the number of cases tested, but doesn't miss any important edge cases that should be checked. > -- >8 -- > Looks like the MMIX port code quality and/or libstdc++ > performance of this test has regressed since > r12-2799-ge9b639c4b53221 by a factor 5. Anyway what was 11+ > minutes runtime then, is now at r14-6859-gd1eacedc6d9ba9 > close to 60 minutes. Better prune the test, not just > increase timeouts. Also of course, investigate the > performance regression, logged as PR113175. > > * testsuite/std/ranges/iota/max_size_type.cc: Adjust > limits from -1000..1000 to -100..100 for simulators. > --- > .../std/ranges/iota/max_size_type.cc | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc > b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc > index a1fbc3241dca..38fa6323d47e 100644 > --- a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc > +++ b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc > @@ -16,6 +16,7 @@ > // <http://www.gnu.org/licenses/>. > > // { dg-do run { target c++20 } } > +// { dg-additional-options "-DSIMULATOR_TEST" { target simulator } } > // { dg-timeout-factor 4 } > > #include <limits> > @@ -31,6 +32,14 @@ using signed_rep_t = __int128; > using signed_rep_t = long long; > #endif > > +#ifdef SIMULATOR_TEST > +#define LIMIT 100 > +#define LOG2_CEIL_LIMIT 7 > +#else > +#define LIMIT 1000 > +#define LOG2_CEIL_LIMIT 10 > +#endif > + > static_assert(sizeof(max_size_t) == sizeof(max_diff_t)); > static_assert(sizeof(rep_t) == sizeof(signed_rep_t)); > > @@ -199,8 +208,8 @@ test02() > using max_type = std::conditional_t<signed_p, max_diff_t, max_size_t>; > using shorten_type = std::conditional_t<shorten_p, hw_type, max_type>; > const int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__; > - const int limit = 1000; > - const int log2_limit = 10; > + const int limit = LIMIT; > + const int log2_limit = LOG2_CEIL_LIMIT; > static_assert((1 << log2_limit) >= limit); > const int min = (signed_p ? -limit : 0); > const int max = limit; > @@ -257,8 +266,8 @@ test03() > using max_type = std::conditional_t<signed_p, max_diff_t, max_size_t>; > using base_type = std::conditional_t<toggle_base_p, hw_type, max_type>; > constexpr int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__; > - constexpr int limit = 1000; > - constexpr int log2_limit = 10; > + constexpr int limit = LIMIT; > + constexpr int log2_limit = LOG2_CEIL_LIMIT; > static_assert((1 << log2_limit) >= limit); > const int min = (signed_p ? -limit : 0); > const int max = limit; > @@ -312,7 +321,7 @@ test03() > void > test04() > { > - constexpr int limit = 1000; > + constexpr int limit = LIMIT; > for (int i = -limit; i <= limit; i++) > { > VERIFY( -max_size_t(-i) == i ); > -- > 2.30.2 > >
On Sat, 30 Dec 2023, Jonathan Wakely wrote: > On Sat, 30 Dec 2023, 01:41 Hans-Peter Nilsson, <hp@bitrange.com> wrote: > > Or perhaps the cause is known? > > Not to me. It probably is a target codegen bug, since all this test really > does is emulate a wide integer type using masks and shifts. If so, a generic code-generator bug. I've repeated the 5x performance regression observation for a native build and updated PR113175 (.32 vs 1.73 seconds). I'll see if I can quickly find out whether it's codegen or libstdc++. I set it the PR to the latter for the moment. > > With this, the test successfully completes in ~34 seconds. > > > > Ok to commit? > > > > Looks OK to me, but Patrick wrote this test so please wait for him to > confirm. I think this just reduces the number of cases tested, but doesn't > miss any important edge cases that should be checked. Understood: holding, but will ping after the usual week. Thanks for the review! brgds, H-P
On Sat, 30 Dec 2023, Hans-Peter Nilsson wrote: > On Sat, 30 Dec 2023, Jonathan Wakely wrote: > > > On Sat, 30 Dec 2023, 01:41 Hans-Peter Nilsson, <hp@bitrange.com> wrote: > > > Or perhaps the cause is known? > > > > Not to me. It probably is a target codegen bug, since all this test really > > does is emulate a wide integer type using masks and shifts. > > If so, a generic code-generator bug. I've repeated the 5x > performance regression observation for a native build and > updated PR113175 (.32 vs 1.73 seconds). I'll see if I can > quickly find out whether it's codegen or libstdc++. I set it > the PR to the latter for the moment. Now changed to /testsuite, because the apparent cause for the magnitude increase (.32 -> 1.63 seconds) is the testsuite part of r14-205. See the PR for details. HNY! > > > > With this, the test successfully completes in ~34 seconds. > > > > > > Ok to commit? > > > > > > > Looks OK to me, but Patrick wrote this test so please wait for him to > > confirm. I think this just reduces the number of cases tested, but doesn't > > miss any important edge cases that should be checked. > > Understood: holding, but will ping after the usual week. > Thanks for the review! > > brgds, H-P >
diff --git a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc index a1fbc3241dca..38fa6323d47e 100644 --- a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc +++ b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc @@ -16,6 +16,7 @@ // <http://www.gnu.org/licenses/>. // { dg-do run { target c++20 } } +// { dg-additional-options "-DSIMULATOR_TEST" { target simulator } } // { dg-timeout-factor 4 } #include <limits> @@ -31,6 +32,14 @@ using signed_rep_t = __int128; using signed_rep_t = long long; #endif +#ifdef SIMULATOR_TEST +#define LIMIT 100 +#define LOG2_CEIL_LIMIT 7 +#else +#define LIMIT 1000 +#define LOG2_CEIL_LIMIT 10 +#endif + static_assert(sizeof(max_size_t) == sizeof(max_diff_t)); static_assert(sizeof(rep_t) == sizeof(signed_rep_t)); @@ -199,8 +208,8 @@ test02() using max_type = std::conditional_t<signed_p, max_diff_t, max_size_t>; using shorten_type = std::conditional_t<shorten_p, hw_type, max_type>; const int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__; - const int limit = 1000; - const int log2_limit = 10; + const int limit = LIMIT; + const int log2_limit = LOG2_CEIL_LIMIT; static_assert((1 << log2_limit) >= limit); const int min = (signed_p ? -limit : 0); const int max = limit; @@ -257,8 +266,8 @@ test03() using max_type = std::conditional_t<signed_p, max_diff_t, max_size_t>; using base_type = std::conditional_t<toggle_base_p, hw_type, max_type>; constexpr int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__; - constexpr int limit = 1000; - constexpr int log2_limit = 10; + constexpr int limit = LIMIT; + constexpr int log2_limit = LOG2_CEIL_LIMIT; static_assert((1 << log2_limit) >= limit); const int min = (signed_p ? -limit : 0); const int max = limit; @@ -312,7 +321,7 @@ test03() void test04() { - constexpr int limit = 1000; + constexpr int limit = LIMIT; for (int i = -limit; i <= limit; i++) { VERIFY( -max_size_t(-i) == i );