Skip to content

Commit

Permalink
Disable ALA and AA for Default Rectangular arrays (#25840)
Browse files Browse the repository at this point in the history
Resolves an issue @bradcray bumped into his PR:
#25754 (comment).
As such, merging this should unblock
#25754.

Consider:

```chpl
forall i in MyArr.domain {
  ... MyArr[i] ...
}
```

This innocent loop is suitable for ALA (and AA in more complicated
scenarios) if MyArr is distributed. In such cases, distributed arrays'
iterators will ensure that the array is accesssed locally. That's not
the case for DefaultRectangular arrays, though. I personally find this
confusing and [want to change
it](#25351). However, until
we do that, we should disable the optimization to comply with the
current semantics/implementation of the parallel iterators.

We could consider making the core analysis account for arrays/dists that
support ALA only through some special dynamic checks. I think that's
definitely doable. But that's not easy and its real impact is
questionable. Thus, this PR disables ALA and AA for DefaultRectangulars.

Note that this is simply done by flipping the default value of the
`config param defaultRectangularSupportsAutoLocalAccess`. I am leaving
that flag in, even though opting into the optimization will result in
potentially erroneous behavior. This flag is not documented and can
still be used for some easy experimentation as the said erroneous
behavior is relatively uncommon.

[Trivial, not reviewed, but direction OK'ed by @bradcray]

Test:
- [x] linux64
- [x] gasnet
  • Loading branch information
e-kayrakli authored Aug 28, 2024
2 parents 19ee3a2 + 7ded4c9 commit 09a23f9
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 6 deletions.
2 changes: 1 addition & 1 deletion modules/internal/DefaultRectangular.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ module DefaultRectangular {
config param earlyShiftData = true;
config param usePollyArrayIndex = false;

config param defaultRectangularSupportsAutoLocalAccess = true;
config param defaultRectangularSupportsAutoLocalAccess = false;

enum ArrayStorageOrder { RMO, CMO }
config param defaultStorageOrder = ArrayStorageOrder.RMO;
Expand Down
65 changes: 65 additions & 0 deletions test/optimizations/autoAggregation/localArrays.bad
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
Start analyzing forall for automatic aggregation [static only ALA clone] (localArrays.chpl:49)
| Found an aggregation candidate (localArrays.chpl:50)
| Potential source aggregation (localArrays.chpl:50)
End analyzing forall for automatic aggregation [static only ALA clone] (localArrays.chpl:49)

Start analyzing forall for automatic aggregation [no ALA clone] (localArrays.chpl:49)
End analyzing forall for automatic aggregation [no ALA clone] (localArrays.chpl:49)

Aggregation candidate has confirmed local child [static only ALA clone] (localArrays.chpl:50)
LHS is local, RHS is nonlocal. Will use source aggregation [static only ALA clone] (localArrays.chpl:50)
Aggregation candidate has confirmed local child [static only ALA clone] (localArrays.chpl:50)
LHS is local, RHS is nonlocal. Will use source aggregation [static only ALA clone] (localArrays.chpl:50)
Aggregation candidate has confirmed local child [static only ALA clone] (localArrays.chpl:50)
LHS is local, RHS is nonlocal. Will use source aggregation [static only ALA clone] (localArrays.chpl:50)
Aggregation candidate has confirmed local child [static only ALA clone] (localArrays.chpl:50)
LHS is local, RHS is nonlocal. Will use source aggregation [static only ALA clone] (localArrays.chpl:50)
Aggregation candidate has confirmed local child [static only ALA clone] (localArrays.chpl:50)
LHS is local, RHS is nonlocal. Will use source aggregation [static only ALA clone] (localArrays.chpl:50)
Replaced assignment with aggregation [static only ALA clone] (localArrays.chpl:50)
Replaced assignment with aggregation [static only ALA clone] (localArrays.chpl:50)
Testing 1D DR
0 0 0 0 0 0 0 0 0 0
End testing 1D DR

Testing 2D DR
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0
End testing 2D DR

Testing 1D COO
SrcAggregator.copy is called
SrcAggregator.copy is called
0 0
End testing 1D COO

Testing 2D COO
0
0

End testing 2D COO

Testing CSR
0
0

End testing CSR

Testing CSC
0
0

End testing CSC

Testing associative domain with string keys
0 0
End testing associative domain with string keys

4 changes: 4 additions & 0 deletions test/optimizations/autoAggregation/localArrays.future
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Locality of DRs can't be determined statically.

Context 1: https://github.com/chapel-lang/chapel/pull/25754#issuecomment-2316297947
Context 2: https://github.com/chapel-lang/chapel/issues/25351
6 changes: 6 additions & 0 deletions test/optimizations/autoLocalAccess/localArrays.notest
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
We are disabling ALA for DR due to some challenges.

See https://github.com/chapel-lang/chapel/pull/25754#issuecomment-2316297947

The symmetric of this test is in autoAggregation/localArrays.chpl. That test
will be futurized.
1 change: 0 additions & 1 deletion test/parallel/forall/checks/const-indices-blc.good
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
const-indices-blc.chpl:5: error: cannot assign to const variable
const-indices-blc.chpl:11: error: cannot assign to const variable
const-indices-blc.chpl:11: error: cannot assign to const variable
2 changes: 0 additions & 2 deletions test/users/alvaradoo/refToSubclassInternalError-shared.good
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
refToSubclassInternalError-shared.chpl:17: In function 'populate':
refToSubclassInternalError-shared.chpl:23: error: initializing a reference using an expression whose type 'shared GenProperty?' is not an instantiation of the declared type 'shared Property'
refToSubclassInternalError-shared.chpl:23: error: initializing a reference using an expression whose type 'shared GenProperty?' is not an instantiation of the declared type 'shared Property'
refToSubclassInternalError-shared.chpl:87: called as populate(dataArray: [domain(1,int(64),one)] int(64), inputVertices: [domain(1,int(64),one)] int(64), vertexProps: [BlockDom(2,int(64),one,unmanaged DefaultDist)] shared GenProperty?, dataTypeMap: [domain(1,int(64),one)] string, colId: int(64))
refToSubclassInternalError-shared.chpl:17: In function 'populate':
refToSubclassInternalError-shared.chpl:23: error: initializing a reference using an expression whose type 'shared GenProperty?' is not an instantiation of the declared type 'shared Property'
refToSubclassInternalError-shared.chpl:23: error: initializing a reference using an expression whose type 'shared GenProperty?' is not an instantiation of the declared type 'shared Property'
refToSubclassInternalError-shared.chpl:88: called as populate(dataArray: [domain(1,int(64),one)] bool, inputVertices: [domain(1,int(64),one)] int(64), vertexProps: [BlockDom(2,int(64),one,unmanaged DefaultDist)] shared GenProperty?, dataTypeMap: [domain(1,int(64),one)] string, colId: int(64))
2 changes: 0 additions & 2 deletions test/users/alvaradoo/refToSubclassInternalError.good
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
refToSubclassInternalError.chpl:17: In function 'populate':
refToSubclassInternalError.chpl:23: error: initializing a reference using an expression whose type 'unmanaged GenProperty?' is not an instantiation of the declared type 'unmanaged Property'
refToSubclassInternalError.chpl:23: error: initializing a reference using an expression whose type 'unmanaged GenProperty?' is not an instantiation of the declared type 'unmanaged Property'
refToSubclassInternalError.chpl:87: called as populate(dataArray: [domain(1,int(64),one)] int(64), inputVertices: [domain(1,int(64),one)] int(64), vertexProps: [BlockDom(2,int(64),one,unmanaged DefaultDist)] unmanaged GenProperty?, dataTypeMap: [domain(1,int(64),one)] string, colId: int(64))
refToSubclassInternalError.chpl:17: In function 'populate':
refToSubclassInternalError.chpl:23: error: initializing a reference using an expression whose type 'unmanaged GenProperty?' is not an instantiation of the declared type 'unmanaged Property'
refToSubclassInternalError.chpl:23: error: initializing a reference using an expression whose type 'unmanaged GenProperty?' is not an instantiation of the declared type 'unmanaged Property'
refToSubclassInternalError.chpl:88: called as populate(dataArray: [domain(1,int(64),one)] bool, inputVertices: [domain(1,int(64),one)] int(64), vertexProps: [BlockDom(2,int(64),one,unmanaged DefaultDist)] unmanaged GenProperty?, dataTypeMap: [domain(1,int(64),one)] string, colId: int(64))

0 comments on commit 09a23f9

Please sign in to comment.