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

[Richard Earnshaw (lists)]

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.

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