This is an archive of the discontinued LLVM Phabricator instance.

[fir] TargetRewrite: Rewrite COMPLEX values
ClosedPublic

Authored by rovka on Nov 5 2021, 5:40 AM.

Details

Summary

Rewrite function signatures and calls to functions that accept or return
COMPLEX values.

This patch is part of the effort for upstreaming the fir-dev branch.

Co-authored-by: Eric Schweitz <eschweitz@nvidia.com>
Co-authored-by: Kiran Chandramohan <kiran.chandramohan@arm.com>
Co-authored-by: Tim Keith <tkeith@nvidia.com>
Co-authored-by: Jean Perier <jperier@nvidia.com>

Diff Detail

Event Timeline

rovka created this revision.Nov 5 2021, 5:40 AM
rovka requested review of this revision.Nov 5 2021, 5:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2021, 5:40 AM
rovka added inline comments.Nov 5 2021, 5:45 AM
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
263

I tried to add a test with mlir::ComplexType, but it told me that the dialect is not registered. It doesn't seem to be registered in fir-dev AFAICT (I hope I'm looking in the right place). Does flang ever actually use mlir::ComplexType, or should I remove this code path?

kiranchandramohan added inline comments.
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
263

Flang seems to use mlir::Complex while generating runtime calls. See code in https://github.com/flang-compiler/f18-llvm-project/blob/3aa4ab214082e718804d431139f59eb45df1f71d/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h#L224

complex function sum_test3(a)
  complex :: a(:)
  sum_test3 = sum(a)
end function

The MLIR generated for the above test contains the MLIR complex<f32> type.

func @_QPsum_test3(%arg0: !fir.box<!fir.array<?x!fir.complex<4>>>) -> !fir.complex<4> {
  %0 = fir.alloca !fir.complex<4>
  %1 = fir.alloca !fir.complex<4> {bindc_name = "sum_test3", uniq_name = "_QFsum_test3Esum_test3"}
  %2 = fir.absent !fir.box<i1>
  %c0 = arith.constant 0 : index
  %3 = fir.address_of(@_QQcl.2E2F7463312E66393000) : !fir.ref<!fir.char<1,10>>
  %c3_i32 = arith.constant 3 : i32
  %4 = fir.convert %0 : (!fir.ref<!fir.complex<4>>) -> !fir.ref<complex<f32>>
  %5 = fir.convert %arg0 : (!fir.box<!fir.array<?x!fir.complex<4>>>) -> !fir.box<none>
  %6 = fir.convert %3 : (!fir.ref<!fir.char<1,10>>) -> !fir.ref<i8>
  %7 = fir.convert %c0 : (index) -> i32
  %8 = fir.convert %2 : (!fir.box<i1>) -> !fir.box<none>
  %9 = fir.call @_FortranACppSumComplex4(%4, %5, %6, %c3_i32, %7, %8) : (!fir.ref<complex<f32>>, !fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> none
  %10 = fir.load %0 : !fir.ref<!fir.complex<4>>
  fir.store %10 to %1 : !fir.ref<!fir.complex<4>>
  %11 = fir.load %1 : !fir.ref<!fir.complex<4>>
  return %11 : !fir.complex<4>
}
func private @_FortranACppSumComplex4(!fir.ref<complex<f32>>, !fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> none attributes {fir.runtime}
fir.global linkonce @_QQcl.2E2F7463312E66393000 constant : !fir.char<1,10> {
  %0 = fir.string_lit "./tc1.f90\00"(10) : !fir.char<1,10>
  fir.has_value %0 : !fir.char<1,10>
}

I guess @jeanPerier @schweitz or @mleair can explain the usage better.

rovka added a comment.Nov 7 2021, 11:46 PM

Good point, thanks for the link. I see that upstream it's fir::ComplexType: https://github.com/llvm/llvm-project/blob/9b6f264d2b09ab5e970e54f77119eb823f0fcc50/flang/lib/Lower/RTBuilder.h#L169
The IR you posted is confusing because it contains both complex<f32> and fir.complex<4>. Are both valid, or are we moving away from one of them?

The IR you posted is confusing because it contains both complex<f32> and fir.complex<4>. Are both valid, or are we moving away from one of them?

Yes they are both valid types, but are not exactly the same thing. complex<f32> is the mlir complex type that is used to represent C++ std::complex types, while fir.complex<4> represents Fortran complex type. The nuance is subtle but essential because C++ std::complex and Fortran complex type (same as C99 complex) may not be compatible in function interface [*], so we want to keep them as distinct types in function signatures.

It is OK to cast a fir.ref<complex<f32>> to a fir.ref<fir.complex<4> because std::complex and Fortran complex are memory layout compatible (since std::complex guarantees this with the C99 complex type).

  • For instance Fortran complex and std::complex are not returned the same way on X86 32 bits.

Good point, thanks for the link. I see that upstream it's fir::ComplexType: https://github.com/llvm/llvm-project/blob/9b6f264d2b09ab5e970e54f77119eb823f0fcc50/flang/lib/Lower/RTBuilder.h#L169
The IR you posted is confusing because it contains both complex<f32> and fir.complex<4>. Are both valid, or are we moving away from one of them?

It seems the upstream llvm-project/flang link that you are pointing to here refers to getModel<c_float_complex_t>(). You can see that the fir-dev branch also uses fir::ComplexType for this function. It is for getModel<std::complex<float>>() that fir-dev uses mlir::ComplexType().
https://github.com/flang-compiler/f18-llvm-project/blob/3aa4ab214082e718804d431139f59eb45df1f71d/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h#L238

Regarding your question, I guess the FIR core team can provide an authoritative answer.

rovka added a comment.Nov 8 2021, 3:08 AM

Ok, that makes sense now, thank you both for the comments! I'll try to add some tests with MLIR complex as well.

rovka updated this revision to Diff 385566.Nov 8 2021, 10:50 AM

Added test for MLIR complex.
Note that I also had to teach insert_value about it.

clementval accepted this revision.Nov 9 2021, 12:08 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Nov 9 2021, 12:08 AM
This revision was automatically updated to reflect the committed changes.