diff mbox series

libstdc++ testsuite/std/ranges/iota/max_size_type.cc: Reduce /10 for simulators

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

Commit Message

Hans-Peter Nilsson Dec. 30, 2023, 1:41 a.m. UTC 
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
and see if I can shift the blame away from MMIX code
generation onto libstdc++-v3.  ;)
Or perhaps the cause is known?

With this, the test successfully completes in ~34 seconds.

Ok to commit?

-- >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(-)

Comments

Jonathan Wakely Dec. 30, 2023, 9:40 a.m. UTC | #1 
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
>
>
Hans-Peter Nilsson Dec. 30, 2023, 6:11 p.m. UTC | #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
Hans-Peter Nilsson Dec. 31, 2023, 4:56 p.m. UTC | #3 
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 mbox series

Patch

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 );