[flang] Fix POSIX.1/XPG checks in intrinsics-library.cpp - #201072
Conversation
PR llvm#201063 breaks the `flang` build on Solaris: ``` flang/lib/Evaluate/intrinsics-library.cpp:225:26: error: address of overloaded function 'acos' does not match required type '__float128 (__float128)' flang/lib/Evaluate/intrinsics-library.cpp:225:26: error: address of overloaded function 'acos' does not match required type '_Complex __float128 (_Complex __float128)' ``` The problem is that the `__float128` support in `intrinsics-library.cpp` is guarded incorrectly: it tests for `_POSIX_C_SOURCE >= 200112L` or `_XOPEN_SOURCE >= 600`, which are no longer defined on Solaris after the PR above. This check is due a misunderstanding of those feature test macros: as detailed in [The Open Group Base Specifications Issue 8, 2.2.1 POSIX.1 Symbols](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html), those macros are expected to be **defined** by the user to ensure that the features introduced by a particular version of POSIX.1 or XPG are enabled. To test if they actually are supported, the values of `_POSIX_VERSION` or `_XOPEN_VERSION` need to be checked instead, as explained in [2.1.3.1 POSIX System Interfaces](https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap02.html). If the XPG6 features required for `flang` are present by default in a compilation environment, `_POSIX_VERSION` or `_XOPEN_VERSION` will be defined to the required values even without defining `_POSIX_C_SOURCE` or `_XOPEN_SOURCE`. Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, and `x86_64-pc-linux-gnu`.
|
@llvm/pr-subscribers-flang-semantics Author: Rainer Orth (rorth) ChangesPR #201063 breaks the The problem is that the If the XPG6 features required for Tested on Full diff: https://github.com/llvm/llvm-project/pull/201072.diff 1 Files Affected:
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index 54726ac539d60..68d47d2c248b3 100644
--- a/flang/lib/Evaluate/intrinsics-library.cpp
+++ b/flang/lib/Evaluate/intrinsics-library.cpp
@@ -440,7 +440,7 @@ struct HostRuntimeLibrary<std::complex<double>, LibraryVersion::Libm> {
// clang libc++ (ok in GNU libstdc++). Instead, the Posix libm
// extensions are used when available below.
-#if _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600
+#if _POSIX_VERSION >= 200112L || _XOPEN_VERSION >= 600
/// Define libm extensions
/// Bessel functions are defined in POSIX.1-2001.
@@ -557,7 +557,7 @@ struct HostRuntimeLibrary<long double, LibraryVersion::LibmExtensions> {
static_assert(map.Verify(), "map must be sorted");
};
#endif // HAS_FLOAT80 || HAS_LDBL128
-#endif //_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600
+#endif //_POSIX_VERSION >= 200112L || _XOPEN_VERSION >= 600
#ifdef _WIN32
template <> struct HostRuntimeLibrary<double, LibraryVersion::LibmExtensions> {
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
@rorth , I assume you are looking at the CI failure, because it shows the exact problem the original code was trying to guard against... |
Sure. However, I didn't see them when testing on Alternatively, one could simply move the part guarded by However, to really investigate I need to know how exactly that CI build is run. Otherwise, it would be impossible to reproduce. |
…d `_XOPEN_VERSION`.
|
At least checking both the |
|
Build successfully on AIX. Thanks |
|
This is still an issue and breaks building LLVM 23.1.0 rc1. What about the revised version? |
|
@MaskRay @jeanPerier @vdonaldson This still seems to be an issue for @rorth, with no review activity in about 7 weeks. Could one of you take a look? |
| // extensions are used when available below. | ||
|
|
||
| #if _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600 | ||
| #if _POSIX_C_SOURCE >= 200112L || _POSIX_VERSION >= 200112L || \ |
There was a problem hiding this comment.
This file doesn't include unistd.h, _POSIX_VERSION and _XOPEN_VERSION feel dead to me.
|
You're right about the There's a mixture of If you change the structure to the compile failures observed when POSIX.1-2001/XPG6 is (incorrectly) assumed to be missing are gone. Let's get to the actual POSIX.1/XPG guards now. Consider a (modified) version of the If you build this with
As you can see, the system headers predefining the feature test macros themselves rather than their I've got a patch doing just this, so far only lightly tested on Solaris and Linux. If we can agree this is the direction to take, I'll test it some more on both Darwin and AIX, unless someone else who has development environments on those readily available beats me to it. I could go on about properly guarding the various versions of the bessel functions, some of them in XPG6, others mere extensions on all sorts of platforms, but for the moment I'm primarily concerned about fixing the Solaris compile failure. |
Move quadmathlib code out of POSIX.1/XPG guard.
|
/cherry-pick 3d94aeb |
|
/pull-request #213109 |
PR llvm#201063 breaks the `flang` build on Solaris: ``` flang/lib/Evaluate/intrinsics-library.cpp:225:26: error: address of overloaded function 'acos' does not match required type '__float128 (__float128)' flang/lib/Evaluate/intrinsics-library.cpp:225:26: error: address of overloaded function 'acos' does not match required type '_Complex __float128 (_Complex __float128)' ``` There are two problems here: - The `__float128` support in `intrinsics-library.cpp` is guarded by the POSIX.1 >= 2001/XPG >= 6 check, but only depends on `HAVE_QUADMATHLIB`. - That check is done incorrectly: it tests for `_POSIX_C_SOURCE >= 200112L` or `_XOPEN_SOURCE >= 600`, which are no longer defined on Solaris after the PR above. This check is due a misunderstanding of those feature test macros: as detailed in [The Open Group Base Specifications Issue 8, 2.2.1 POSIX.1 Symbols](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html), those macros are expected to be **defined** by the user to ensure that the features introduced by a particular version of POSIX.1 or XPG are enabled, and only be checked by system headers. To test if they actually are supported, the values of `_POSIX_VERSION` or `_XOPEN_VERSION` need to be checked instead, as explained in [2.1.3.1 POSIX System Interfaces](https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap02.html). The feature test macros being defined by the system headers is a non-portable glibc/AIX extension, it seems. If the XPG6 features required for `flang` are present by default in a compilation environment, `_POSIX_VERSION` or `_XOPEN_VERSION` will be defined to the required values even without defining `_POSIX_C_SOURCE` or `_XOPEN_SOURCE`. Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, and `x86_64-pc-linux-gnu`.
PR #201063 breaks the
flangbuild on Solaris:There are two problems here:
__float128support inintrinsics-library.cppis guarded by the POSIX.1 >= 2001/XPG >= 6 check, but only depends onHAVE_QUADMATHLIB._POSIX_C_SOURCE >= 200112Lor_XOPEN_SOURCE >= 600, which are no longer defined on Solaris after the PR above. This check is due a misunderstanding of those feature test macros: as detailed in The Open Group Base Specifications Issue 8, 2.2.1 POSIX.1 Symbols, those macros are expected to be defined by the user to ensure that the features introduced by a particular version of POSIX.1 or XPG are enabled, and only be checked by system headers. To test if they actually are supported, the values of_POSIX_VERSIONor_XOPEN_VERSIONneed to be checked instead, as explained in 2.1.3.1 POSIX System Interfaces. The feature test macros being defined by the system headers is a non-portable glibc/AIX extension, it seems.If the XPG6 features required for
flangare present by default in a compilation environment,_POSIX_VERSIONor_XOPEN_VERSIONwill be defined to the required values even without defining_POSIX_C_SOURCEor_XOPEN_SOURCE.Tested on
amd64-pc-solaris2.11,sparcv9-sun-solaris2.11, andx86_64-pc-linux-gnu.