Skip to content

Commit

Permalink
Fix an ALA bug due to a missing dsiEqualDMaps overload (#25857)
Browse files Browse the repository at this point in the history
This PR fixes an issue exposed by llm.chpl where rank-change array views
and ALA did not work well in some scenarios. These issues would cause:

```chpl
$CHPL_HOME/modules/internal/ChapelAutoLocalAccess.chpl:57: In function 'chpl__ala_dynamicCheck':
$CHPL_HOME/modules/internal/ChapelAutoLocalAccess.chpl:63: error: unresolved call 'unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,3*bool,3*int(64)).dsiEqualDMaps(unmanaged DefaultDist)'
```

This was because a missing `dsiEqualDMaps` overload in the rank-change
array views. The PR adds that overload, and tests that reproduced the
issue.

I also confirmed that the same issue doesn't exist in slice arrayviews.
I am adding a symmetric test to lock in that behavior.

[Trivial bugfix, not reviewed]

Test:
- [x] linux64
- [x] gasnet
  • Loading branch information
e-kayrakli authored Sep 3, 2024
2 parents d6c8dc3 + 049274c commit c18eea7
Show file tree
Hide file tree
Showing 13 changed files with 113 additions and 0 deletions.
4 changes: 4 additions & 0 deletions modules/internal/ArrayViewRankChange.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ module ArrayViewRankChange {
this.idx == that.idx &&
this.downDist.dsiEqualDMaps(that.downDist));
}

proc dsiEqualDMaps(that) {
return false;
}
}

private proc downDomType(param rank : int,
Expand Down
18 changes: 18 additions & 0 deletions test/optimizations/autoLocalAccess/rankChangeDynamic.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use BlockDist;
var BaseArr = blockDist.createArray({1..2, 1..7}, int);

ref RankChangeViewLocal = BaseArr[1, ..];
ref RankChangeViewRemote = BaseArr[2, ..];

const Dom = {1..7};

forall i in Dom {
RankChangeViewLocal[i] = i;
}

// this should fail the dynamic check
forall i in Dom {
RankChangeViewRemote[i] = i;
}

writeln(BaseArr);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-slogAllArrEltAccess
29 changes: 29 additions & 0 deletions test/optimizations/autoLocalAccess/rankChangeDynamic.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

Start analyzing forall (rankChangeDynamic.chpl:9)
| Found loop domain (rankChangeDynamic.chpl:7)
| Will attempt static and dynamic optimizations (rankChangeDynamic.chpl:9)
|
| Start analyzing call (rankChangeDynamic.chpl:10)
| Can't determine the domain of access base (rankChangeDynamic.chpl:4)
| This call is a dynamic optimization candidate (rankChangeDynamic.chpl:10)
|
End analyzing forall (rankChangeDynamic.chpl:9)


Start analyzing forall (rankChangeDynamic.chpl:14)
| Found loop domain (rankChangeDynamic.chpl:7)
| Will attempt static and dynamic optimizations (rankChangeDynamic.chpl:14)
|
| Start analyzing call (rankChangeDynamic.chpl:15)
| Can't determine the domain of access base (rankChangeDynamic.chpl:5)
| This call is a dynamic optimization candidate (rankChangeDynamic.chpl:15)
|
End analyzing forall (rankChangeDynamic.chpl:14)

Static check successful. Using localAccess with dynamic check (rankChangeDynamic.chpl:10)
Static check successful. Using localAccess with dynamic check (rankChangeDynamic.chpl:15)
1 2 3 4 5 6 7
1 2 3 4 5 6 7

Numbers collected by prediff:
localAccess was called 7 times
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2
3 changes: 3 additions & 0 deletions test/optimizations/autoLocalAccess/rankChangeDynamic.prediff
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh

./PREDIFF-filter-accessors $1 $2 --no-this
2 changes: 2 additions & 0 deletions test/optimizations/autoLocalAccess/rankChangeDynamic.skipif
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# this test's logic is wired for 2 locales
CHPL_COMM==none
19 changes: 19 additions & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use BlockDist;
var BaseArr = blockDist.createArray({1..2, 1..7}, int);

ref SliceViewLocal = BaseArr[1..1, ..];
ref SliceViewRemote = BaseArr[2..2, ..];

const LocalsDom = {1..1, 1..7};
const RemotesDom = {2..2, 1..7};

forall idx in LocalsDom {
SliceViewLocal[idx] = 1;
}

// this should fail the dynamic check
forall idx in RemotesDom {
SliceViewRemote[idx] = 2;
}

writeln(BaseArr);
1 change: 1 addition & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.compopts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-slogAllArrEltAccess
29 changes: 29 additions & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

Start analyzing forall (sliceDynamic.chpl:10)
| Found loop domain (sliceDynamic.chpl:7)
| Will attempt static and dynamic optimizations (sliceDynamic.chpl:10)
|
| Start analyzing call (sliceDynamic.chpl:11)
| Can't determine the domain of access base (sliceDynamic.chpl:4)
| This call is a dynamic optimization candidate (sliceDynamic.chpl:11)
|
End analyzing forall (sliceDynamic.chpl:10)


Start analyzing forall (sliceDynamic.chpl:15)
| Found loop domain (sliceDynamic.chpl:8)
| Will attempt static and dynamic optimizations (sliceDynamic.chpl:15)
|
| Start analyzing call (sliceDynamic.chpl:16)
| Can't determine the domain of access base (sliceDynamic.chpl:5)
| This call is a dynamic optimization candidate (sliceDynamic.chpl:16)
|
End analyzing forall (sliceDynamic.chpl:15)

Static check successful. Using localAccess with dynamic check (sliceDynamic.chpl:11)
Static check successful. Using localAccess with dynamic check (sliceDynamic.chpl:16)
1 1 1 1 1 1 1
2 2 2 2 2 2 2

Numbers collected by prediff:
localAccess was called 7 times
1 change: 1 addition & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.numlocales
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2
3 changes: 3 additions & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.prediff
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh

./PREDIFF-filter-accessors $1 $2 --no-this
2 changes: 2 additions & 0 deletions test/optimizations/autoLocalAccess/sliceDynamic.skipif
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# this test's logic is wired for 2 locales
CHPL_COMM==none

0 comments on commit c18eea7

Please sign in to comment.