Skip to content

Commit

Permalink
Convert the Stencil distribution to a record (chapel-lang#23016)
Browse files Browse the repository at this point in the history
[reviewed by @benharsh — thanks!]

This follows the grand tradition of chapel-lang#22657 , chapel-lang#22951 , and chapel-lang#22991 , this
PR changes the `Stencil` distribution to a record. It:

* adds a new `Stencil` record, modeled closely on `Block`
* converts the old `Stencil` class to a `StencilImpl` record
* adds the necessary `StencilDom.dsiGetDist()` method
* updates the dmapType deprecation test to include a dmap + Stencil
example
* updates other tests that were using dmap and Stencil together

Not directly related to the main point of the PR (other than that I hit
it while updating tests), it also:
* fixes the argument and return intent of two of the internal routines
used to build dmap information to work in the event that a `const`
record is passed in; this is strictly needed to support legacy `dmap`
behavior which is why I hadn't noticed it previously
* adds a future because for awhile I had changed it to accept a `const
ref` and return the arg by `ref`, which should be illegal

And then even less related, I made further updates to the untested
hpl.hpcc2012.chpl and hpl.performance.chpl tests to better reflect the
current language / avoid false positives on `git grep`
  • Loading branch information
bradcray authored Aug 22, 2023
2 parents 1096d69 + 1d06817 commit 079a6fb
Show file tree
Hide file tree
Showing 14 changed files with 214 additions and 70 deletions.
168 changes: 138 additions & 30 deletions modules/dists/StencilDist.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,108 @@ config param disableStencilLazyRAD = defaultDisableLazyRADOpt;
compile if the array element is not an array or a class.
*/
class Stencil : BaseDist {
pragma "ignore noinit"
record Stencil {
param rank: int;
type idxType = int;
param ignoreFluff = false;

forwarding const chpl_distHelp: chpl_PrivatizedDistHelper(unmanaged StencilImpl(rank, idxType, ignoreFluff));

proc init(boundingBox: domain,
targetLocales: [] locale = Locales,
dataParTasksPerLocale=getDataParTasksPerLocale(),
dataParIgnoreRunningTasks=getDataParIgnoreRunningTasks(),
dataParMinGranularity=getDataParMinGranularity(),
param rank = boundingBox.rank,
type idxType = boundingBox.idxType,
fluff: rank*idxType = makeZero(rank, idxType),
periodic: bool = false,
param ignoreFluff = false) {
const value = new unmanaged StencilImpl(boundingBox, targetLocales,
dataParTasksPerLocale,
dataParIgnoreRunningTasks,
dataParMinGranularity,
rank, idxType, fluff, periodic,
ignoreFluff);
this.rank = rank;
this.idxType = idxType;
this.ignoreFluff = ignoreFluff;
this.chpl_distHelp = new chpl_PrivatizedDistHelper(
if _isPrivatized(value)
then _newPrivatizedClass(value)
else nullPid,
value);
}

proc init(_pid : int, _instance, _unowned : bool) {
this.rank = _instance.rank;
this.idxType = _instance.idxType;
this.ignoreFluff = _instance.ignoreFluff;
this.chpl_distHelp = new chpl_PrivatizedDistHelper(_pid,
_instance,
_unowned);
}

proc init(value) {
this.rank = value.rank;
this.idxType = value.idxType;
this.ignoreFluff = value.ignoreFluff;
this.chpl_distHelp = new chpl_PrivatizedDistHelper(
if _isPrivatized(value)
then _newPrivatizedClass(value)
else nullPid,
_to_unmanaged(value));
}

// Note: This does not handle the case where the desired type of 'this'
// does not match the type of 'other'. That case is handled by the compiler
// via coercions.
proc init=(const ref other : Stencil(?)) {
this.init(other._value.dsiClone());
}

proc clone() {
return new Stencil(this._value.dsiClone());
}

@chpldoc.nodoc
inline operator ==(d1: Stencil(?), d2: Stencil(?)) {
if (d1._value == d2._value) then
return true;
return d1._value.dsiEqualDMaps(d2._value);
}

@chpldoc.nodoc
inline operator !=(d1: Stencil(?), d2: Stencil(?)) {
return !(d1 == d2);
}

proc writeThis(x) {
chpl_distHelp.writeThis(x);
}
}


@chpldoc.nodoc
@unstable(category="experimental", reason="assignment between distributions is currently unstable due to lack of testing")
operator =(ref a: Stencil(?), b: Stencil(?)) {
if a._value == nil {
__primitive("move", a, chpl__autoCopy(b.clone(), definedConst=false));
} else {
if a._value.type != b._value.type then
compilerError("type mismatch in distribution assignment");
if a._value == b._value {
// do nothing
} else
a._value.dsiAssign(b._value);
if _isPrivatized(a._instance) then
_reprivatize(a._value);
}
}


class StencilImpl : BaseDist {
param rank: int;
type idxType = int;
param ignoreFluff: bool;
Expand All @@ -265,8 +366,8 @@ class Stencil : BaseDist {
//
// Local Stencil Distribution Class
//
// rank : generic rank that matches Stencil.rank
// idxType: generic index type that matches Stencil.idxType
// rank : generic rank that matches StencilImpl.rank
// idxType: generic index type that matches StencilImpl.idxType
// myChunk: a non-distributed domain that defines this locale's indices
//
class LocStencil {
Expand All @@ -287,7 +388,7 @@ class LocStencil {
//
class StencilDom: BaseRectangularDom(?) {
param ignoreFluff : bool;
const dist: unmanaged Stencil(rank, idxType, ignoreFluff);
const dist: unmanaged StencilImpl(rank, idxType, ignoreFluff);
var locDoms: [dist.targetLocDom] unmanaged LocStencilDom(rank, idxType,
strides);
var whole: domain(rank=rank, idxType=idxType, strides=strides);
Expand Down Expand Up @@ -430,7 +531,7 @@ private proc makeZero(param rank : int, type idxType) {
//
// Stencil initializer for clients of the Stencil distribution
//
proc Stencil.init(boundingBox: domain,
proc StencilImpl.init(boundingBox: domain,
targetLocales: [] locale = Locales,
dataParTasksPerLocale=getDataParTasksPerLocale(),
dataParIgnoreRunningTasks=getDataParIgnoreRunningTasks(),
Expand Down Expand Up @@ -496,7 +597,7 @@ proc Stencil.init(boundingBox: domain,
}
}

proc Stencil.dsiAssign(other: this.type) {
proc StencilImpl.dsiAssign(other: this.type) {
coforall locid in targetLocDom do
on targetLocales(locid) do
delete locDist(locid);
Expand All @@ -519,35 +620,35 @@ proc Stencil.dsiAssign(other: this.type) {
// Stencil distributions are equivalent if they share the same bounding
// box and target locale set.
//
proc Stencil.dsiEqualDMaps(that: Stencil(?)) {
proc StencilImpl.dsiEqualDMaps(that: StencilImpl(?)) {
return (this.boundingBox == that.boundingBox &&
this.targetLocales.equals(that.targetLocales) &&
this.fluff == that.fluff &&
this.periodic == that.periodic);
}


proc Stencil.dsiEqualDMaps(that) param {
proc StencilImpl.dsiEqualDMaps(that) param {
return false;
}



proc Stencil.dsiClone() {
return new unmanaged Stencil(boundingBox, targetLocales,
proc StencilImpl.dsiClone() {
return new unmanaged StencilImpl(boundingBox, targetLocales,
dataParTasksPerLocale, dataParIgnoreRunningTasks,
dataParMinGranularity, fluff=fluff, periodic=periodic,
ignoreFluff=this.ignoreFluff);
}

override proc Stencil.dsiDestroyDist() {
override proc StencilImpl.dsiDestroyDist() {
coforall ld in locDist do {
on ld do
delete ld;
}
}

override proc Stencil.dsiDisplayRepresentation() {
override proc StencilImpl.dsiDisplayRepresentation() {
writeln("boundingBox = ", boundingBox);
writeln("targetLocDom = ", targetLocDom);
writeln("targetLocales = ", for tl in targetLocales do tl.id);
Expand All @@ -558,7 +659,7 @@ override proc Stencil.dsiDisplayRepresentation() {
writeln("locDist[", tli, "].myChunk = ", locDist[tli].myChunk);
}

override proc Stencil.dsiNewRectangularDom(param rank: int, type idxType,
override proc StencilImpl.dsiNewRectangularDom(param rank: int, type idxType,
param strides: strideKind, inds) {
if idxType != this.idxType then
compilerError("Stencil domain index type does not match distribution's");
Expand Down Expand Up @@ -590,7 +691,7 @@ override proc Stencil.dsiNewRectangularDom(param rank: int, type idxType,
//
// output distribution
//
proc Stencil.writeThis(x) throws {
proc StencilImpl.writeThis(x) throws {
x.writeln("Stencil");
x.writeln("-------");
x.writeln("distributes: ", boundingBox);
Expand All @@ -602,19 +703,19 @@ proc Stencil.writeThis(x) throws {
" owns chunk: ", locDist(locid).myChunk);
}

proc Stencil.dsiIndexToLocale(ind: idxType) where rank == 1 {
proc StencilImpl.dsiIndexToLocale(ind: idxType) where rank == 1 {
return targetLocales(targetLocsIdx(ind));
}

proc Stencil.dsiIndexToLocale(ind: rank*idxType) where rank > 1 {
proc StencilImpl.dsiIndexToLocale(ind: rank*idxType) where rank > 1 {
return targetLocales(targetLocsIdx(ind));
}

//
// compute what chunk of inds is owned by a given locale -- assumes
// it's being called on the locale in question
//
proc Stencil.getChunk(inds, locid) {
proc StencilImpl.getChunk(inds, locid) {
// use domain slicing to get the intersection between what the
// locale owns and the domain's index set
//
Expand All @@ -639,11 +740,11 @@ proc Stencil.getChunk(inds, locid) {
//
// get the index into the targetLocales array for a given distributed index
//
proc Stencil.targetLocsIdx(ind: idxType) where rank == 1 {
proc StencilImpl.targetLocsIdx(ind: idxType) where rank == 1 {
return targetLocsIdx((ind,));
}

proc Stencil.targetLocsIdx(ind: rank*idxType) {
proc StencilImpl.targetLocsIdx(ind: rank*idxType) {
var result: rank*int;
for param i in 0..rank-1 do
result(i) = max(0, min((targetLocDom.dim(i).sizeAs(int)-1):int,
Expand All @@ -654,7 +755,7 @@ proc Stencil.targetLocsIdx(ind: rank*idxType) {
}

// TODO: This will not trigger the bounded-coforall optimization
iter Stencil.activeTargetLocales(const space : domain = boundingBox) {
iter StencilImpl.activeTargetLocales(const space : domain = boundingBox) {
const locSpace = {(...space.dims())}; // make a local domain in case 'space' is distributed
const low = chpl__tuplify(targetLocsIdx(locSpace.first));
const high = chpl__tuplify(targetLocsIdx(locSpace.last));
Expand Down Expand Up @@ -716,6 +817,13 @@ proc LocStencil.init(param rank, type idxType, param dummy: bool) where dummy {
this.idxType = idxType;
}

proc StencilDom.dsiGetDist() {
if _isPrivatized(dist) then
return new Stencil(dist.pid, dist, _unowned=true);
else
return new Stencil(nullPid, dist, _unowned=true);
}

override proc StencilDom.dsiDisplayRepresentation() {
writeln("whole = ", whole);
for tli in dist.targetLocDom do
Expand Down Expand Up @@ -1550,7 +1658,7 @@ iter StencilArr.dsiBoundaries(param tag : iterKind) where tag == iterKind.standa
//
pragma "no copy return"
proc StencilArr.noFluffView() {
var tempDist = new unmanaged Stencil(dom.dist.boundingBox, dom.dist.targetLocales,
var tempDist = new unmanaged StencilImpl(dom.dist.boundingBox, dom.dist.targetLocales,
dom.dist.dataParTasksPerLocale, dom.dist.dataParIgnoreRunningTasks,
dom.dist.dataParMinGranularity, ignoreFluff=true);
pragma "no auto destroy" var newDist = new _distribution(tempDist);
Expand Down Expand Up @@ -1760,7 +1868,7 @@ override proc StencilDom.dsiSupportsAutoLocalAccess() param { return true; }
//
// Privatization
//
proc Stencil.init(other: Stencil, privateData,
proc StencilImpl.init(other: StencilImpl, privateData,
param rank = other.rank,
type idxType = other.idxType,
param ignoreFluff = other.ignoreFluff) {
Expand All @@ -1778,21 +1886,21 @@ proc Stencil.init(other: Stencil, privateData,
dataParMinGranularity = privateData(4);
}

override proc Stencil.dsiSupportsPrivatization() param do return true;
override proc StencilImpl.dsiSupportsPrivatization() param do return true;

proc Stencil.dsiGetPrivatizeData() {
proc StencilImpl.dsiGetPrivatizeData() {
return (boundingBox.dims(), targetLocDom.dims(),
dataParTasksPerLocale, dataParIgnoreRunningTasks, dataParMinGranularity,
fluff, periodic);
}

proc Stencil.dsiPrivatize(privatizeData) {
return new unmanaged Stencil(_to_unmanaged(this), privatizeData);
proc StencilImpl.dsiPrivatize(privatizeData) {
return new unmanaged StencilImpl(_to_unmanaged(this), privatizeData);
}

proc Stencil.dsiGetReprivatizeData() do return boundingBox.dims();
proc StencilImpl.dsiGetReprivatizeData() do return boundingBox.dims();

proc Stencil.dsiReprivatize(other, reprivatizeData) {
proc StencilImpl.dsiReprivatize(other, reprivatizeData) {
boundingBox = {(...reprivatizeData)};
targetLocDom = other.targetLocDom;
targetLocales = other.targetLocales;
Expand Down Expand Up @@ -1904,11 +2012,11 @@ proc StencilDom.dsiTargetLocales() const ref {
return dist.targetLocales;
}

proc Stencil.dsiTargetLocales() const ref {
proc StencilImpl.dsiTargetLocales() const ref {
return targetLocales;
}

proc Stencil.chpl__locToLocIdx(loc: locale) {
proc StencilImpl.chpl__locToLocIdx(loc: locale) {
for locIdx in targetLocDom do
if (targetLocales[locIdx] == loc) then
return (true, locIdx);
Expand Down
4 changes: 2 additions & 2 deletions modules/internal/ChapelArray.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -691,15 +691,15 @@ module ChapelArray {
proc chpl__buildDistValue(in x:owned) where isSubtype(x.borrow().type, BaseDist) {
return new _distribution(owned.release(x));
}
proc chpl__buildDistValue(ref x:record) ref {
proc chpl__buildDistValue(const ref x:record) const ref {
return x;
}

proc chpl__buildDistValue(x) {
compilerError("illegal domain map value specifier - must be a subclass of BaseDist");
}

proc chpl__buildDistDMapValue(ref x: record) ref {
proc chpl__buildDistDMapValue(const ref x: record) const ref {
compilerWarning("The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'");
return chpl__buildDistValue(x);
}
Expand Down
6 changes: 6 additions & 0 deletions test/deprecated/dmapConstRecord.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
use BlockDist;

config const n = 10;

const block = new Block({1..n});
var D = {1..n} dmapped new dmap(block);
1 change: 1 addition & 0 deletions test/deprecated/dmapConstRecord.good
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dmapConstRecord.chpl:6: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
10 changes: 10 additions & 0 deletions test/deprecated/dmapType.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ config var n = 10;
const Dist2: dmap(Replicated) = new Replicated();
writeln(Dist2.type:string);
}

{
use StencilDist;
var Dist = new dmap(new Stencil({1..n}));
var Dom = {1..n} dmapped Dist;
var A: [Dom] real;
writeln(A);
const Dist2: dmap(Stencil(1)) = new Stencil({1..n});
writeln(Dist2.type:string);
}
4 changes: 4 additions & 0 deletions test/deprecated/dmapType.good
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ dmapType.chpl:14: warning: The use of 'dmap' is deprecated for this distribution
dmapType.chpl:18: warning: The use of 'dmap' is depreacted for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
dmapType.chpl:24: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
dmapType.chpl:28: warning: The use of 'dmap' is depreacted for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
dmapType.chpl:34: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
dmapType.chpl:38: warning: The use of 'dmap' is depreacted for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
Block(1,int(64),unmanaged DefaultDist)
0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
Cyclic(1,int(64))
0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
Replicated
0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
Stencil(1,int(64),false)
2 changes: 2 additions & 0 deletions test/functions/intents/constRefArgRefReturn.bad
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
43
43
9 changes: 9 additions & 0 deletions test/functions/intents/constRefArgRefReturn.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
proc foo(const ref x) ref {
return x;
}

const answer = 42;
ref alias = foo(answer);
alias += 1;
writeln(alias);
writeln(answer);
3 changes: 3 additions & 0 deletions test/functions/intents/constRefArgRefReturn.future
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
bug: compiler permits 'const ref' formals to be returned as 'ref'

(resulting in the ability to change 'const' values)
Loading

0 comments on commit 079a6fb

Please sign in to comment.