On 21/09/18 11:39 +0100, Richard Earnshaw (lists) wrote:
On 21/09/18 11:34, Jonathan Wakely wrote:
On 21/09/18 11:24 +0100, Richard Earnshaw (lists) wrote:
On 21/09/18 01:52, Hans-Peter Nilsson wrote:
Date: Thu, 20 Sep 2018 15:22:23 +0100
From: Jonathan Wakely <jwak...@redhat.com>

On 20/09/18 15:36 +0200, Christophe Lyon wrote:
On Wed, 19 Sep 2018 at 23:13, Rainer Orth
<r...@cebitec.uni-bielefeld.de> wrote:

Hi Christophe,

I have noticed failures on hypot-long-double.cc on arm, so I
suggest we add:

diff --git
a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc

b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc

index 8a05473..4c2e33b 100644
---
a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc

+++
b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc

@@ -17,7 +17,7 @@

 // { dg-options "-std=gnu++17" }
 // { dg-do run { target c++17 } }
-// { dg-xfail-run-if "PR 78179" { powerpc-ibm-aix* hppa-*-linux*
nios2-*-* } }
+// { dg-xfail-run-if "PR 78179" { powerpc-ibm-aix* hppa-*-linux*
nios2-*-* arm*-*-* } }

 // Run the long double tests from hypot.cc separately, because
they fail on a
 // number of targets. See PR libstdc++/78179 for details.

OK?

just a nit (and not a review): I'd prefer the target list to be
sorted
alphabetically, not completely random.


Sure, I can sort the whole list, if OK on principle.

Yes, please go ahead and commit it with the sorted list.

"Me too".  Can I please, rather than piling on to a target list,
replace the whole xfail-list with the equivalent of "target { !
large_long_double }" (an already-existing "effective target")?

I'll leave the thought of running the test only for
large_long_double targets (qualifying the dg-do run) instead of
an xfail-clause for maintainers.

brgds, H-P


Xfailing sounds wrong to me anyway.  An xfailed test ought to run, but
some critical component other than the bug/regression in the test
prevents it happening.  In this case the test can never be made to work
because the target environment simply doesn't support it.

That's not true, the test would work fine with different tolerances
for the expected results. If we used the same tolerances as for
double, then the targets where double and long double are the same
would pass.

So better to
just skip it.  If we want a separate compile-only test, then that's a
different issue and should be in a separate test.

I was going to say that the advantage of dg-xfail-run-if is that we
still compile it, we just know it fails to produce the expected
results. There's definitely a benefit to checking it compiles OK even
when sizeof(long double) == sizeof(double).

So +1 for using large_long_double.

Or we could un-split the test and do this instead:



patch.txt


diff --git 
a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc 
b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc
deleted file mode 100644
index bcc1fec635b..00000000000
--- a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright (C) 2016-2018 Free Software Foundation, Inc.
-//
-// This file is part of the GNU ISO C++ Library.  This library is free
-// software; you can redistribute it and/or modify it under the
-// terms of the GNU General Public License as published by the
-// Free Software Foundation; either version 3, or (at your option)
-// any later version.
-
-// This library is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License along
-// with this library; see the file COPYING3.  If not see
-// <http://www.gnu.org/licenses/>.
-
-// { dg-options "-std=gnu++17" }
-// { dg-do run { target c++17 } }
-// { dg-xfail-run-if "PR 78179" { arm*-*-* hppa-*-linux* nios2-*-* 
powerpc-ibm-aix* } }
-
-// Run the long double tests from hypot.cc separately, because they fail on a
-// number of targets. See PR libstdc++/78179 for details.
-#define TEST_HYPOT_LONG_DOUBLE
-#include "hypot.cc"
diff --git a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc 
b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
index 36c7553c5e8..886952d39b6 100644
--- a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
+++ b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
@@ -126,12 +126,12 @@ void
 test01()
 {
   // See hypot-long-double.cc for this macro
-#ifndef TEST_HYPOT_LONG_DOUBLE
   test(data1, toler1);
   test(data2, toler2);
-#else
-  test(data3, toler3);
-#endif
+  if (sizeof(long double) > sizeof(long double))

I presume you intend the second to be sizeof (double)...
Otherwise the test is always false.

Ha, yes :-)

I wrote it as sizeof(double) < sizeof(long double) at first, but
decided to flip it. And didn't finish doing so.

+    test(data3, toler3);
+  else
+    test(data3, (long double)toler2);

Also this should be (long double)toler1 because toler2 is the float
version.

Here's the corrected patch, any objections to this?




commit 2c41ac40a0fdfbe3ef5b8d48de3b62d70ce1b7ef
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Fri Sep 21 12:19:18 2018 +0100

    Un-split hypot<long double> tests
    
    Remove the hypot-long-double.cc file that used dg-xfail-run-if and
    simply use the lower tolerance for double if long double is not larger
    than double.
    
            * testsuite/26_numerics/headers/cmath/hypot-long-double.cc: Remove.
            * testsuite/26_numerics/headers/cmath/hypot.cc: Restore test for
            long double unconditionally, but use lower tolerance when
            sizeof(long double) == sizeof(double).

diff --git a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc
deleted file mode 100644
index bcc1fec635b..00000000000
--- a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot-long-double.cc
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright (C) 2016-2018 Free Software Foundation, Inc.
-//
-// This file is part of the GNU ISO C++ Library.  This library is free
-// software; you can redistribute it and/or modify it under the
-// terms of the GNU General Public License as published by the
-// Free Software Foundation; either version 3, or (at your option)
-// any later version.
-
-// This library is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License along
-// with this library; see the file COPYING3.  If not see
-// <http://www.gnu.org/licenses/>.
-
-// { dg-options "-std=gnu++17" }
-// { dg-do run { target c++17 } }
-// { dg-xfail-run-if "PR 78179" { arm*-*-* hppa-*-linux* nios2-*-* powerpc-ibm-aix* } }
-
-// Run the long double tests from hypot.cc separately, because they fail on a
-// number of targets. See PR libstdc++/78179 for details.
-#define TEST_HYPOT_LONG_DOUBLE
-#include "hypot.cc"
diff --git a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
index 36c7553c5e8..9fe8f53b00e 100644
--- a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
+++ b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
@@ -125,13 +125,12 @@ const long double toler3 = 1e-16l;
 void
 test01()
 {
-  // See hypot-long-double.cc for this macro
-#ifndef TEST_HYPOT_LONG_DOUBLE
   test(data1, toler1);
   test(data2, toler2);
-#else
-  test(data3, toler3);
-#endif
+  if (sizeof(long double) > sizeof(double))
+    test(data3, toler3);
+  else
+    test(data3, (long double)toler1);
 }
 
 int

Reply via email to