Page MenuHomePhabricator

[flang] Add runtime interface for NUM_IMAGES
AcceptedPublic

Authored by craig.rasmussen on Sep 9 2021, 2:52 PM.

Details

Summary

NUM_IMAGES takes optional arguments TEAM and TEAM_NUMBER.

This patch provides an interface for only the optional TEAM_NUMBER dummy argument. It is likely that a different function will be required to support TEAM arguments.

Diff Detail

Event Timeline

craig.rasmussen requested review of this revision.Sep 9 2021, 2:52 PM
craig.rasmussen created this revision.
craig.rasmussen added inline comments.Sep 9 2021, 3:14 PM
flang/runtime/coarray.h
21 ↗(On Diff #371727)

I think the default (team_number = 0) should work here because when forming a new team (11.6.9 FORM TEAM statement) the team_number has to be greater than 0. In 9.6 paragraph 3 the team number has to be from a FORM TEAM statement or be the initial team. In 16.9.189 TEAM_NUMBER paragraph 5 it states that the initial team shall have the value -1.

So team_number==0 isn't a valid team number and can represent NUM_IMAGES called with no actual arguments.

PeteSteinfeld added inline comments.Sep 10 2021, 12:38 PM
flang/runtime/coarray.h
1 ↗(On Diff #371727)

Note that patch https://reviews.llvm.org/D109107 rearranged the location of the runtime header files. So the correct location for coarray.h is now .../flang/include/flang/Runtime.

15 ↗(On Diff #371727)

With the new file organization, the #include statement for entry-names.h is

#include "flang/Runtime/entry-names.h"
21 ↗(On Diff #371727)

The interface looks OK to me for two of the three forms of NUM_IMAGES. We'll probably want a second interface for the NUM_IMAGES(TEAM) version. Also, your comment about using a team number of 0 to indicate the current team should go into a comment in the code rather than in the Phabricator review.

Changed location of header file to flang/include/flang/Runtime/coarray.h.

Added comments explaining why team_number=0 is a good default value when NUM_IMAGES is called without an actual argument for TEAM_NUMBER.

PeteSteinfeld accepted this revision.Sep 13 2021, 2:55 PM

So far, so good.

As I commented earlier, I suspect that we'll need an additional interface to cover all forms of NUM_IMAGES().

This revision is now accepted and ready to land.Sep 13 2021, 2:55 PM
klausler added inline comments.Sep 13 2021, 3:06 PM
flang/runtime/coarray.h
21 ↗(On Diff #371727)

I think the default (team_number = 0) should work here because when forming a new team (11.6.9 FORM TEAM statement) the team_number has to be greater than 0. In 9.6 paragraph 3 the team number has to be from a FORM TEAM statement or be the initial team. In 16.9.189 TEAM_NUMBER paragraph 5 it states that the initial team shall have the value -1.

So team_number==0 isn't a valid team number and can represent NUM_IMAGES called with no actual arguments.

Or you could have two entry points, one with a team number and one without, and avoid exposing this kind of trick in the API. It could still be used as a hidden part of a common implementation.

Peter Klausler wrote:

Or you could have two entry points, one with a team number and one without, and avoid exposing this kind of trick in the API. It could still be used as a hidden part of a common implementation.

I can revisit this when I add a TEAM type dummy (probably after you complete the type magic in the front end). For now this allows me to implement NUM_IMAGES for a single image. Possible function names: NumImages, NumImagesTeam NumImagesTeamNum.

As you say, the implementation can do whatever it wants.

-craig

From: Peter Klausler via Phabricator <reviews@reviews.llvm.org>
Date: Monday, September 13, 2021 at 3:06 PM
To: Rasmussen, Craig E. <rasmussen17@llnl.gov>, sscalpone@nvidia.com <sscalpone@nvidia.com>, psteinfeld@nvidia.com <psteinfeld@nvidia.com>, pklausler@nvidia.com <pklausler@nvidia.com>
Cc: tim@tkeith.com <tim@tkeith.com>
Subject: [PATCH] D109547: [flang] Add runtime interface for NUM_IMAGES
klausler added inline comments.

Comment at: flang/runtime/coarray.h:21
+// 16.9.145 NUM_IMAGES
+int RTNAME(NumImages)(int team_number = 0,

+ const char *sourceFile = nullptr, int sourceLine = 0);

craig.rasmussen wrote:

I think the default (team_number = 0) should work here because when forming a new team (11.6.9 FORM TEAM statement) the team_number has to be greater than 0. In 9.6 paragraph 3 the team number has to be from a FORM TEAM statement or be the initial team. In 16.9.189 TEAM_NUMBER paragraph 5 it states that the initial team shall have the value -1.

So team_number==0 isn't a valid team number and can represent NUM_IMAGES called with no actual arguments.
I think the default (team_number = 0) should work here because when forming a new team (11.6.9 FORM TEAM statement) the team_number has to be greater than 0. In 9.6 paragraph 3 the team number has to be from a FORM TEAM statement or be the initial team. In 16.9.189 TEAM_NUMBER paragraph 5 it states that the initial team shall have the value -1.

So team_number==0 isn't a valid team number and can represent NUM_IMAGES called with no actual arguments.

Or you could have two entry points, one with a team number and one without, and avoid exposing this kind of trick in the API. It could still be used as a hidden part of a common implementation.

CHANGES SINCE LAST ACTION

https://urldefense.us/v3/__https://reviews.llvm.org/D109547/new/__;!!G2kpM7uM-TzIFchu!neIxNJ4lJsWyAIvx6heTGuRQA6DGC8sGAzRI639eyb5fm7Gj4uXoj6rpMY6Nj4XCIU8K$<https://urldefense.us/v3/__https:/reviews.llvm.org/D109547/new/__;!!G2kpM7uM-TzIFchu!neIxNJ4lJsWyAIvx6heTGuRQA6DGC8sGAzRI639eyb5fm7Gj4uXoj6rpMY6Nj4XCIU8K$>

https://urldefense.us/v3/__https://reviews.llvm.org/D109547__;!!G2kpM7uM-TzIFchu!neIxNJ4lJsWyAIvx6heTGuRQA6DGC8sGAzRI639eyb5fm7Gj4uXoj6rpMY6Nj3z2uaZ1$https://urldefense.us/v3/__https:/reviews.llvm.org/D109547__;!!G2kpM7uM-TzIFchu!neIxNJ4lJsWyAIvx6heTGuRQA6DGC8sGAzRI639eyb5fm7Gj4uXoj6rpMY6Nj3z2uaZ1$

MehdiChinoune added a project: Restricted Project.Sep 15 2021, 11:48 AM
MehdiChinoune added a subscriber: flang-commits.

Inlined call to Fortran::semantics::IsCoarray (one line change) to fix link error.

Also minor cleanup suggested by clang-format.

Hopefully this will open the issue and is the correct way to correct a push that was reverted.

Note comment on line 97 of flang/include/flang/Evaluate/tools.h. This fixes link error for shared libraries.

flang/include/flang/Evaluate/tools.h
97

This line inlined from semantics::IsCoarray. This was only change needed to fix link error.

I did shared and non-shared builds, and both were successful.