diff mbox series

[libstdc++,testsuite] require cmath for [PR114359]

Message ID or4j9xgv5l.fsf@lxoliva.fsfla.org
State New
Headers show
Series [libstdc++,testsuite] require cmath for [PR114359] | expand

Commit Message

Alexandre Oliva June 13, 2024, 7:27 a.m. UTC
When !_GLIBCXX_USE_C99_MATH_TR1, binomial_distribution doesn't use the
optimized algorithm that was fixed in response to PR114359.  Without
that optimized algorithm, operator() ends up looping very very long
for the test, to the point that it would time out by several orders of
magnitude, without even exercising the optimized algorithm that we're
testing for regressions.  Arrange for the test to be skipped if that
bit won't be exercised.

Regstrapped on x86_64-linux-gnu, also tested with gcc-13
x-aarch64-rtems6, where the problem was detected.  Ok to install?


for  libstdc++-v3/ChangeLog

	PR libstdc++/114359
	* testsuite/26_numerics/random/binomial_distribution/114359.cc:
	Require cmath.
---
 .../random/binomial_distribution/114359.cc         |   13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Jonathan Wakely June 13, 2024, 10:41 a.m. UTC | #1
On Thu, 13 Jun 2024 at 08:27, Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> When !_GLIBCXX_USE_C99_MATH_TR1, binomial_distribution doesn't use the
> optimized algorithm that was fixed in response to PR114359.  Without
> that optimized algorithm, operator() ends up looping very very long
> for the test, to the point that it would time out by several orders of
> magnitude, without even exercising the optimized algorithm that we're
> testing for regressions.  Arrange for the test to be skipped if that
> bit won't be exercised.
>
> Regstrapped on x86_64-linux-gnu, also tested with gcc-13
> x-aarch64-rtems6, where the problem was detected.  Ok to install?

OK.

This finding suggests we shouldn't even bother defining
binomial_distribution without the <cmath> functions. It doesn't seem
useful if it's that slow (or is that just because this test uses 1<<30
as the t parameter?)

Either way, the change to the test is fine.

>
>
> for  libstdc++-v3/ChangeLog
>
>         PR libstdc++/114359
>         * testsuite/26_numerics/random/binomial_distribution/114359.cc:
>         Require cmath.
> ---
>  .../random/binomial_distribution/114359.cc         |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
> index c1e4c380bf91d..12d967dcbfd34 100644
> --- a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
> +++ b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
> @@ -2,6 +2,19 @@
>
>  // Bug 114359 - std::binomial_distribution hangs in infinite loop
>
> +// { dg-require-cmath "" }
> +
> +// The requirement above is not strictly true.  The test should work
> +// without cmath, and it probably does, but without cmath,
> +// binomial_distribution::operator() skips the optimized algorithm and
> +// calls _M_waiting to loop a gazillion times.  On aarch64-rtems6
> +// qemu, that loop takes over 5 minutes to go through a small fraction
> +// of the iteration space (__x at 22k, limited at 1G; __sum at 2e-5,
> +// limited at 0.69).  The bug we're regression-testing here was in the
> +// cmath-requiring bit, so even if this could conceivably not time out
> +// on a really fast machine, there's hardly any reason to exercise
> +// this extreme case.
> +
>  #include <random>
>
>  int main()
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
>
Chen, Yee Men (GE Vernova) June 13, 2024, 11:01 a.m. UTC | #2
Hi,

Please raise a defect.

Yee Men

-----Original Message-----
From: Jonathan Wakely <jwakely@redhat.com> 
Sent: Thursday, June 13, 2024 11:42 AM
To: Alexandre Oliva <oliva@adacore.com>
Cc: gcc-patches@gcc.gnu.org; libstdc++@gcc.gnu.org
Subject: EXT: Re: [PATCH] [libstdc++] [testsuite] require cmath for [PR114359]

WARNING: This email originated from outside of GE. Please validate the sender's email address before clicking on links or attachments as they may not be safe.

On Thu, 13 Jun 2024 at 08:27, Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> When !_GLIBCXX_USE_C99_MATH_TR1, binomial_distribution doesn't use the 
> optimized algorithm that was fixed in response to PR114359.  Without 
> that optimized algorithm, operator() ends up looping very very long 
> for the test, to the point that it would time out by several orders of 
> magnitude, without even exercising the optimized algorithm that we're 
> testing for regressions.  Arrange for the test to be skipped if that 
> bit won't be exercised.
>
> Regstrapped on x86_64-linux-gnu, also tested with gcc-13 
> x-aarch64-rtems6, where the problem was detected.  Ok to install?

OK.

This finding suggests we shouldn't even bother defining binomial_distribution without the <cmath> functions. It doesn't seem useful if it's that slow (or is that just because this test uses 1<<30 as the t parameter?)

Either way, the change to the test is fine.

>
>
> for  libstdc++-v3/ChangeLog
>
>         PR libstdc++/114359
>         * testsuite/26_numerics/random/binomial_distribution/114359.cc:
>         Require cmath.
> ---
>  .../random/binomial_distribution/114359.cc         |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git 
> a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/1143
> 59.cc 
> b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/1143
> 59.cc index c1e4c380bf91d..12d967dcbfd34 100644
> --- 
> a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/1143
> 59.cc
> +++ b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/
> +++ 114359.cc
> @@ -2,6 +2,19 @@
>
>  // Bug 114359 - std::binomial_distribution hangs in infinite loop
>
> +// { dg-require-cmath "" }
> +
> +// The requirement above is not strictly true.  The test should work 
> +// without cmath, and it probably does, but without cmath, // 
> +binomial_distribution::operator() skips the optimized algorithm and 
> +// calls _M_waiting to loop a gazillion times.  On aarch64-rtems6 // 
> +qemu, that loop takes over 5 minutes to go through a small fraction 
> +// of the iteration space (__x at 22k, limited at 1G; __sum at 2e-5, 
> +// limited at 0.69).  The bug we're regression-testing here was in 
> +the // cmath-requiring bit, so even if this could conceivably not 
> +time out // on a really fast machine, there's hardly any reason to 
> +exercise // this extreme case.
> +
>  #include <random>
>
>  int main()
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity 
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
>
diff mbox series

Patch

diff --git a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
index c1e4c380bf91d..12d967dcbfd34 100644
--- a/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
+++ b/libstdc++-v3/testsuite/26_numerics/random/binomial_distribution/114359.cc
@@ -2,6 +2,19 @@ 
 
 // Bug 114359 - std::binomial_distribution hangs in infinite loop
 
+// { dg-require-cmath "" }
+
+// The requirement above is not strictly true.  The test should work
+// without cmath, and it probably does, but without cmath,
+// binomial_distribution::operator() skips the optimized algorithm and
+// calls _M_waiting to loop a gazillion times.  On aarch64-rtems6
+// qemu, that loop takes over 5 minutes to go through a small fraction
+// of the iteration space (__x at 22k, limited at 1G; __sum at 2e-5,
+// limited at 0.69).  The bug we're regression-testing here was in the
+// cmath-requiring bit, so even if this could conceivably not time out
+// on a really fast machine, there's hardly any reason to exercise
+// this extreme case.
+
 #include <random>
 
 int main()