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

removing runtime types: design considerations #20424

Open
vasslitvinov opened this issue Aug 11, 2022 · 8 comments
Open

removing runtime types: design considerations #20424

vasslitvinov opened this issue Aug 11, 2022 · 8 comments

Comments

@vasslitvinov
Copy link
Member

vasslitvinov commented Aug 11, 2022

If the answer to #19292 is "yes", how will the RTT-free world look like? Here are some possibilities.

An array type is a syntactic form

X : [someDomain] elemType means "X is of a certain array type" and "X.domain shall be someDomain".

var X: [D] real;                            // source code
var X: [D.type] real = new array(D, real);  // translated into this

var X: [D] real = someExpr;                           // source code
var X: [D.type] real = new array(D, real, someExpr);  // translated into this

// source code
record R {
  var D: domain(1);
  var A: [D] real;
  proc init(Darg, Aarg) { D = Darg; A = Aarg; }
}
// translated into this
record R {
  var D: domain(1);
  var A: [D.type] real;
  proc init(Darg, Aarg) { D = Darg; A = new array(D, Aarg); }
}

proc p(arg: [D] real) { BODY; }                             // source code
proc P(arg: [D.type] real) { checkArrayArg(arg, D); BODY; } // translated into this

Arrays of arrays

  • Bounds checking is preserved when assigning to an inner array.
var A: [D] [D2] real;                                       // source code
var A: [D.type] [D2.type] real = new array((D,D2), real);   // translated into this

A[i] = someArray;  // error if A[i] and someArray have different shapes
var A: [D][domain(1)] real;  // error: cannot default-initialize A[i]

var Askyline: [D][domain(1)] real = [idx in D] new array(1..idx, real);  // ok
  • The elements of the outer array are not default-initializable.
var A: [D] [D2] real;

D = someDomain;  // error if this adds new indices to D

// use D.unsafeAssign instead
manage D.unsafeAssign(someDomain) as mgr {
  for idx in mgr.newIndices() do
    mgr.initialize(A, idx, new array(1..idx, real));  // ok if {1..idx} != D2
}
@mppf
Copy link
Member

mppf commented Aug 11, 2022

By the way, something like new array(D, real, someExpr) is IMO a useful feature all on its own, and something that can make initializers clearer with arrays.

@mppf
Copy link
Member

mppf commented Aug 11, 2022

The elements of the outer array are not default-initializable.

I don't think that is strictly required. If we want to allow outer array elements to be default-initialized, we just need to arrange for the outer array to have a link to the inner domain, somehow, right?

Seemed like it might be related to your idea

var A: [D.type] [D2.type] real = new array((D,D2), real);   // translated into this

where the array A itself would maybe need to store (D, D2) ?

@vasslitvinov
Copy link
Member Author

var A: [D.type] [D2.type] real = new array((D,D2), real);
where the array A itself would maybe need to store (D, D2) ?

I am open to that, however this is a new feature. Today A already stores D, which is A.domain. We can consider having A store D2 as well.

Given that we want to (continue to) support skyline arrays, I see two design choices:

  • make it optional for A to store D2, or
  • have A store D2 as advisory info, without requiring that it matches A[i].domain.

Alternatively, we could toss this in user's court and have them wrap A and D2 manually into another datatype if desired.

@mppf
Copy link
Member

mppf commented Aug 11, 2022

Given that we want to (continue to) support skyline arrays

IMO we can consider dropping the minimal support we have today for skyline arrays. They weren't really designed & what works today is a side effect of other efforts. But in any case it seems to me at the moment that we probably would like the declarations for skyline arrays to be different enough that the compiler can make the choice between relying on D2 vs ignoring it.

@bradcray
Copy link
Member

IMO we can consider dropping the minimal support we have today for skyline arrays.

I'm not sure whether you mean "temporarily" or "permanently", but I'd much rather see us building up full-fledged support for skyline arrays than dropping them. They've been useful to me in the past, and users ask about them relatively frequently. To me, the large number of users who create arrays of records-wrapped-arrays suggests a desire for them as well. The language is also well-designed syntactically for supporting them in forms like:

var Multigrid: [i in 1..numLevels] [{1..n, 1..n, 1..n} by 2**i] real;
var Tri: [i in 1..numRows] [1..i] real;

@vasslitvinov
Copy link
Member Author

vasslitvinov commented Aug 13, 2022

@bradcray how acceptable to you is the following syntax for declaring skyline arrays?

var Multigrid = [i in 1..numLevels] new array({1..n, 1..n, 1..n} by 2**i, real);
var Tri = [i in 1..numRows] new array(1..i, real);
// more levels of nesting:
var Tri4 = [i in 1..numRows] [j in 1..numColumns] new array({1..i,1..j}, real);
// the type of Tri4 will be: [domain(1)][domain(1)][domain(2)] real

[added] We can also translate the code in your comment to the code in this comment under the hood.

@mppf
Copy link
Member

mppf commented Aug 15, 2022

IMO we can consider dropping the minimal support we have today for skyline arrays.

I'm not sure whether you mean "temporarily" or "permanently",

Temporarily. I don't think we've actually made the language design decisions to have any confidence about the skyline array patterns we currently have implemented. (What is implemented was more a side effect of some other changes I was making). From a very high level, in some ways skyline arrays are easier to see how to implement, if we don't have runtime types at all. Even if we continue to have array-of-array behave as it does now with regard to the inner domain, as long as the compiler can differentiate skyline arrays from other arrays-of-arrays, I think we'll be able to build what we need to, in terms of skyline array support.

@bradcray
Copy link
Member

how acceptable to you is the following syntax for declaring skyline arrays?

I wouldn't like if a user would have to write it that way to get a skyline array (since I think doing so breaks the nice syntactic symmetry we have between array types and forall expressions—i.e., [i in 1..n] [1..i] real; vs. [i in 1..n] [j in 1..i] (i*1000 + j);

Temporarily

Thanks for clarifying.

I don't think we've actually made the language design decisions to have any confidence about the skyline array patterns we currently have implemented.

I'd agree with that.

dlongnecke-cray added a commit that referenced this issue Aug 22, 2022
…ic-record-rtt

Fix generic record initialization when substitution is a runtime type

Attempt to recover runtime types used in the type construction of
default initialized records. This fixes some cases where a
default initialized record containing an array or domain field
would halt at runtime when said field was used.

Resolves #19118.
Resolves #20167.

The parent issue for this effort is: #15929.

Also consider: #19292, #20424

This PR does not fix all bugs related to default initialization of
aggregates that contain runtime types. The bug persists when
a type constructor containing a runtime type is obfuscated by
indirection (e.g, it is returned by a function, or uses 2+ levels of
type alias). See the added future `RuntimeTypeEscapes.chpl` for an
example.

Reviewed by @vasslitvinov. Thanks!

TESTING

- [x] `ALL` on `linux64`, `standard`

Signed-off-by: David Longnecker <dlongnecke-cray@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants