Support calling assumed length dummy procedure #1393

jeanPerier · 2022-01-18T16:15:55Z

Lowering was not able to compile the following kind of program:

subroutine foo(bar)
 character*(*) :: bar
 print *, bar()
end subroutine

character(10) function bar()
  bar = "abcdefghijkl"
end function

program test
  character(10) :: bar
  external :: bar
  call foo(bar)
end

This resulted in error missing length for character.

To support this kind of program, pass the result character dummy procedure as
character dummy objects: with its length. This matches nvfortran, ifort,
nag and xlf behavior (but not gfortran that does not support the above
code).

This is done by using fir.boxchar. The rational for using it instead of
manually mangling the length argument is that in call like
foo(c1, bar, c2) with c1 and c2 F77 like characters, the result
length of bar must be placed between the one of c1 and c2 after
all other arguments. fir.boxchar guarantees exactly that.

Three places are impacted:

The function interface lowering (mustPassDummyProcedureAsBoxChar is
the central place that can completly disable this feature On and Off).
When placing a call to a dummy procedure, use the fir.boxchar length
as a last resort if present in the interface.
When passing a procedure designator, forward the length for character
function based on mustPassDummyProcedureAsBoxChar.

psteinfeld

A few nits ...

I can confirm that, aside from the nits, all builds, tests, and looks good. But you should probably get someone more familiar with the code to approve.

flang/include/flang/Lower/CallInterface.h

flang/lib/Lower/CallInterface.cpp

flang/lib/Lower/ConvertExpr.cpp

flang/lib/Lower/CallInterface.cpp

schweitzpgi · 2022-01-18T23:54:44Z

flang/lib/Lower/CallInterface.cpp

+    // array function with assumed length (f18 forbides defining such
+    // interfaces). Hence, passing the length is most likely useless, but stick
+    // with ifort/nag/xlf interface here.
+    if (std::optional<Fortran::evaluate::DynamicType> type =


OK, but this is really pretty strange. Why wouldn't the argument have the type () -> !fir.boxchar<k> or (!fir.ref<!fir.char<k,l>>, index) -> !fir.boxchar<k> (since a character return value is also a hidden argument)? The actual argument must be a function that returns a boxchar, right? It can never just be a value of boxchar type (which cannot be called).

How would () -> !fir.boxchar<k> carry the length value on top of the function address ? Are you thinking of making () -> !fir.boxchar<k> a tuple like fir.boxchar under the hood ? Why not, but then we would need special op to create these addresses + length tuple like emboxchar and a way to extract the length.

flang/lib/Lower/ConvertExpr.cpp

flang/test/Lower/dummy-procedure-character.f90

schweitzpgi · 2022-01-19T00:07:05Z

flang/test/Lower/dummy-procedure-character.f90

+  end interface
+! CHECK:  %[[VAL_0:.*]] = fir.address_of(@_QPbar1) : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:  %[[VAL_1:.*]] = arith.constant 7 : i64
+! CHECK:  %[[VAL_2:.*]] = fir.convert %[[VAL_0]] : ((!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>) -> !fir.ref<!fir.char<1,?>>


This cast is lossy and odd looking to me. It is lossy in that we have a function pointer and are converting it to a data pointer, which loses information that the pointer is to code and not data. Subsequently, the IR has no way to reason about this value correctly.

Makes sense, what do you see as an alternative here ? It is currently not possible to create a fir.embox_char with a function address. Should we allow that to avoid the cast or should we create a new operation here (other alternatives welcomed if you see any) ?

jeanPerier

Thanks @schweitzpgi and @psteinfeld for the review. I have updated all the code comments and applied the small modifications.

@schweitzpgi I understand you are against using fir.boxchar as a tuple <function address , result length if know in the siganture> ? I can look into using an actual mlir tuple type here to keep the function type visible in the tuple. However this will require deeper change: A new case to handle in argument lowering and a modification in codegen to deal with the tuple like fir.boxchar<> (I liked the idea of using fir.boxchar to avoid having to add special handling in codegen). Does that sounds a better solution to you ?

flang/include/flang/Lower/CallInterface.h

flang/lib/Lower/CallInterface.cpp

jeanPerier · 2022-01-19T11:59:23Z

flang/lib/Lower/CallInterface.cpp

+    // array function with assumed length (f18 forbides defining such
+    // interfaces). Hence, passing the length is most likely useless, but stick
+    // with ifort/nag/xlf interface here.
+    if (std::optional<Fortran::evaluate::DynamicType> type =


How would () -> !fir.boxchar<k> carry the length value on top of the function address ? Are you thinking of making () -> !fir.boxchar<k> a tuple like fir.boxchar under the hood ? Why not, but then we would need special op to create these addresses + length tuple like emboxchar and a way to extract the length.

flang/lib/Lower/ConvertExpr.cpp

flang/test/Lower/dummy-procedure-character.f90

jeanPerier · 2022-01-21T16:52:03Z

@schweitzpgi , I have updated the PR to lower the character dummy procedure argument to a tuple<function address, length value> and added an optimizer patch that takes care of splitting this like with characters (It is in its own commit and has its own test so that we can upstream it directly).

Note that the function address type in the tuple remains poor (() -> ()) because so far we do not type dummy procedure as good as we could (this is not directly related to this PR:

f18-llvm-project/flang/lib/Lower/CallInterface.cpp

Line 38 in 94aa416

// TODO: Get actual function type of the dummy procedure, at least when an

).

jeanPerier · 2022-01-24T10:11:56Z

Rebased to solve conflicts and added comments in Character.h.

schweitzpgi

Gave it a quick look. LGTM.

When passing a character procedure as a dummy procedure, the result length must be passed along the function address. This is to cover the cases where the dummy procedure is declared with assumed length inside the scope that will call it (it will need the length to allocate the result on the caller side). To be compatible with other Fortran compiler, this length must be appended after all other argument just like character objects (fir.boxchar). A fir.boxchar cannot be used to implement this feature because it is meant to take an object address, not a function address. Instead, argument like `tuple<function type, integer type> {fir.char_proc}` will be recognized as being character dummy procedure in FIR. That way lowering does not have to do the argument split. This patch adds tools in Character.h to create this type and tuple values as well as to recognize them and extract its tuple members. It also updates the target rewrite pass to split these arguments like fir.boxchar. See: #1393

Lowering was not able to compile the following kind of program: ``` subroutine foo(bar) character*(*) :: bar print *, bar() end subroutine ``` This resulted in error `missing length for character`. To support this kind of program, pass the result character dummy procedure as character dummy objects: with its length. This matches nvfortran, ifort, nag and xlf behavior (but not gfortran that does not support the above code). This is done by using the tuple<function address, length> added in the Optimnizer to implement character dummy procedure and let codegen split and order the argument so that in call like `foo(c1, bar, c2)` with `c1` and `c2` F77 like characters, the result length of `bar` is placed between the one of `c1` and `c2` after all other arguments. Three places are impacted: - The function interface lowering (mustPassDummyProcedureAsBoxChar is the central place that can completely disable this feature On and Off). - When placing a call to a dummy procedure, use the tuple length as a last resort if present in the interface. - When passing a procedure designator, forward the length for character function based on mustPassDummyProcedureAsBoxChar. See: #1393

When passing a character procedure as a dummy procedure, the result length must be passed along the function address. This is to cover the cases where the dummy procedure is declared with assumed length inside the scope that will call it (it will need the length to allocate the result on the caller side). To be compatible with other Fortran compiler, this length must be appended after all other argument just like character objects (fir.boxchar). A fir.boxchar cannot be used to implement this feature because it is meant to take an object address, not a function address. Instead, argument like `tuple<function type, integer type> {fir.char_proc}` will be recognized as being character dummy procedure in FIR. That way lowering does not have to do the argument split. This patch adds tools in Character.h to create this type and tuple values as well as to recognize them and extract its tuple members. It also updates the target rewrite pass to split these arguments like fir.boxchar. See: flang-compiler#1393

When passing a character procedure as a dummy procedure, the result length must be passed along the function address. This is to cover the cases where the dummy procedure is declared with assumed length inside the scope that will call it (it will need the length to allocate the result on the caller side). To be compatible with other Fortran compiler, this length must be appended after all other argument just like character objects (fir.boxchar). A fir.boxchar cannot be used to implement this feature because it is meant to take an object address, not a function address. Instead, argument like `tuple<function type, integer type> {fir.char_proc}` will be recognized as being character dummy procedure in FIR. That way lowering does not have to do the argument split. This patch adds tools in Character.h to create this type and tuple values as well as to recognize them and extract its tuple members. It also updates the target rewrite pass to split these arguments like fir.boxchar. See: #1393

jeanPerier · 2022-01-25T08:43:56Z

Patch for optimizer related change in LLVM: https://reviews.llvm.org/D118108

When passing a character procedure as a dummy procedure, the result length must be passed along the function address. This is to cover the cases where the dummy procedure is declared with assumed length inside the scope that will call it (it will need the length to allocate the result on the caller side). To be compatible with other Fortran compiler, this length must be appended after all other argument just like character objects (fir.boxchar). A fir.boxchar cannot be used to implement this feature because it is meant to take an object address, not a function address. Instead, argument like `tuple<function type, integer type> {fir.char_proc}` will be recognized as being character dummy procedure in FIR. That way lowering does not have to do the argument split. This patch adds tools in Character.h to create this type and tuple values as well as to recognize them and extract its tuple members. It also updates the target rewrite pass to split these arguments like fir.boxchar. This part is part of fir-dev upstreaming. It was reviwed previously in: flang-compiler#1393 Differential Revision: https://reviews.llvm.org/D118108

When passing a character procedure as a dummy procedure, the result length must be passed along the function address. This is to cover the cases where the dummy procedure is declared with assumed length inside the scope that will call it (it will need the length to allocate the result on the caller side). To be compatible with other Fortran compiler, this length must be appended after all other argument just like character objects (fir.boxchar). A fir.boxchar cannot be used to implement this feature because it is meant to take an object address, not a function address. Instead, argument like `tuple<function type, integer type> {fir.char_proc}` will be recognized as being character dummy procedure in FIR. That way lowering does not have to do the argument split. This patch adds tools in Character.h to create this type and tuple values as well as to recognize them and extract its tuple members. It also updates the target rewrite pass to split these arguments like fir.boxchar. This part is part of fir-dev upstreaming. It was reviwed previously in: flang-compiler/f18-llvm-project#1393 Differential Revision: https://reviews.llvm.org/D118108

jeanPerier requested review from schweitzpgi, vdonaldson and psteinfeld January 18, 2022 16:15

psteinfeld reviewed Jan 18, 2022

View reviewed changes

flang/include/flang/Lower/CallInterface.h Outdated Show resolved Hide resolved

flang/lib/Lower/CallInterface.cpp Outdated Show resolved Hide resolved

flang/lib/Lower/ConvertExpr.cpp Outdated Show resolved Hide resolved

schweitzpgi reviewed Jan 19, 2022

View reviewed changes

jeanPerier commented Jan 19, 2022

View reviewed changes

jeanPerier force-pushed the jpr-dummy-char-proc branch from 061f1c4 to 15deaf4 Compare January 24, 2022 10:11

schweitzpgi approved these changes Jan 24, 2022

View reviewed changes

jeanPerier added 2 commits January 24, 2022 23:34

jeanPerier force-pushed the jpr-dummy-char-proc branch from 15deaf4 to a306179 Compare January 25, 2022 07:35

jeanPerier merged commit 7dc8e20 into fir-dev Jan 25, 2022

jeanPerier deleted the jpr-dummy-char-proc branch January 25, 2022 08:43

jeanPerier mentioned this pull request Jan 31, 2022

Fix character procedure dummy capture in host procedure caused by 1393 #1427

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support calling assumed length dummy procedure #1393

Support calling assumed length dummy procedure #1393

jeanPerier commented Jan 18, 2022

psteinfeld left a comment

schweitzpgi Jan 18, 2022

jeanPerier Jan 19, 2022

schweitzpgi Jan 19, 2022

jeanPerier Jan 19, 2022

jeanPerier left a comment

jeanPerier Jan 19, 2022

jeanPerier commented Jan 21, 2022

jeanPerier commented Jan 24, 2022

schweitzpgi left a comment

jeanPerier commented Jan 25, 2022

Support calling assumed length dummy procedure #1393

Support calling assumed length dummy procedure #1393

Conversation

jeanPerier commented Jan 18, 2022

psteinfeld left a comment

Choose a reason for hiding this comment

schweitzpgi Jan 18, 2022

Choose a reason for hiding this comment

jeanPerier Jan 19, 2022

Choose a reason for hiding this comment

schweitzpgi Jan 19, 2022

Choose a reason for hiding this comment

jeanPerier Jan 19, 2022

Choose a reason for hiding this comment

jeanPerier left a comment

Choose a reason for hiding this comment

jeanPerier Jan 19, 2022

Choose a reason for hiding this comment

jeanPerier commented Jan 21, 2022

jeanPerier commented Jan 24, 2022

schweitzpgi left a comment

Choose a reason for hiding this comment

jeanPerier commented Jan 25, 2022