release/23.x: [flang] Fix POSIX.1/XPG checks in intrinsics-library.cpp (#201072) - #213109
Open
llvmbot wants to merge 1 commit into
Open
release/23.x: [flang] Fix POSIX.1/XPG checks in intrinsics-library.cpp (#201072)#213109llvmbot wants to merge 1 commit into
llvmbot wants to merge 1 commit into
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)' ``` 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`. (cherry picked from commit 3d94aeb)
Member
Author
|
@MaskRay What do you think about merging this PR to the release branch? |
llvmorg-github-actions
Bot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: llvmbot ChangesBackport 3d94aeb Requested by: @rorth Full diff: https://github.com/llvm/llvm-project/pull/213109.diff 1 Files Affected:
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index 84a24eeb31812..f2c1a7bfaf50b 100644
--- a/flang/lib/Evaluate/intrinsics-library.cpp
+++ b/flang/lib/Evaluate/intrinsics-library.cpp
@@ -27,6 +27,9 @@
#include "flang/Common/float128.h"
#include "flang/Common/float80.h"
#include <type_traits>
+#ifndef _WIN32
+#include <unistd.h> // _POSIX_VERSION, _XOPEN_VERSION
+#endif
namespace Fortran::evaluate {
@@ -439,7 +442,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.
@@ -459,7 +462,58 @@ template <> struct HostRuntimeLibrary<float, LibraryVersion::LibmExtensions> {
static constexpr HostRuntimeMap map{table};
static_assert(map.Verify(), "map must be sorted");
};
-#endif
+#endif // !_AIX && !__APPLE__
+
+template <> struct HostRuntimeLibrary<double, LibraryVersion::LibmExtensions> {
+ using F = FuncPointer<double, double>;
+ using FN = FuncPointer<double, int, double>;
+ static constexpr HostRuntimeFunction table[]{
+ FolderFactory<F, F{::j0}>::Create("bessel_j0"),
+ FolderFactory<F, F{::j1}>::Create("bessel_j1"),
+ FolderFactory<FN, FN{::jn}>::Create("bessel_jn"),
+ FolderFactory<F, F{::y0}>::Create("bessel_y0"),
+ FolderFactory<F, F{::y1}>::Create("bessel_y1"),
+ FolderFactory<FN, FN{::yn}>::Create("bessel_yn"),
+ };
+ static constexpr HostRuntimeMap map{table};
+ static_assert(map.Verify(), "map must be sorted");
+};
+
+#if defined(__GLIBC__) && (HAS_FLOAT80 || HAS_LDBL128)
+template <>
+struct HostRuntimeLibrary<long double, LibraryVersion::LibmExtensions> {
+ using F = FuncPointer<long double, long double>;
+ using FN = FuncPointer<long double, int, long double>;
+ static constexpr HostRuntimeFunction table[]{
+ FolderFactory<F, F{::j0l}>::Create("bessel_j0"),
+ FolderFactory<F, F{::j1l}>::Create("bessel_j1"),
+ FolderFactory<FN, FN{::jnl}>::Create("bessel_jn"),
+ FolderFactory<F, F{::y0l}>::Create("bessel_y0"),
+ FolderFactory<F, F{::y1l}>::Create("bessel_y1"),
+ FolderFactory<FN, FN{::ynl}>::Create("bessel_yn"),
+ };
+ static constexpr HostRuntimeMap map{table};
+ static_assert(map.Verify(), "map must be sorted");
+};
+#endif // __GLIBC__ && (HAS_FLOAT80 || HAS_LDBL128)
+#endif // _POSIX_VERSION >= 200112L || _XOPEN_VERSION >= 600
+
+#ifdef _WIN32
+template <> struct HostRuntimeLibrary<double, LibraryVersion::LibmExtensions> {
+ using F = FuncPointer<double, double>;
+ using FN = FuncPointer<double, int, double>;
+ static constexpr HostRuntimeFunction table[]{
+ FolderFactory<F, F{::_j0}>::Create("bessel_j0"),
+ FolderFactory<F, F{::_j1}>::Create("bessel_j1"),
+ FolderFactory<FN, FN{::_jn}>::Create("bessel_jn"),
+ FolderFactory<F, F{::_y0}>::Create("bessel_y0"),
+ FolderFactory<F, F{::_y1}>::Create("bessel_y1"),
+ FolderFactory<FN, FN{::_yn}>::Create("bessel_yn"),
+ };
+ static constexpr HostRuntimeMap map{table};
+ static_assert(map.Verify(), "map must be sorted");
+};
+#endif // _WIN32
#if HAS_QUADMATHLIB
template <> struct HostRuntimeLibrary<__float128, LibraryVersion::Libm> {
@@ -522,58 +576,7 @@ template <> struct HostRuntimeLibrary<__complex128, LibraryVersion::Libm> {
static constexpr HostRuntimeMap map{table};
static_assert(map.Verify(), "map must be sorted");
};
-#endif
-
-template <> struct HostRuntimeLibrary<double, LibraryVersion::LibmExtensions> {
- using F = FuncPointer<double, double>;
- using FN = FuncPointer<double, int, double>;
- static constexpr HostRuntimeFunction table[]{
- FolderFactory<F, F{::j0}>::Create("bessel_j0"),
- FolderFactory<F, F{::j1}>::Create("bessel_j1"),
- FolderFactory<FN, FN{::jn}>::Create("bessel_jn"),
- FolderFactory<F, F{::y0}>::Create("bessel_y0"),
- FolderFactory<F, F{::y1}>::Create("bessel_y1"),
- FolderFactory<FN, FN{::yn}>::Create("bessel_yn"),
- };
- static constexpr HostRuntimeMap map{table};
- static_assert(map.Verify(), "map must be sorted");
-};
-
-#if defined(__GLIBC__) && (HAS_FLOAT80 || HAS_LDBL128)
-template <>
-struct HostRuntimeLibrary<long double, LibraryVersion::LibmExtensions> {
- using F = FuncPointer<long double, long double>;
- using FN = FuncPointer<long double, int, long double>;
- static constexpr HostRuntimeFunction table[]{
- FolderFactory<F, F{::j0l}>::Create("bessel_j0"),
- FolderFactory<F, F{::j1l}>::Create("bessel_j1"),
- FolderFactory<FN, FN{::jnl}>::Create("bessel_jn"),
- FolderFactory<F, F{::y0l}>::Create("bessel_y0"),
- FolderFactory<F, F{::y1l}>::Create("bessel_y1"),
- FolderFactory<FN, FN{::ynl}>::Create("bessel_yn"),
- };
- static constexpr HostRuntimeMap map{table};
- static_assert(map.Verify(), "map must be sorted");
-};
-#endif // HAS_FLOAT80 || HAS_LDBL128
-#endif //_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600
-
-#ifdef _WIN32
-template <> struct HostRuntimeLibrary<double, LibraryVersion::LibmExtensions> {
- using F = FuncPointer<double, double>;
- using FN = FuncPointer<double, int, double>;
- static constexpr HostRuntimeFunction table[]{
- FolderFactory<F, F{::_j0}>::Create("bessel_j0"),
- FolderFactory<F, F{::_j1}>::Create("bessel_j1"),
- FolderFactory<FN, FN{::_jn}>::Create("bessel_jn"),
- FolderFactory<F, F{::_y0}>::Create("bessel_y0"),
- FolderFactory<F, F{::_y1}>::Create("bessel_y1"),
- FolderFactory<FN, FN{::_yn}>::Create("bessel_yn"),
- };
- static constexpr HostRuntimeMap map{table};
- static_assert(map.Verify(), "map must be sorted");
-};
-#endif
+#endif // HAS_QUADMATHLIB
/// Define pgmath description
#if LINK_WITH_LIBPGMATH
|
Contributor
|
This is pretty crucial to get into 23.x: it unbreaks the Solaris build. |
Contributor
|
Hi @MaskRay, can you review whether this change should go into the release branch? |
Contributor
|
This one is crucial to get into the 23.1.0 release to unbreak the Solaris flang build. It has been on |
Contributor
As part of the process, we need it to be approved before we merge changes into the release branch. @MaskRay, @jeanPerier and @sscalpone, could one of you review whether you feel this change is okay to port to the 23.x release branch? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport 3d94aeb
Requested by: @rorth