From cdc17276422b2e14cda9f97b7ad7e84d41463e12 Mon Sep 17 00:00:00 2001
From: Jordan Henderson <jhenderson@hdfgroup.org>
Date: Wed, 3 Apr 2024 23:46:19 -0500
Subject: [PATCH] Revise _Float16 configure checks

Run configure checks with and without CFLAGS/CMAKE_C_FLAGS since some
compilers work in one case while not working in the other case

Sync CMake configure checks with Autotools
---
 config/cmake/ConfigureChecks.cmake | 34 ++++++++++++-----
 config/cmake/ConversionTests.c     |  4 +-
 configure.ac                       | 61 +++++++++++++++++++++++++-----
 3 files changed, 78 insertions(+), 21 deletions(-)

diff --git a/config/cmake/ConfigureChecks.cmake b/config/cmake/ConfigureChecks.cmake
index 81633da8909..2fe4e217eb0 100644
--- a/config/cmake/ConfigureChecks.cmake
+++ b/config/cmake/ConfigureChecks.cmake
@@ -923,25 +923,30 @@ if (HDF5_ENABLE_NONSTANDARD_FEATURE_FLOAT16)
       # compile a program that will generate these functions to check for _Float16
       # support. If we fail to compile this program, we will simply disable
       # _Float16 support for the time being.
+      H5ConversionTests (
+          ${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK
+          FALSE
+          "Checking if compiler can convert _Float16 type with casts"
+      )
 
       # Some compilers, notably AppleClang on MacOS 12, will succeed in the
-      # configure check below when optimization flags like -O3 are manually
+      # configure check above when optimization flags like -O3 are manually
       # passed in CMAKE_C_FLAGS. However, the build will then fail when it
       # reaches compilation of H5Tconv.c because of the issue mentioned above.
-      # MacOS 13 appears to have fixed this, but, just to be sure, backup and
-      # clear CMAKE_C_FLAGS before performing these configure checks.
+      # MacOS 13 appears to have fixed this, but, just to be sure, make sure
+      # the check also passes without the passed in CMAKE_C_FLAGS.
       set (cmake_c_flags_backup "${CMAKE_C_FLAGS}")
       set (CMAKE_C_FLAGS "")
 
       H5ConversionTests (
-          ${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK
+          ${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK_NO_FLAGS
           FALSE
-          "Checking if compiler can convert _Float16 type with casts"
+          "Checking if compiler can convert _Float16 type with casts (without CMAKE_C_FLAGS)"
       )
 
       set (CMAKE_C_FLAGS "${cmake_c_flags_backup}")
 
-      if (${${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK})
+      if (${${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK} AND ${${HDF_PREFIX}_FLOAT16_CONVERSION_FUNCS_LINK_NO_FLAGS})
         # Finally, MacOS 13 appears to have a bug specifically when converting
         # long double values to _Float16. Release builds of the dt_arith test
         # would cause any assignments to a _Float16 variable to be elided,
@@ -949,20 +954,25 @@ if (HDF5_ENABLE_NONSTANDARD_FEATURE_FLOAT16)
         # simply chopping off all the bytes of the value except for the first 2.
         # These tests pass on MacOS 14, so let's perform a quick test to check
         # if the hardware conversion is done correctly.
+        H5ConversionTests (
+            ${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT
+            TRUE
+            "Checking if correctly converting long double to _Float16 values"
+        )
 
-        # Backup and clear CMAKE_C_FLAGS before performing configure checks
+        # Backup and clear CMAKE_C_FLAGS before performing configure check again
         set (cmake_c_flags_backup "${CMAKE_C_FLAGS}")
         set (CMAKE_C_FLAGS "")
 
         H5ConversionTests (
-            ${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT
+            ${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT_NO_FLAGS
             TRUE
-            "Checking if correctly converting long double to _Float16 values"
+            "Checking if correctly converting long double to _Float16 values (without CMAKE_C_FLAGS)"
         )
 
         set (CMAKE_C_FLAGS "${cmake_c_flags_backup}")
 
-        if (NOT ${${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT})
+        if (NOT ${${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT} OR NOT ${${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT_NO_FLAGS})
           message (VERBOSE "Conversions from long double to _Float16 appear to be incorrect. These will be emulated through a soft conversion function.")
         endif ()
 
@@ -985,3 +995,7 @@ else ()
   unset (${HDF_PREFIX}_HAVE__FLOAT16 CACHE)
   unset (${HDF_PREFIX}_LDOUBLE_TO_FLOAT16_CORRECT CACHE)
 endif ()
+
+if (NOT ${HDF_PREFIX}_HAVE__FLOAT16)
+  set (HDF5_ENABLE_NONSTANDARD_FEATURE_FLOAT16 OFF CACHE BOOL "Enable support for _Float16 C datatype" FORCE)
+endif ()
diff --git a/config/cmake/ConversionTests.c b/config/cmake/ConversionTests.c
index 8e103bd3e97..b5d2af2ff38 100644
--- a/config/cmake/ConversionTests.c
+++ b/config/cmake/ConversionTests.c
@@ -286,7 +286,7 @@ int HDF_NO_UBSAN main(void)
 
 #endif
 
-#ifdef H5_FLOAT16_CONVERSION_FUNCS_LINK_TEST
+#if defined(H5_FLOAT16_CONVERSION_FUNCS_LINK_TEST) || defined(H5_FLOAT16_CONVERSION_FUNCS_LINK_NO_FLAGS_TEST)
 
 #define __STDC_WANT_IEC_60559_TYPES_EXT__
 
@@ -379,7 +379,7 @@ int HDF_NO_UBSAN main(void)
 
 #endif
 
-#ifdef H5_LDOUBLE_TO_FLOAT16_CORRECT_TEST
+#if defined(H5_LDOUBLE_TO_FLOAT16_CORRECT_TEST) || defined(H5_LDOUBLE_TO_FLOAT16_CORRECT_NO_FLAGS_TEST)
 
 #define __STDC_WANT_IEC_60559_TYPES_EXT__
 
diff --git a/configure.ac b/configure.ac
index cca338867ea..5fb3718ef1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -674,11 +674,34 @@ if test "X$ENABLE_FLOAT16" = "Xyes"; then
       AC_CACHE_VAL([hdf5_cv_float16_conversion_funcs_link],
       [AC_RUN_IFELSE(
           [AC_LANG_SOURCE([$TEST_SRC])],
-          [hdf5_cv_float16_conversion_funcs_link=yes], [hdf5_cv_float16_conversion_funcs_link=no], [hdf5_cv_float16_conversion_funcs_link=no])])
+          [hdf5_cv_float16_conversion_funcs_link=yes],
+          [hdf5_cv_float16_conversion_funcs_link=no],
+          [hdf5_cv_float16_conversion_funcs_link=no])])
+      AC_MSG_RESULT(${hdf5_cv_float16_conversion_funcs_link})
+
+      # Some compilers, notably AppleClang on MacOS 12, will succeed in the
+      # configure check above when optimization flags like -O3 are manually
+      # passed in CFLAGS. However, the build will then fail when it reaches
+      # compilation of H5Tconv.c because of the issue mentioned above. MacOS
+      # 13 appears to have fixed this, but, just to be sure, make sure the
+      # check also passes without the passed in CFLAGS.
+      conftest_cflags_backup="$CFLAGS"
+      CFLAGS=""
+
+      AC_MSG_CHECKING([if compiler can correctly compile and run a test program which converts _Float16 to other types with casts (without CFLAGS)])
+      TEST_SRC="`(echo \"#define H5_FLOAT16_CONVERSION_FUNCS_LINK_NO_FLAGS_TEST 1\"; cat $srcdir/config/cmake/ConversionTests.c)`"
+      AC_CACHE_VAL([hdf5_cv_float16_conversion_funcs_link_no_flags],
+      [AC_RUN_IFELSE(
+          [AC_LANG_SOURCE([$TEST_SRC])],
+          [hdf5_cv_float16_conversion_funcs_link_no_flags=yes],
+          [hdf5_cv_float16_conversion_funcs_link_no_flags=no],
+          [hdf5_cv_float16_conversion_funcs_link_no_flags=no])])
+      AC_MSG_RESULT(${hdf5_cv_float16_conversion_funcs_link_no_flags})
 
-      if test ${hdf5_cv_float16_conversion_funcs_link} = "yes"; then
-        AC_MSG_RESULT([yes])
+      CFLAGS="$conftest_cflags_backup"
 
+      if test ${hdf5_cv_float16_conversion_funcs_link} = "yes" &&
+         test ${hdf5_cv_float16_conversion_funcs_link_no_flags} = "yes"; then
         # Finally, MacOS 13 appears to have a bug specifically when converting
         # long double values to _Float16. Release builds of the dt_arith test
         # would cause any assignments to a _Float16 variable to be elided,
@@ -694,15 +717,37 @@ if test "X$ENABLE_FLOAT16" = "Xyes"; then
           AC_CACHE_VAL([hdf5_cv_ldouble_to_float16_correct],
           [AC_RUN_IFELSE(
               [AC_LANG_SOURCE([$TEST_SRC])],
-              [hdf5_cv_ldouble_to_float16_correct=yes], [hdf5_cv_ldouble_to_float16_correct=no], [hdf5_cv_ldouble_to_float16_correct=yes])])
+              [hdf5_cv_ldouble_to_float16_correct=yes],
+              [hdf5_cv_ldouble_to_float16_correct=no],
+              [hdf5_cv_ldouble_to_float16_correct=yes])])
         fi
+        AC_MSG_RESULT(${hdf5_cv_ldouble_to_float16_correct})
+
+        # Backup and clear CFLAGS before performing configure check again
+        conftest_cflags_backup="$CFLAGS"
+        CFLAGS=""
 
-        if test ${hdf5_cv_ldouble_to_float16_correct} = "yes"; then
+        AC_MSG_CHECKING([if compiler can correctly convert long double values to _Float16 (without CFLAGS)])
+        TEST_SRC="`(echo \"#define H5_LDOUBLE_TO_FLOAT16_CORRECT_NO_FLAGS_TEST 1\"; cat $srcdir/config/cmake/ConversionTests.c)`"
+        if test ${ac_cv_sizeof_long_double} = 0; then
+          hdf5_cv_ldouble_to_float16_correct_no_flags=${hdf5_cv_ldouble_to_float16_correct_no_flags=no}
+        else
+          AC_CACHE_VAL([hdf5_cv_ldouble_to_float16_correct_no_flags],
+          [AC_RUN_IFELSE(
+              [AC_LANG_SOURCE([$TEST_SRC])],
+              [hdf5_cv_ldouble_to_float16_correct_no_flags=yes],
+              [hdf5_cv_ldouble_to_float16_correct_no_flags=no],
+              [hdf5_cv_ldouble_to_float16_correct_no_flags=yes])])
+        fi
+        AC_MSG_RESULT(${hdf5_cv_ldouble_to_float16_correct_no_flags})
+
+        CFLAGS="$conftest_cflags_backup"
+
+        if test ${hdf5_cv_ldouble_to_float16_correct} = "yes" &&
+           test ${hdf5_cv_ldouble_to_float16_correct_no_flags} = "yes"; then
           AC_DEFINE([LDOUBLE_TO_FLOAT16_CORRECT], [1],
             [Define if your system can convert long double to _Float16 values correctly.])
-          AC_MSG_RESULT([yes])
         else
-          AC_MSG_RESULT([no])
           AC_MSG_NOTICE([Conversions from long double to _Float16 appear to be incorrect. These will be emulated through a soft conversion function.])
         fi
 
@@ -714,8 +759,6 @@ if test "X$ENABLE_FLOAT16" = "Xyes"; then
 
         # Define HAVE__FLOAT16 macro for H5pubconf.h if _Float16 is available.
         AC_DEFINE([HAVE__FLOAT16], [1], [Determine if _Float16 is available])
-      else
-        AC_MSG_RESULT([no])
       fi
     fi