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

[Jean Perier via Phabricator via flang-commits]

jeanPerier added a comment.

In this very spot, the goal of the template is to digest the runtime headers in order to lower their function signature to an `mlir::FuncOp` declaration that is compatible with the runtime and will be call in user programs.
So if we use `std::complex` here and introduce a wrapper indirection in C around the runtime, then the cost of this indirection will be paid by the user program. I am not comfortable with having a Fortran programmer pay the price of a C/C++ compatibility issues.

However, you are absolutely correct that in this very code spot we are discussing, using `_Complex` is not essential. All we need to know "this runtime interface is using C99 complex", so we could define a type like  `Fortran::common::C99complex<T>` instead, and  use it as a tag here.

In folding the the C99 `_Complex` functions are called from C++ the code around the comment I linked above is dealing with this interface (or was, since part of it was reverted). Your wrappers would be a solution here given folding performance is not as critical, but given folding with pgmath is the non-default option (under ifdef, I think it is easier to only enable it if `_Complex` of `_FComplex` is available and resort to the default usage of cmath in the other cases.

Thanks for pointing out we do not really use the interface in this very spot ! I think I can now put up a solution that should fulfill all wishes:

- no warnings, compiles on all C++ compliant compiler.
- no silent undefined behavior for X86_32 and the whatever other ABI out there deals with _Complex in a special way
- no user program performance hit
- still possible to fold with pgmath if you are using a mainstream C++ compiler to compile flang (g++, clang++, msvc when possible)
- no hard error if using pgmath complex functions is not possible
- not too much additional code complexity in flang or the runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83397