Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Piotr,

Thanks for doing this, some comments are inlined.

on 2022/5/11 07:32, Piotr Kubaj via Gcc-patches wrote:
> Is there anything more required?
> 
> On 22-05-03 12:33:43, Piotr Kubaj wrote:
>> Here are gmake check-gfortran results requested by FX.
>>
>> Before patching:
>>                 === gfortran Summary ===
>>
>> # of expected passes            65106
>> # of unexpected failures        6
>> # of expected failures          262
>> # of unsupported tests          367
>>
>> After patching:
>>                 === gfortran Summary ===
>>
>> # of expected passes            65384
>> # of unexpected failures        6
>> # of expected failures          262
>> # of unsupported tests          373
>>

Nice!

>> In both cases, unexpected failures are:
>> FAIL: gfortran.dg/pr98076.f90   -O0  execution test
>> FAIL: gfortran.dg/pr98076.f90   -O1  execution test
>> FAIL: gfortran.dg/pr98076.f90   -O2  execution test
>> FAIL: gfortran.dg/pr98076.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
>> FAIL: gfortran.dg/pr98076.f90   -O3 -g  execution test
>> FAIL: gfortran.dg/pr98076.f90   -Os  execution test
>>
>> But this seems unrelated to my patch.>>
>> On 22-05-03 12:21:12, pkubaj@FreeBSD.org wrote:
>>> From: Piotr Kubaj <pkubaj@FreeBSD.org>
>>>
>>> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header.
>>>
>>> Signed-off-by: Piotr Kubaj <pkubaj@FreeBSD.org>
>>> ---
>>>  libgfortran/configure    | 41 +++++++++++++++++++++++++++++++++++++++-
>>>  libgfortran/configure.ac | 17 ++++++++++++++++-
>>>  2 files changed, 56 insertions(+), 2 deletions(-)
>>>
... snip 
>>>  # At least for glibc, clock_gettime is in librt.  But don't
>>>  # pull that in if it still doesn't give us the function we want.  This
>>> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
>>> index 97cc490cb5e..2dd6d345b22 100644
>>> --- a/libgfortran/configure.ac
>>> +++ b/libgfortran/configure.ac
>>> @@ -602,8 +602,23 @@ fi
>>>  # Check whether we have a __float128 type, depends on enable_libquadmath_support
>>>  LIBGFOR_CHECK_FLOAT128
>>>  
>>> +case x$target in
>>> +  xpowerpc*-freebsd*)
>>> +    AC_CACHE_CHECK([for fenv.h and feenableexcept], have_feenableexcept, [
>>> +      AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
>>> +        [[ #include <fenv.h>  ]],
>>> +        [[ feenableexcept(FE_DIVBYZERO | FE_INVALID); ]])],
>>> +        [ have_feenableexcept="yes" ],
>>> +        [ have_feenableexcept="no"  ])])
>>> +    if test "x$have_feenableexcept" = "xyes"; then
>>> +      AC_DEFINE(HAVE_FEENABLEEXCEPT,1,[fenv.h includes feenableexcept])
>>> +    fi;
>>> +    ;;
>>> +  *)

As your explanation in [1], IMHO it's not a good idea to take powerpc*-freebsd* specially
here, for your mentioned triples that may also suffer this same issue, we will have to
update this hunk for them in future.

How about: do the glibc check first as before, if it fails then do one further general check
(for all) with one compilation on fenv.h as what your patch proposes.

Besides, IMHO it should use AC_LINK_IFELSE, since one fenv.h which doesn't implement
feenableexcept could also pass the check.  And there is one AC_CHECK_HEADERS_ONCE checking
for fenv.h existence, not sure if it's worth to guarding the further check with its result.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593248.html

BR,
Kewen