Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise _Float16 configure checks #4323

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Revise _Float16 configure checks #4323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abe0a7a
Revise _Float16 configure checks
jhendersonHDF Apr 4, 2024
22f121d
Merge remote-tracking branch 'canonical/develop' into float16_config_…
lrknox Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions config/cmake/ConfigureChecks.cmake
View file
Open in desktop
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the issue related to the OS or the C clang version? Though I've not tried it, I would assume that gcc installed on 13 OS systems would work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just referring to the default AppleClang here for a particular MacOS version. I'd assume GCC should work as well, but haven't tested that.

# 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
View file
Open in desktop
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
View file
Open in desktop
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
Loading