Skip to content

Conversation

@kkwli
Copy link
Collaborator

@kkwli kkwli commented Oct 29, 2024

On AIX, the implementation of std::sqrt is different from that of csqrtf, it leads to different results from compile-time folding and runtime evaluation. This patch attempts to resolve the discrepancy found in #110682 to make the compile-time folding calling csqrtf.

@github-actions
Copy link

github-actions bot commented Oct 29, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@kkwli
Copy link
Collaborator Author

kkwli commented Nov 6, 2024

@klausler @jeanPerier I would like to get feedback of this approach to get the compile time folding and runtime evaluation calling the same routines. On AIX, csqrtf and std::sqrt somehow have different implementations. Thanks in advance.

kkwli added 3 commits November 7, 2024 18:42
- remove the wrapper files
- simplify code
@kkwli
Copy link
Collaborator Author

kkwli commented Nov 8, 2024

@klausler I simplify the code and remove the wrapper file. Please take a look. Thanks.

@kkwli kkwli marked this pull request as ready for review November 8, 2024 22:34
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Nov 8, 2024
@kkwli kkwli self-assigned this Nov 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2024

@llvm/pr-subscribers-flang-semantics

Author: Kelvin Li (kkwli)

Changes

On AIX, the implementation of std::sqrt is different from that of csqrtf, it leads to different results from compile-time folding and runtime evaluation. This patch attempts to resolve the discrepancy found in #110682 to make the compile-time folding calling csqrtf.


Full diff: https://github.com/llvm/llvm-project/pull/114106.diff

1 Files Affected:

  • (modified) flang/lib/Evaluate/intrinsics-library.cpp (+73-2)
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index bb439a6bb3a746..45c6d1876b5500 100644
--- a/flang/lib/Evaluate/intrinsics-library.cpp
+++ b/flang/lib/Evaluate/intrinsics-library.cpp
@@ -15,6 +15,7 @@
 #include "fold-implementation.h"
 #include "host.h"
 #include "flang/Common/erfc-scaled.h"
+#include "flang/Common/idioms.h"
 #include "flang/Common/static-multimap-view.h"
 #include "flang/Evaluate/expression.h"
 #include <cfloat>
@@ -277,6 +278,76 @@ static std::complex<HostT> StdPowF2B(
   return std::pow(x, y);
 }
 
+#ifdef _AIX
+#ifdef __clang_major__
+#pragma clang diagnostic ignored "-Wc99-extensions"
+#endif
+
+extern "C" {
+float _Complex cacosf(float _Complex);
+double _Complex cacos(double _Complex);
+float _Complex csqrtf(float _Complex);
+double _Complex csqrt(double _Complex);
+}
+
+enum CRI { Real, Imag };
+template <typename TR, typename TA> static TR &reIm(TA &x, CRI n) {
+  return reinterpret_cast<TR(&)[2]>(x)[n];
+}
+template <typename TR, typename T> static TR CppToC(const std::complex<T> &x) {
+  TR r;
+  reIm<T, TR>(r, CRI::Real) = x.real();
+  reIm<T, TR>(r, CRI::Imag) = x.imag();
+  return r;
+}
+template <typename T, typename TA> static std::complex<T> CToCpp(const TA &x) {
+  TA &z{const_cast<TA&>(x)};
+  return std::complex<T>(reIm<T, TA>(z, CRI::Real), reIm<T, TA>(z, CRI::Imag));
+}
+#endif
+
+template <typename HostT>
+static std::complex<HostT> CSqrt(const std::complex<HostT> &x) {
+  std::complex<HostT> res;
+#ifdef _AIX
+  // On AIX, the implementation of csqrt[f] and std::sqrt is different,
+  // use csqrt[f] in folding.
+  if constexpr (std::is_same_v<HostT, float>) {
+    float _Complex r{csqrtf(CppToC<float _Complex, float>(x))};
+    res = CToCpp<float, float _Complex>(r);
+  } else if constexpr (std::is_same_v<HostT, double>) {
+    double _Complex r{csqrt(CppToC<double _Complex, double>(x))};
+    res = CToCpp<double, double _Complex>(r);
+  } else {
+    DIE("bad complex component type");
+  }
+#else
+  res = std::sqrt(x);
+#endif
+  return res;
+}
+
+template <typename HostT>
+static std::complex<HostT> CAcos(const std::complex<HostT> &x) {
+  std::complex<HostT> res;
+#ifdef _AIX
+  // On AIX, the implementation of cacos[f] and std::acos is different,
+  // use cacos[f] in folding.
+  if constexpr (std::is_same_v<HostT, float>) {
+    float _Complex r{cacosf(CppToC<float _Complex, float>(x))};
+    res = CToCpp<float, float _Complex>(r);
+  } else if constexpr (std::is_same_v<HostT, double>) {
+    double _Complex r{cacos(CppToC<double _Complex, double>(x))};
+    res = CToCpp<double, double _Complex>(r);
+  } else {
+    DIE("bad complex component type");
+  }
+#else
+  res = std::acos(x);
+#endif
+  return res;
+}
+
 template <typename HostT>
 struct HostRuntimeLibrary<std::complex<HostT>, LibraryVersion::Libm> {
   using F = FuncPointer<std::complex<HostT>, const std::complex<HostT> &>;
@@ -287,7 +358,7 @@ struct HostRuntimeLibrary<std::complex<HostT>, LibraryVersion::Libm> {
   using F2B = FuncPointer<std::complex<HostT>, const std::complex<HostT> &,
       const HostT &>;
   static constexpr HostRuntimeFunction table[]{
-      FolderFactory<F, F{std::acos}>::Create("acos"),
+      FolderFactory<F, F{CAcos}>::Create("acos"),
       FolderFactory<F, F{std::acosh}>::Create("acosh"),
       FolderFactory<F, F{std::asin}>::Create("asin"),
       FolderFactory<F, F{std::asinh}>::Create("asinh"),
@@ -302,7 +373,7 @@ struct HostRuntimeLibrary<std::complex<HostT>, LibraryVersion::Libm> {
       FolderFactory<F2B, F2B{StdPowF2B}>::Create("pow"),
       FolderFactory<F, F{std::sin}>::Create("sin"),
       FolderFactory<F, F{std::sinh}>::Create("sinh"),
-      FolderFactory<F, F{std::sqrt}>::Create("sqrt"),
+      FolderFactory<F, F{CSqrt}>::Create("sqrt"),
       FolderFactory<F, F{std::tan}>::Create("tan"),
       FolderFactory<F, F{std::tanh}>::Create("tanh"),
   };

@kkwli
Copy link
Collaborator Author

kkwli commented Nov 8, 2024

Fixes #110682

@kkwli kkwli requested a review from klausler November 8, 2024 22:36
@kkwli kkwli merged commit 71d4f34 into llvm:main Nov 12, 2024
8 checks passed
@kkwli kkwli deleted the folding branch November 12, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants