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, pku...@freebsd.org wrote: >>> From: Piotr Kubaj <pku...@freebsd.org> >>> >>> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. >>> >>> Signed-off-by: Piotr Kubaj <pku...@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
