Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][RFC] Adding a design document for assumed-rank objects #71959

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

jeanPerier
Copy link
Contributor

This patch adds a document describing assumed-rank objects and the related features as well as how they will be implemented in Flang.

This patch adds a document describing assumed-rank objects and the
related features as well as how they will be implemented in Flang.
@llvmbot llvmbot added the flang Flang issues not falling into any other category label Nov 10, 2023
Copy link
Contributor

@klausler klausler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, ship it.

flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
flang/docs/AssumedRank.md Outdated Show resolved Hide resolved

An assumed-rank is a data object that takes its rank from its effective
argument. It is a dummy, or the associated entity of a SELECT RANK in the `RANK
DEFAULT` or `RANK(*)` blocks. Its rank is not known at compile time. The rank
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just RANK DEFAULT. In a RANK(*) block, the construct entity is not assumed-rank, but rather an assumed-size vector with bounds (1:*).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I read the RANK(*) case too fast. I updated the descriptions and example.

Note that my understanding is that the descriptor created when passing an assumed-size actual to an assumed-rank dummy should still have the assumed-size rank (not forced to 1 at that point), and that it is only when reaching a RANK(*) that the original rank of the assumed-size should be "lost".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please think about the case where an assumed-size array is associated with an assumed-rank dummy argument that is later forwarded to another assumed-size dummy argument. When will the rank change then? I seems best to change the rank to 1 at the first call; it reduces overall code size in this case, and makes the check for RANK(*) in SELECT RANK perhaps faster -- look for rank 1 and upper bound of -1.

Copy link
Contributor Author

@jeanPerier jeanPerier Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seems best to change the rank to 1 at the first call;

I agree but is it legal?

I think this would prevent RANK(x) to return the rank of the effective argument of the assumed rank as requested by the standard (there is no special case for assumed-rank associated with assumed-size in RANK(), nor in 8.5.8.7).
UBOUND and al. also does not say that an assumed-rank associated to an assumed-size has rank one (only mention a special case for the "final dimension").

I think we can only change it in the RANK(*), even if it makes all SELECT RANK checking a bit more expensive (especially since we must not fall in RANK(cst) for an assumed-size even if there is not RANK(*)).

All other compilers that can compile the program below seem to keep the rank of the assumed-size attached to the assumed-rank:

module m
contains
subroutine print_my_rank(x)
  real :: x(..)
  print *, rank(x)
end subroutine

subroutine test(x1, x2, x3)
 real :: x1(*)
 real :: x2(1, *)
 real :: x3(1, 1, *)
 call print_my_rank(x1)
 call print_my_rank(x2)
 call print_my_rank(x3)
end subroutine
end module

 use m
 real :: x1(1)
 real :: x2(1, 1)
 real :: x3(1, 1, 1)
 call test(x1, x2, x3)
 end
```

Prints "1,2,3".

flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
flang/docs/AssumedRank.md Show resolved Hide resolved
flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
flang/docs/AssumedRank.md Show resolved Hide resolved
flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
- update RANK(*) description and example with the understanding that
  the related associate entity has rank one (instead of being an
  assumed-rank as previously described).
- update C839 constraint description
- various typo corrections, and double spaces elimination.
maximized using the maximum rank, which the runtime code already does when
creating temporary descriptor in many cases. Inline code also needs care if it
needs to access the descriptor addendum (like the type descriptor), since its
offset will not be a compile time constant as usual.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it also affect privatization in OpenMP or OpenACC? We always create a private descriptor with maximum rank.

subroutine f(x)
  real :: x(..)
!$omp parallel private(x)
  select rank (z => x)
  rank(2)
    x(i,j) = ...
  rank(3)
    x(i,j,k) = ...
  end select
!$omp end parallel
end

How about the implication for mapping?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will indeed need to create a private descriptor with maximum rank and pay attention to not blindly copy the whole incoming descriptor as if it has the maximum rank (the incoming descriptor storage may be smaller since it was allocated with the actual rank knowledge).

Is this happening in createHostAssociateVarClone inside Bridge.cpp, or is this private copy made later in some OpenMP IR pass?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An advise here on the implementation will be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An advise here on the implementation will be useful.

It depends where this is implemented: is this abstracted in the MLIR and only made explicit in when generating LLVM IR, or is the copy already created in lowering in createHostAssociateVarClone?

If it is in createHostAssociateVarClone, the burden of the copy complexity will be borne by the fir.load/fir.store operations. So you can do an copy of the descriptor in the IR as:

  %new_box_addr = fir.alloca !fir.box<fir.array<..xT>> ! allcoates maximum ranked descriptor storage
  %fir.store %old_box to %new_box_addr ! does a "non trivial" dynamic copy

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this RFC. I have a few minor points/questions.

-> Does this have an effect on the processing of the ignore_tkr directive?
-> I think the new mpi module (mpi_f08) uses assumed rank arrays. Some early references talk about assumed-rank as a request from MPI (https://j3-fortran.org/doc/year/09/09-100.txt, https://j3-fortran.org/doc/year/08/08-271.txt). Might be reasonable test for the feature.
-> Please add the document to the index file (https://github.com/llvm/llvm-project/blob/main/flang/docs/index.md).

flang/docs/AssumedRank.md Outdated Show resolved Hide resolved
@jeanPerier
Copy link
Contributor Author

Does this have an effect on the processing of the ignore_tkr directive?

Good question. No more than when passing to an assumed-shape.

  • It is forbidden (and pointless) to use IGNORE_TKR(r) on an assumed rank dummy
  • It is possible to use IGNORE_TKR(tk) on an assumed rank dummy (but highly preferable to use TYPE(*)). In which case on caller side, the dynamic type information in the descriptor will be set according to the actual. But just like with assumed shapes, if the procedure with ignore_tkr is implemented in Fortran, it is quite unspecified what type it will be considered to have inside the procedure. Any direct usages of the incoming descriptor in the Fortran runtime will behave according to the actual argument type, while any new inline and new descriptor created will use the dummy type information.
  • It is possible to pass an assumed-rank object to an explicit shape/assumed-size array dummy that has IGNORE_TKR(R). In which case, lowering will do copy_in/copy_out as when passing to a contiguous assumed-rank, and then pass only the base address instead of the descriptor.

I think the new mpi module (mpi_f08) uses assumed rank arrays.

Indeed, there is mention of assumed rank in the MPI standard https://www.mpi-forum.org/docs/mpi-3.1/mpi31-report.pdf
I added a note in annex about using mpi_f08 to test for this feature, thanks for pointing to it.

Please add the document to the index file (https://github.com/llvm/llvm-project/blob/main/flang/docs/index.md).

Done, thanks.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@jeanPerier jeanPerier merged commit 915f6c3 into llvm:main Nov 17, 2023
4 checks passed
@jeanPerier jeanPerier deleted the jpr-assumed-rank-design branch November 17, 2023 10:10
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
…71959)

This patch adds a document describing assumed-rank objects and the
related features as well as how they will be implemented in Flang.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…71959)

This patch adds a document describing assumed-rank objects and the
related features as well as how they will be implemented in Flang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants