From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 127320 invoked by alias); 21 Sep 2018 11:21:37 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 127277 invoked by uid 89); 21 Sep 2018 11:21:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=12513, tolerance, 12512 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 21 Sep 2018 11:21:34 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BF5B3307D926; Fri, 21 Sep 2018 11:21:32 +0000 (UTC) Received: from localhost (unknown [10.33.36.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 057DC1001938; Fri, 21 Sep 2018 11:21:31 +0000 (UTC) Date: Fri, 21 Sep 2018 11:36:00 -0000 From: Jonathan Wakely To: "Richard Earnshaw (lists)" Cc: Hans-Peter Nilsson , christophe.lyon@linaro.org, ro@cebitec.uni-bielefeld.de, libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org, danglin@gcc.gnu.org, sandra@gcc.gnu.org Subject: Re: [PATCH] PR libstdc++/78179 run long double tests separately Message-ID: <20180921112130.GL23172@redhat.com> References: <201809210052.w8L0qngp025707@ignucius.se.axis.com> <9478e135-b586-c36b-6c54-51388d2ba280@arm.com> <20180921103419.GK23172@redhat.com> <2bd9e665-b57a-41ac-503e-6e2a09180465@arm.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="GyRA7555PLgSTuth" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2bd9e665-b57a-41ac-503e-6e2a09180465@arm.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.2 (2017-12-15) X-SW-Source: 2018-09/txt/msg01227.txt.bz2 --GyRA7555PLgSTuth Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-length: 5890 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 >>>> >>>>> On 20/09/18 15:36 +0200, Christophe Lyon wrote: >>>>>> On Wed, 19 Sep 2018 at 23:13, Rainer Orth >>>>>> 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 >> -// . >> - >> -// { 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? --GyRA7555PLgSTuth Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" Content-length: 2739 commit 2c41ac40a0fdfbe3ef5b8d48de3b62d70ce1b7ef Author: Jonathan Wakely Date: Fri Sep 21 12:19:18 2018 +0100 Un-split hypot 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 -// . - -// { 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 --GyRA7555PLgSTuth--