[flang] remove support for std::complex value lowering.

[jeanPerier]

Part of the RFC to use mlir complex type.

[DavidTruby]

I'm not familiar with the exact ABI of the mlir::TupleType, do they guarantee that it has the same ABI as a struct with the same members? I assume that is what it is underneath.
If so, LGTM.

Would this change allow us to use std::complex directly in interfaces in the runtime? That'd help us clean up the Windows stuff a lot (since _Complex doesn't exist there)

[DavidTruby]

On second thought, should we possibly remove the by-value models entirely? I think those will only cause issues.
For an example off the top of my head, std::complex<float> and struct {float; float;}; do not have the same return value ABI on Windows. So I think we should only ever be using std::complex by reference

[jeanPerier]

I'm not familiar with the exact ABI of the mlir::TupleType, do they guarantee that it has the same ABI as a struct with the same members? I assume that is what it is underneath.

It lowers to { float, float } in LLVM IR. I can't guarantee this is OK on all ABIs. MLIR is not really equipped to deal with aggregate by value ABIs. As you mentioned, we should rather stay away from this altogether.

On second thought, should we possibly remove the by-value models entirely? I think those will only cause issues.

I agree with you here. It is currently only required for the REDUCE intrinsics, and more specifically for the callback signature. I think it is most likely a runtime bug since the callback is defined in Fortran, so I opened an issue.

For an example off the top of my head, std::complex<float> and struct {float; float;}; do not have the same return value ABI on Windows.

I know this is true for _Complex vs struct {float; float;};, but the C++ standard defines std::complex as class template that is storage compatible with struct {float; float;}. So if it is a class, it falls into the same ABI rules as a struct as far as I know. On 32 bit machine where _Complex and std::complex ABI differ, std::complex and struct {float; float;}; are lowered the same way in intefaces (see this godbolt example). Anyway, I agree with your point that it is best to stay away from it.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: None (jeanPerier)

Changes

Part of the RFC to use mlir complex type.

---

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

2 Files Affected:

• (modified) flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h (+51-20)
• (modified) flang/unittests/Optimizer/RTBuilder.cpp (+1)

diff --git a/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h b/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h
index a103861f1510b8..6acabf53853911 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h
@@ -400,17 +400,19 @@ constexpr TypeBuilderFunc getModel<bool &>() {
     return fir::ReferenceType::get(f(context));
   };
 }
-template <>
-constexpr TypeBuilderFunc getModel<std::complex<float>>() {
-  return [](mlir::MLIRContext *context) -> mlir::Type {
-    return mlir::ComplexType::get(mlir::FloatType::getF32(context));
-  };
-}
+
+// getModel<std::complex<T>> are not implemented on purpose.
+// Prefer passing/returning the complex by reference in the runtime to
+// avoid ABI issues.
+// C++ std::complex is not an intrinsic type, and it while it is storage
+// compatible with C/Fortran complex type, it follows the struct value passing
+// ABI rule, which may differ from how C complex are passed on some platforms.
+
 template <>
 constexpr TypeBuilderFunc getModel<std::complex<float> &>() {
   return [](mlir::MLIRContext *context) -> mlir::Type {
-    TypeBuilderFunc f{getModel<std::complex<float>>()};
-    return fir::ReferenceType::get(f(context));
+    mlir::Type floatTy = getModel<float>()(context);
+    return fir::ReferenceType::get(mlir::ComplexType::get(floatTy));
   };
 }
 template <>
@@ -422,16 +424,10 @@ constexpr TypeBuilderFunc getModel<const std::complex<float> *>() {
   return getModel<std::complex<float> *>();
 }
 template <>
-constexpr TypeBuilderFunc getModel<std::complex<double>>() {
-  return [](mlir::MLIRContext *context) -> mlir::Type {
-    return mlir::ComplexType::get(mlir::FloatType::getF64(context));
-  };
-}
-template <>
 constexpr TypeBuilderFunc getModel<std::complex<double> &>() {
   return [](mlir::MLIRContext *context) -> mlir::Type {
-    TypeBuilderFunc f{getModel<std::complex<double>>()};
-    return fir::ReferenceType::get(f(context));
+    mlir::Type floatTy = getModel<double>()(context);
+    return fir::ReferenceType::get(mlir::ComplexType::get(floatTy));
   };
 }
 template <>
@@ -521,10 +517,45 @@ REDUCTION_VALUE_OPERATION_MODEL(double)
 REDUCTION_REF_OPERATION_MODEL(long double)
 REDUCTION_VALUE_OPERATION_MODEL(long double)
 
-REDUCTION_REF_OPERATION_MODEL(std::complex<float>)
-REDUCTION_VALUE_OPERATION_MODEL(std::complex<float>)
-REDUCTION_REF_OPERATION_MODEL(std::complex<double>)
-REDUCTION_VALUE_OPERATION_MODEL(std::complex<double>)
+// FIXME: the runtime is not using the correct ABIs when calling complex
+// callbacks. lowering either need to create wrappers or just have an inline
+// implementation for it. https://github.com/llvm/llvm-project/issues/110674
+template <>
+constexpr TypeBuilderFunc
+getModel<Fortran::runtime::ValueReductionOperation<std::complex<float>>>() {
+  return [](mlir::MLIRContext *context) -> mlir::Type {
+    mlir::Type cplx = mlir::ComplexType::get(getModel<float>()(context));
+    auto refTy = fir::ReferenceType::get(cplx);
+    return mlir::FunctionType::get(context, {cplx, cplx}, refTy);
+  };
+}
+template <>
+constexpr TypeBuilderFunc
+getModel<Fortran::runtime::ValueReductionOperation<std::complex<double>>>() {
+  return [](mlir::MLIRContext *context) -> mlir::Type {
+    mlir::Type cplx = mlir::ComplexType::get(getModel<double>()(context));
+    auto refTy = fir::ReferenceType::get(cplx);
+    return mlir::FunctionType::get(context, {cplx, cplx}, refTy);
+  };
+}
+template <>
+constexpr TypeBuilderFunc
+getModel<Fortran::runtime::ReferenceReductionOperation<std::complex<float>>>() {
+  return [](mlir::MLIRContext *context) -> mlir::Type {
+    mlir::Type cplx = mlir::ComplexType::get(getModel<float>()(context));
+    auto refTy = fir::ReferenceType::get(cplx);
+    return mlir::FunctionType::get(context, {refTy, refTy}, refTy);
+  };
+}
+template <>
+constexpr TypeBuilderFunc getModel<
+    Fortran::runtime::ReferenceReductionOperation<std::complex<double>>>() {
+  return [](mlir::MLIRContext *context) -> mlir::Type {
+    mlir::Type cplx = mlir::ComplexType::get(getModel<double>()(context));
+    auto refTy = fir::ReferenceType::get(cplx);
+    return mlir::FunctionType::get(context, {refTy, refTy}, refTy);
+  };
+}
 
 REDUCTION_CHAR_OPERATION_MODEL(char)
 REDUCTION_CHAR_OPERATION_MODEL(char16_t)
diff --git a/flang/unittests/Optimizer/RTBuilder.cpp b/flang/unittests/Optimizer/RTBuilder.cpp
index d6cf96c4351c2b..2ccc4353f9ee40 100644
--- a/flang/unittests/Optimizer/RTBuilder.cpp
+++ b/flang/unittests/Optimizer/RTBuilder.cpp
@@ -9,6 +9,7 @@
 #include "flang/Optimizer/Builder/Runtime/RTBuilder.h"
 #include "gtest/gtest.h"
 #include "flang/Optimizer/Support/InitFIR.h"
+#include <complex>
 
 // Check that it is possible to make a difference between complex runtime
 // function using C99 complex and C++ std::complex. This is important since

[jeanPerier]

@DavidTruby, I went with your suggestion and just removed the std::complex value support all together. The REDUCE implementation for complex functions needs to be fixed independently.

[clementval]

LGTM