Skip to content

Commit

Permalink
Revise _Float16 configure checks (HDFGroup#4323)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jhendersonHDF authored Apr 4, 2024
1 parent 3db6957 commit 90a1bae
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 21 deletions.
34 changes: 24 additions & 10 deletions config/cmake/ConfigureChecks.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -923,46 +923,56 @@ 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,
# whereas Debug builds would perform incorrect hardware conversions by
# 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 ()

Expand All @@ -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 ()
4 changes: 2 additions & 2 deletions config/cmake/ConversionTests.c
Original file line number Diff line number Diff line change
Expand Up @@ -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__

Expand Down Expand Up @@ -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__

Expand Down
61 changes: 52 additions & 9 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand All @@ -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

Expand Down

0 comments on commit 90a1bae

Please sign in to comment.