[PATCH] PR libstdc++/78179 run long double tests separately

[Jonathan Wakely]

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 <jwakely@redhat.com>
>>>>
>>>>> On 20/09/18 15:36 +0200, Christophe Lyon wrote:
>>>>>> On Wed, 19 Sep 2018 at 23:13, Rainer Orth
>>>>>> <ro@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?




-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 2739 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180921/6476bcac/attachment.bin>