Skip to content

[flang] Add flag to disable Float128 runtime #110116

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

Closed
wants to merge 1 commit into from
Closed

[flang] Add flag to disable Float128 runtime #110116

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions flang/runtime/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,13 @@ append(${NO_LTO_FLAGS} CMAKE_CXX_FLAGS)
add_definitions(-U_GLIBCXX_ASSERTIONS)
add_definitions(-U_LIBCPP_ENABLE_ASSERTIONS)

add_subdirectory(Float128Math)
set(FLANG_RUNTIME_ENABLE_F128 On CACHE BOOL
Copy link
Contributor

Choose a reason for hiding this comment

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

We check for F128 in other places too. See #106957

Copy link
Member Author

Choose a reason for hiding this comment

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

I was sort of just trying to get this as a quick fix for building the runtime. I think fixing up all the warnings etc might be a bit more effort.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is okay as an engineering temporary switch, but, in general, uncoordinated settings for float128 math for the compiler driver and runtime may result in completely unsuable driver, e.g. if Float128Math is packaged as a separate library (rather than as part of FortranRuntime) then the driver will try to link it always, so it has to be built during the runtime build. As I uderstand this is not your case, and Float128Math is a part of FortranRuntime (except that it does not build at all due to missing ynl).

If ynl is the only missing piece in those configurations, you can try to ifdef DEFINE_SIMPLE_ALIAS(Yn, ynl) in math-entries.h for those targets. The library will compile, but there will be a runtime error if a program uses bessel_yn for real(16).

Copy link
Member Author

Choose a reason for hiding this comment

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

As I understand it, we currently check if the libc has float 128 support by checking if LDBL_MANT_DIG has a certain value. The issue I guess is that that's sufficient for checking that libc will provide float 128 support for the standard libc functions, but ynl is a glibc-only function.

I think it'd be nice if we could have this support everywhere that does have float128 not just glibc; could we maybe re-use the implementation from llvm/libc++ somehow without introducing a runtime dependency on the system c++ library? And possibly the same for any other glibc-specific functions that might be being used (I haven't checked for others)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or I suppose if there are no targets on which we expect float128 to be available but not a C++ standard library, could we introduce a dependency on the system c++ library just for float128 support?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should use #if defined(__GLIBC__) && defined(_GNU_SOURCE) for defining the simple alias for ynl.

Regarding adding dependency on the C++ library, I think we want to avoid this for FortranRuntime, in general. When float128 support is enabled via libc, the float128 math functions are actually a part of FortranRuntime and so using libc++ would break this "rule".

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try that as a fix instead and see what happens. If it works I can open another PR and close this one. ynl might not be the only glibc-specific function though.

"Specifies whether to build the IEEE-754 128-bit float math support in the\
Flang runtime. This is unsupported on certain platforms.")

if (FLANG_RUNTIME_ENABLE_F128)
add_subdirectory(Float128Math)
endif()

set(sources
ISO_Fortran_binding.cpp
Expand Down Expand Up @@ -232,7 +238,7 @@ set(supported_files
enable_cuda_compilation(FortranRuntime "${supported_files}")
enable_omp_offload_compilation("${supported_files}")

if (NOT TARGET FortranFloat128Math)
if (FLANG_RUNTIME_ENABLE_F128 AND NOT TARGET FortranFloat128Math)
# If FortranFloat128Math is not defined, then we are not building
# standalone FortranFloat128Math library. Instead, include
# the relevant sources into FortranRuntime itself.
Expand Down
Loading