chapel icon indicating copy to clipboard operation
chapel copied to clipboard

Extend remote variables to support multi-declarations

Open DanilaFe opened this issue 6 months ago • 0 comments

Closes https://github.com/chapel-lang/chapel/issues/25429. Closes https://github.com/chapel-lang/chapel/issues/25287.

This PR adds support for remote multi-variable declarations:

on loc var a, b = 2;

Since remote variables are desugared early (and since they apply nontrivial transformations to the user-written types and initialization expressions), doing so required duplicating existing multi-decl logic (which occurs later) at the point where variables are desugared. Instead, I simply moved all the logic earlier, into convert-uast.cpp, where an additional set of data is passed to convertVariable when it's being converted as part of a multi-variable declaration. This data contains the locale on which to execute as well as the previous type/init expressions to use if none are provided.

A limitation of multi-declarations for remote variables is that they spawn an on task per locale, just like a sequence of var declarations would. This is due to the fact that in order for a single on-statement to initialize to multiple remote variables, all of these variables have to be declared (with a type) prior to the on block. Since the types and values of multi-decl'ed variables can be dependent on the types and values computed while initializing other multi-decls, this seems to impose an impossible constraint. A case that clearly doesn't work is:

on Locales[1] var x = computeLength(), y: [1..x] int;

But I suspect even simpler cases will run into trouble here. There may be ways to do this, but they are involved and I suspect will cause overhead, so I've not given them a shot yet.

Future Work

This PR "duplicates" some logic for flattening multi-variable declarations. It seems plausible to move the flattening into convert-uast.cpp and unify the two implementations, getting rid of the original one in cleanup.cpp.

Testing

  • [x] paratest GASNet
  • [x] paratest local
  • [x] GPU testing

DanilaFe avatar Aug 13 '24 21:08 DanilaFe