public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] [libstdc++] [testsuite] require cmath for [PR114359]
@ 2024-06-13  7:27 Alexandre Oliva
  2024-06-13 10:41 ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Oliva @ 2024-06-13  7:27 UTC (permalink / raw)
  To: gcc-patches, libstdc++


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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [libstdc++] [testsuite] require cmath for [PR114359]
  2024-06-13  7:27 [PATCH] [libstdc++] [testsuite] require cmath for [PR114359] Alexandre Oliva
@ 2024-06-13 10:41 ` Jonathan Wakely
  2024-06-13 11:01   ` EXT: " Chen, Yee Men (GE Vernova)
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2024-06-13 10:41 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches, libstdc++

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
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: EXT: Re: [PATCH] [libstdc++] [testsuite] require cmath for [PR114359]
  2024-06-13 10:41 ` Jonathan Wakely
@ 2024-06-13 11:01   ` Chen, Yee Men (GE Vernova)
  0 siblings, 0 replies; 3+ messages in thread
From: Chen, Yee Men (GE Vernova) @ 2024-06-13 11:01 UTC (permalink / raw)
  To: Jonathan Wakely, Alexandre Oliva; +Cc: gcc-patches, libstdc++

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
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-13 11:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-13  7:27 [PATCH] [libstdc++] [testsuite] require cmath for [PR114359] Alexandre Oliva
2024-06-13 10:41 ` Jonathan Wakely
2024-06-13 11:01   ` EXT: " Chen, Yee Men (GE Vernova)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).