[PATCH] D83397: [flang] Replace uses of _Complex with std::complex

[Jean Perier via Phabricator via llvm-commits]

jeanPerier requested changes to this revision.
jeanPerier added a comment.
This revision now requires changes to proceed.

Sorry I was harvesting potatoes far from any computers when all these discussions happened. First of all, thanks for detecting the warning and proposing a fix for it. I will try to explain our issue and why we cannot use safely `std::complex` here so we can find a fix for this.

1. We want to have the possibility to target the pgmath runtime library that is open sourced here: https://github.com/flang-compiler/flang/tree/master/runtime/libpgmath. It is written in C99 and uses `_Complex` at the interface of its complex functions (you are absolutely right to call out windows issues here, pgmath actually uses `_Fcomplex` there, and we should at least align in that case to help supporting windows support in flang).
2. We want to both be able to generate code calling this runtime and be able to call this very runtime at compile time while folding for best numerical stability (at least when host and target are the same). So we need flang to safely link with pgmath on top of emitting code for it (You can actually already do that if you have a pgmath library under the hands by using `-DLIBPGMATH_DIR=<path to pgmath lib>` in your flang cmake command. This cmake flag is only advertised in flang/documentation/Intrinsics.md so far).
3. `_Complex` and `std::complex` **are indeed layout compatible, but they are not guaranteed to be compatible in call interface**. `std::complex<>` is an aggregate type that is opaque to the compiler, `_Complex` is a fundamental type for which some ABIs have different interface requirements. So we cannot declare pgmath functions in flang with `std::complex`. More justification below.

**Proposal fix: **
My proposal is therefore to follow libpgmath handling and define `float_complex_t`/ `double_complex_t` alias that will be `_Complex` on platform that supports it (disabling the warning in such cases) and _Fcomplex/_Dcomplex on windows (@echristo, I am not aware of compilers other than MSVC that do not support _Complex (but I honestly have little experience outside of gcc/clang) do you know of any ? I am not sure what to do in case _Complex is not supported and this is not MSVC. The solution I see would be to abort compilation if such platform/compiler exists and add correct _Complex replacement in this context.

Does that seems right to you ?

**Justification of 3.** :

@DavidTruby, your example happens to work on all architecture I know of but is not defined by the C++ standard (so that is a happy accident ;) ). As documented in this comment hidden in flang code: https://github.com/llvm/llvm-project/blob/master/flang/lib/Evaluate/intrinsics-library.cpp#L239, had you written a function returning a complex by value, I could actually have pointed at least X86 as an architecture where your example would crash or return an incorrect value. Many of the pgmath functions are returning complex values.

You do not have to trust me on this, you can just try throwing the following code to clang 10:

  #include <complex>
  extern "C" std::complex<float> conjugate(std::complex<float>);

You will get `warning: 'conjugate' has C-linkage specified, but returns user-defined type 'std::complex<float>' which is incompatible with C [-Wreturn-type-c-linkage]`.
That is because linkage and layout compatibility are not the same thing. Layout compatibility can only be brought-up when things are in memory, but to my knowledge, nothing in the C99 standard says that `_Complex` have to be passed/returned in memory, this choice is left to the target architecture ABIs. For instance, if you have a look at the System V i386 ABI (https://uclibc.org/docs/psABI-i386.pdf) page 14, table 2.4, you will read regarding `float _Complex` :  "The real part is returned in %eax. The imaginary part is returned in %edx.". However, `std::complex<float>` which is a user defined class (from the compiler point of view, STL types are not very different from user defined types) must be returned in memory (page 12: "Aggregate types (structs and unions) are always returned in memory.").
You can check on https://godbolt.org/ that the following program in with x86-64 clang 10 and -O2 -m32 flags yields different (incompatible) assembly for foo_cpp and foo_c99:

  #include <complex>
  extern "C" std::complex<float> foo_cpp(std::complex<float> x) {
   return x;
  }
  extern "C" float _Complex foo_c99(float _Complex x) {
   return x;
  }

output:

  foo_cpp:                                # @foo_cpp
          movl    4(%esp), %eax
          movsd   8(%esp), %xmm0          # xmm0 = mem[0],zero
          movsd   %xmm0, (%eax)
          retl    $4
  foo_c99:                                # @foo_c99
          movl    4(%esp), %eax
          movl    8(%esp), %edx
          retl


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83397/new/

https://reviews.llvm.org/D83397