[flang-commits] [flang] [llvm] [flang] Use backend fp128 support to determine REAL(16) availability (PR #221907)

[Michael Kruse via flang-commits]

https://github.com/Meinersbur requested changes to this pull request.

The logic should be simple: Backend can generate code for fp128 (regardless of whether via instruction or runtime call) -> REAL(16) is made available to the user. Do not overcomplicate what could be that simple.


 * Non-issue: `flang-rt/test/Driver/compare_iso_fortran_env_symbols.f90` ensures that the implementation code written in C++ is consistent with what flang would have generated. Whatever conditional compilation is in https://github.com/llvm/llvm-project/blob/main/flang-rt/lib/runtime/iso_fortran_env_impl.f90 should be in https://github.com/llvm/llvm-project/blob/main/flang-rt/lib/runtime/iso_fortran_env_impl.cpp as well. That's all it is supposed to check.

 * Non-issue: REAL(16) now compiles on targets where it did not before. Adding features is not a regression.

 * Slight regression: Fortran programs that use `selected_real_kind` to select `REAL(16)` but their libraries does not have the required functions. Apart from that this kind of kind is really rare, users have the ability to add `-fdisable-real-16` to tell the compiler that their execution environment does not support `REAL(16)`, which is something the compiler conceptually cannot find out. 
 
 * Slight regression: Users get a linker error instead of a compiler error. Add documentation to flang/docs that search engines can find such as 
> Q: I get the error message `some_file.f90:123: undefined reference to `_FortranATanhF128'`
> A: Your Flang runtime was compiled without quadmath support. Recompile Flang-RT with quadmath support using `-DFLANG_RT_SUPPORTS_REAL16=ON` (GNU libquadmath must be available on that machine) or disable quadmath support with  `flang -fdisable-real-16` .
>
> Q: I get the error message `some_file.f90:123: undefined reference to 'tanhq'`
> A: You don't did not link your executable to GNU libquadmath. Enusre that `-lquadmath` is added to the linker command line, or disable quadmath support with  `flang -fdisable-real-16`.

 * Non-issue: In bootstrapping builds fakeflang is used. Detection is only relevant for building the modules files, and here the Flang itself can define a preprocessor symbol declaring whether `REAL(16)` is available like it is already done for e.g. `__x86_64__` (https://github.com/llvm/llvm-project/blob/0ebe3a65e87819ff5068f21fb5a6615b3d580e64/flang/lib/Frontend/CompilerInvocation.cpp#L1971). The only reason why this isnt't done like this is because `REAL(16)` support depends on external libraries.

 * Non-issue: Flang-RT and Flang determine independently whether quadmath support is available, disagreement coud happen:
   * Flang-RT enables quadmath support, but Flang does not: Apart from that this would mean a limitation of the LLVM backend that cannot generate code for something that the C++ compiler used to compile flang-rt can (e.g. gcc), we have an unused libflang_rt.quadmath.a library. Not a problem
   * Flang enables quadmath support, but Flang-RT does not: this is a build environment problem, not the responsibility of the compiler. Just add documentation that users and AIs can find via internet search.
  

We already discussed the regressions, but they are only regressions if you build Flang and your user application on the same machine. With increased availability of Flang in package manager such as SPACK, Linux distribution this uses case becomes less and less relevant. On the contrary: it creates major problems for package distributions. Generally a false-reject (Not compiling a program using `REAL(16)` even though it should work) is much worse that a false-accept (Compiling a program that will not work) and under no circumstance would I trade fixing a false-accept at the cost of a false-reject. Flang is unique among compiler trying to determine library support at compile time. Similar cases all regress in a linker error:
> `undefined reference to '__atomic_fetch_add_8'` (missing libatomic, libclang_rt.atomic or libclang_rt.builtins)
> `undefined reference to '__cxa_begin_catch'` (missing libc++abi, libsupc++)
> `undefined reference to '__divdi3'` (missing libclang_rt.builtins, libgcc_s)
> `undefined reference to '__asan_init'` (missing libclang_rt.asan)
> `undefined reference to '__stack_chk_guard'` (missing libc, libssp)

These are normal build problems and no compiler I know of tries to make you stop using a feature because those libraries were not available. This tries to solve to solve a problem that nobody has.

At most the driver could pass `-fdisable-real-16` to the flang -fc1 invocation if is linking as well and does not find `flang_rt.runtime` in the library search path. But most build tools separate compiler and linker invocation and do not pass `-L` to the compile command, so even this creates more problems than it actually solves.


We are rediscussing that same things again and again. Let me make clear what is required for me to accept this PR (or convince me that I am very wrong):
1. Do not care in Flang whether library support is available to decide whether `REAL(16)` is valid. Conceptually that cannot work reliably. Stop trying to do it anyway.
3. Remove all uses of preprocessor symbols such as `FLANG_RUNTIME_F128_MATH_LIB` from Flang.
4. Move https://github.com/llvm/llvm-project/blob/main/flang/cmake/modules/FlangCommon.cmake to flang-rt. Flang itself has no need do it.

https://github.com/llvm/llvm-project/pull/221907