Skip to content

Lower elemental subroutine calls (outside of user assignment case) #1069

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

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

jeanPerier
Copy link
Collaborator

Use the array lowering framework to lower elemental subroutine.
This required:

  • Splitting the part of genIterSpace that creates the
    implicit loops into a new genImplicitLoops function. genIterspace
    deals with the destination and other explicit contexts set-ups that
    do not matter in the subroutine case (there is no destination).
    Other than having no destination, a big difference is that there is
    no threaded innerArg. genImplicitLoops handles this variation.

  • Fixing a bug in the iteration shape determination. The code was
    assuming there would always be at least one array_load, which is
    not true if there are only RefTransparent operands. This is always
    true in the subroutine case, but was actually also an issue for things
    like print *, elem_func(array) with the previous code (compile crash).
    Add a new ArrayOperands structure to keep track of what matters to
    later deduce the shape of the array expression instead of keeping
    track of array_loads for that purpose.

  • Elemental subroutine with intent(out)/intent(inout) arguments must be
    applied "in array element order" (15.8.3). It turns out this is also
    the case for impure elemental functions (10.1.4 p5). Instead of always
    creating loops as "unordered=true" add an unordered field to the ArrayExprLowering
    class. It is set to true by default, but will be set to false if any
    impure elemental function is lowered, or in case of
    intent(out)/intent(inout) arguments in elemental subroutines.

  • Last, instead of using createFirExpr to lowering subroutine calls, use
    a new createSubroutineCall that deals with the array/scalar
    dispatching.

A TODO is added for the user defined elemental assignment, because
overlap analysis between RHS/LHS must be done, and this require somehow
plugging this special and very restricted case of elemental calls into
the array_load/array_update/array_merge framework or some ad-hoc
lowering.

Note: some dead code in the code that became genImplicitLoops is removed here (FORALL refactoring leftover ?). The loop bounds are now evaluated after the ccPrelude mask due to the genImplicitLoops code move, hence the forall lit test change.

Copy link
Collaborator

@psteinfeld psteinfeld left a comment

Choose a reason for hiding this comment

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

I don't really understand the code, so someone who does should approve.

I've verified that everything builds and tests correctly.

Use the array lowering framework to lower elemental subroutine.
This required:

- Splitting the part of genIterSpace that creates the
  implicit loops into a new genImplicitLoops function. genIterspace
  deals with the destination and other explicit contexts set-ups that
  do not matter in the subroutine case (there is no destination).
  Other than having no destination, a big difference is that there is
  no threaded innerArg. genImplicitLoops handles this variation.

- Fixing a bug in the iteration shape determination. The code was
  assuming there would always be at least one array_load, which is
  not true if there are only RefTransparent operands. This is always
  true in the subroutine case, but was actually also an issue for things
  like `print *, elem_func(array)` with the previous code (compile crash).
  Add a new ArrayOperands structure to keep track of what matters to
  later deduce the shape of the array expression instead of keeping
  track of array_loads for that purpose.

- Elemental subroutine with intent(out)/intent(inout) arguments must be
  applied "in array element order" (15.8.3). It turns out this is also
  the case for impure elemental functions (10.1.4 p5). Instead of always
  creating loops as "unordered=true" add an unordered field to the ArrayExprLowering
  class. It is set to true by default, but will be set to false if any
  impure elemental function is lowered, or in case of
  intent(out)/intent(inout) arguments in elemental subroutines.

- Last, instead of using createFirExpr to lowering subroutine calls, use
  a new createSubroutineCall that deals with the array/scalar
  dispatching.

A TODO is added for the user defined elemental assignment, because
overlap analysis between RHS/LHS must be done, and this require somehow
plugging this special and very restricted case of elemental calls into
the array_load/array_update/array_merge framework or some ad-hoc
lowering.
Copy link
Collaborator

@clementval clementval left a comment

Choose a reason for hiding this comment

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

LGTM

@jeanPerier jeanPerier merged commit 0b3fa79 into fir-dev Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants