From 0aa5b70df82cbe8759906a3cc1b309d5dfa481f3 Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Sun, 20 Aug 2023 21:31:41 -0700 Subject: [PATCH 1/5] Convert Stencil to record Signed-off-by: Brad Chamberlain --- modules/dists/StencilDist.chpl | 168 ++++++++++++++---- modules/internal/ChapelArray.chpl | 4 +- test/deprecated/dmapType.chpl | 10 ++ test/deprecated/dmapType.good | 4 + test/studies/hpcc/HPL/vass/hpl.hpcc2012.chpl | 38 ++-- .../hpcc/HPL/vass/hpl.performance.chpl | 32 ++-- test/studies/prk/Stencil/stencil.chpl | 2 +- 7 files changed, 190 insertions(+), 68 deletions(-) diff --git a/modules/dists/StencilDist.chpl b/modules/dists/StencilDist.chpl index 2dc70a35d285..00c2d78f91cc 100644 --- a/modules/dists/StencilDist.chpl +++ b/modules/dists/StencilDist.chpl @@ -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; @@ -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 { @@ -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); @@ -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(), @@ -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); @@ -519,7 +620,7 @@ 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 && @@ -527,27 +628,27 @@ proc Stencil.dsiEqualDMaps(that: Stencil(?)) { } -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); @@ -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"); @@ -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); @@ -602,11 +703,11 @@ 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)); } @@ -614,7 +715,7 @@ proc Stencil.dsiIndexToLocale(ind: rank*idxType) where rank > 1 { // 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 // @@ -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, @@ -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)); @@ -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 @@ -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); @@ -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) { @@ -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; @@ -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); diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 002d978cb4d9..bb27694e6356 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -691,7 +691,7 @@ 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) ref { return x; } @@ -699,7 +699,7 @@ module ChapelArray { 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) ref { compilerWarning("The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new ())' with 'new ()'"); return chpl__buildDistValue(x); } diff --git a/test/deprecated/dmapType.chpl b/test/deprecated/dmapType.chpl index 8eefd4d77b46..a0fde7d01e7b 100644 --- a/test/deprecated/dmapType.chpl +++ b/test/deprecated/dmapType.chpl @@ -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); +} diff --git a/test/deprecated/dmapType.good b/test/deprecated/dmapType.good index d23d430cafba..baa28ee474e7 100644 --- a/test/deprecated/dmapType.good +++ b/test/deprecated/dmapType.good @@ -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(())' with '()' dmapType.chpl:24: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new ())' with 'new ()' dmapType.chpl:28: warning: The use of 'dmap' is depreacted for this distribution; please replace 'dmap(())' with '()' +dmapType.chpl:34: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new ())' with 'new ()' +dmapType.chpl:38: warning: The use of 'dmap' is depreacted for this distribution; please replace 'dmap(())' with '()' 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) diff --git a/test/studies/hpcc/HPL/vass/hpl.hpcc2012.chpl b/test/studies/hpcc/HPL/vass/hpl.hpcc2012.chpl index 3d1e7750d573..a4e1d34a86cf 100644 --- a/test/studies/hpcc/HPL/vass/hpl.hpcc2012.chpl +++ b/test/studies/hpcc/HPL/vass/hpl.hpcc2012.chpl @@ -2,7 +2,7 @@ // Use standard modules for vector and matrix Norms, Random numbers // and Timing routines // -use LinearAlgebra, Random, Time; +use LinearAlgebra, Random, Time, Math, ChplConfig; // // Use the user module for computing HPCC problem sizes @@ -12,8 +12,8 @@ use LinearAlgebra, Random, Time; // // Use the distributions we need for this computation // -use DimensionalDist2D, ReplicatedDim, BlockCycDim; -use ReplicatedDist, ChplConfig; +use DimensionalDist2D, ReplicatedDim, BlockCycDim, DSIUtil; +use ReplicatedDist; // // The number of matrices and the element type of those matrices @@ -93,10 +93,10 @@ compilerAssert(CHPL_NETWORK_ATOMICS == "none", // Replicate 'targetLocales' to reduce comm. // 'targetLocales' itself could be replicated instead, // but this way the changes are smaller. - const targetIdsRepl = targetIds dmapped new dmap(distReplicated); + const targetIdsRepl = targetIds dmapped distReplicated; var targetLocalesRepl: [targetIdsRepl] locale; coforall (l, id) in zip(Locales, LocaleSpace) do on l { - targetLocalesRepl._value.localArrs[id].arrLocalRep = targetLocales; + targetLocalesRepl._value.localArrs[id]!.arrLocalRep = targetLocales; } // Create individual dimension descriptors @@ -146,7 +146,7 @@ compilerAssert(CHPL_NETWORK_ATOMICS == "none", const replKD = {0..0, 1..n+1} dmapped new dmap(dist1r2b); var replK: [replKD] elemType; - const replUD = {0..#blkSize, 0..#blkSize} dmapped new dmap(distReplicated); + const replUD = {0..#blkSize, 0..#blkSize} dmapped distReplicated; // can't use targetLocales because of "consistency" req. - that's OK var replU: [replUD] elemType; @@ -172,7 +172,7 @@ compilerAssert(CHPL_NETWORK_ATOMICS == "none", if elemType == real(64) then chpl__processorAtomicType(real) else compilerError("need to define cpuAtomicElemType"); type cpuAtomicCountType = chpl__processorAtomicType(int); - const replPD = {0..#blkSize} dmapped new dmap(distReplicated); + const replPD = {0..#blkSize} dmapped distReplicated; // for the working 'x' vector, replX, we'll reuse replK @@ -192,7 +192,7 @@ compilerAssert(CHPL_NETWORK_ATOMICS == "none", } // (Only needed on diagonal nodes, for simplicity create everywhere.) - const bsOthersPartSumsD = {0..#tl2} dmapped new dmap(distReplicated); + const bsOthersPartSumsD = {0..#tl2} dmapped distReplicated; var bsOthersPartSums: [bsOthersPartSumsD] bsSetofPartSums; initAB(); @@ -494,12 +494,12 @@ proc psReduce(blk, k) { ref locAB = Ab._value.dsiLocalSlice1((iRange, k)); for i in iRange do myResult.updateE(i, locAB[i]); if myResult.absmx > maxRes.read() { // only lock if we might update - upd$ = true; // lock + upd$.writeEF(true); // lock if myResult.absmx > maxRes.read() { // check again while locked locResult.updateE(myResult); maxRes.write(myResult.absmx); } - upd$; // unlock + upd$.readFE(); // unlock } } // forall } // local @@ -520,7 +520,7 @@ proc psReduce(blk, k) { proc psCompute(panel, blk, k, pivotVal) { if k == n then return; // nothing to do - const dim2end = panel.dim(1).alignedHigh; + const dim2end = panel.dim(1).high; const lid2 = targetLocalesIndexForAbIndex(2, k); coforall lid1 in 0..#tl1 { @@ -647,7 +647,7 @@ proc updateBlockRow( const blkStarts = tr.dim(1)[.. by blkSize align 1]; const lid1 = targetLocalesIndexForAbIndex(1, blk); - const blkStartsStart = blkStarts.alignedLow; + const blkStartsStart = blkStarts.low; coforall lid2 in 0..#tl2 { on targetLocalesRepl[lid1, lid2] { @@ -659,7 +659,7 @@ proc updateBlockRow( local { const dim2 = js..min(js+blkSize-1,n+1); ref locAB = Ab._value.dsiLocalSlice1((dim1, dim2)); - ref locU = replU._value.localArrs[here.id].arrLocalRep; + ref locU = replU._value.localArrs[here.id]!.arrLocalRep; for row in dim1 { const i = row, iRel = i-blk; @@ -774,7 +774,7 @@ proc bsComputeRow(diaFrom, diaTo, locId1, locId2, diaLocId2) { } else { // off the diagonal - ref myPartSums = bsLocalPartSums._value.localArrs[here.id].arrLocalRep; + ref myPartSums = bsLocalPartSums._value.localArrs[here.id]!.arrLocalRep; ensureDR(myPartSums._value, "bsR myPartSums"); var gotBlocks: bool; @@ -829,7 +829,7 @@ proc bsComputeMyXsWithB(diaFrom, diaTo, locAB, locX, locB) { proc bsIncorporateOthersPartSums(diaFrom, diaTo, locId1, locId2) { // Apart from asserts and writeln(), everything here should be local. - ref partSums = bsOthersPartSums._value.localArrs[here.id].arrLocalRep; + ref partSums = bsOthersPartSums._value.localArrs[here.id]!.arrLocalRep; ref locX = replK._value.dsiLocalSlice1((0, diaFrom..diaTo)); ensureDR(partSums._value, "bsI partSums"); ensureDR(locX._value, "bsI locX"); @@ -999,7 +999,7 @@ proc replicateA(abIx, dim2) { ref locAB = Ab._value.dsiLocalSlice1((iRange, dim2)); // locReplA[iRange,..] = locAB[iRange, dim2]; - const jStart = dim2.alignedLow, + const jStart = dim2.low, rStart = replA._value.dom.dom1._dsiStorageIdx(iStart); for i in iRange { for j in dim2 do @@ -1034,7 +1034,7 @@ proc replicateB(abIx, dim1) { ref locAB = Ab._value.dsiLocalSlice1((dim1, jRange)); // locReplB[..,jRange] = locAB[dim1, jRange]; - const iStart = dim1.alignedLow, + const iStart = dim1.low, rStart = replB._value.dom.dom2._dsiStorageIdx(jStart); for i in dim1 { for j in jRange do @@ -1068,10 +1068,10 @@ proc replicateU(abIx) { const fromLocId1 = targetLocalesIndexForAbIndex(1, abIx); const fromLocId2 = targetLocalesIndexForAbIndex(2, abIx); // verify that we are on a good locale, optionally - ref myLocalArr = replU._value.localArrs[here.id].arrLocalRep; + ref myLocalArr = replU._value.localArrs[here.id]!.arrLocalRep; coforall targloc in targetLocales[fromLocId1, ..] do if targloc != here then - replU._value.localArrs[targloc.id].arrLocalRep = myLocalArr; + replU._value.localArrs[targloc.id]!.arrLocalRep = myLocalArr; } proc targetLocalesIndexForAbIndex(param dim, abIx) do diff --git a/test/studies/hpcc/HPL/vass/hpl.performance.chpl b/test/studies/hpcc/HPL/vass/hpl.performance.chpl index ab4c5d3b1885..762ad3d11f6e 100644 --- a/test/studies/hpcc/HPL/vass/hpl.performance.chpl +++ b/test/studies/hpcc/HPL/vass/hpl.performance.chpl @@ -2,7 +2,7 @@ // Use standard modules for vector and matrix Norms, Random numbers // and Timing routines // -use LinearAlgebra, Random, Time; +use LinearAlgebra, Random, Time, Math; // // Use the user module for computing HPCC problem sizes @@ -12,7 +12,7 @@ use HPCCProblemSize; // // Use the distributions we need for this computation // -use DimensionalDist2D, ReplicatedDim, BlockCycDim; +use DimensionalDist2D, ReplicatedDim, BlockCycDim, DSIUtil; use ReplicatedDist; // @@ -106,10 +106,10 @@ var tInit, tPS1iter, tUBR1iter, tSC1call, tLF1iter, tBScall, tVer: VTimer; // Replicate 'targetLocales' to reduce comm. // 'targetLocales' itself could be replicated instead, // but this way the changes are smaller. - const targetIdsRepl = targetIds dmapped new dmap(distReplicated); + const targetIdsRepl = targetIds dmapped distReplicated; var targetLocalesRepl: [targetIdsRepl] locale; coforall (l, id) in zip(Locales, LocaleSpace) do on l { - targetLocalesRepl._value.localArrs[id].arrLocalRep = targetLocales; + targetLocalesRepl._value.localArrs[id]!.arrLocalRep = targetLocales; } vmsg("replicated targetLocales"); @@ -161,7 +161,7 @@ var tInit, tPS1iter, tUBR1iter, tSC1call, tLF1iter, tBScall, tVer: VTimer; const replKD = {0..0, 1..n+1} dmapped new dmap(dist1r2b); var replK: [replKD] elemType; - const replUD = {0..#blkSize, 0..#blkSize} dmapped new dmap(distReplicated); + const replUD = {0..#blkSize, 0..#blkSize} dmapped distReplicated; // can't use targetLocales because of "consistency" req. - that's OK var replU: [replUD] elemType; @@ -174,7 +174,7 @@ var tInit, tPS1iter, tUBR1iter, tSC1call, tLF1iter, tBScall, tVer: VTimer; if elemType == real(64) then chpl__processorAtomicType(real) else compilerError("need to define cpuAtomicElemType"); type cpuAtomicCountType = chpl__processorAtomicType(int); - const replPD = {0..#blkSize} dmapped new dmap(distReplicated); + const replPD = {0..#blkSize} dmapped distReplicated; // the working 'x' vector - we reuse replK // @@ -200,7 +200,7 @@ var tInit, tPS1iter, tUBR1iter, tSC1call, tLF1iter, tBScall, tVer: VTimer; } // (Only needed on diagonal nodes, for simplicity create everywhere.) - const bsOthersPartSumsD = {0..#tl2} dmapped new dmap(distReplicated); + const bsOthersPartSumsD = {0..#tl2} dmapped distReplicated; var bsOthersPartSums: [bsOthersPartSumsD] bsSetofPartSums; initAB(); @@ -703,7 +703,7 @@ proc bsComputeRow(diaFrom, diaTo, locId1, locId2, diaLocId2) { } else { // off the diagonal - ref myPartSums = bsLocalPartSums._value.localArrs[here.id].arrLocalRep; + ref myPartSums = bsLocalPartSums._value.localArrs[here.id]!.arrLocalRep; ensureDR(myPartSums._value, "bsR myPartSums"); if checkBsub { @@ -774,7 +774,7 @@ proc bsComputeMyXsWithB(diaFrom, diaTo, locAB, locX, locB) { proc bsIncorporateOthersPartSums(diaFrom, diaTo, locId1, locId2) { // Apart from asserts and writeln(), everything here should be local. - ref partSums = bsOthersPartSums._value.localArrs[here.id].arrLocalRep; + ref partSums = bsOthersPartSums._value.localArrs[here.id]!.arrLocalRep; ref locX = replX._value.dsiLocalSlice1((0, diaFrom..diaTo)); ensureDR(partSums._value, "bsI partSums"); ensureDR(locX._value, "bsI locX"); @@ -959,8 +959,8 @@ iter initABalt(param tag: iterKind, followThis): real //writeln("follower ", followThis, " on ", here.id); // we know that followThis.low will be >= 0 - const f1 = followThis(1).low + 1, - f2 = followThis(2).low + 1; + const f1 = followThis(0).low + 1, + f2 = followThis(1).low + 1; var cur = iaStart + 0.77 / f1 + 0.83 / f2; const step = iaStep / f1 / f2; @@ -973,8 +973,8 @@ iter initABalt(param tag: iterKind, followThis): real } if boundsChecking { - for f1 in followThis(1) do - for f2 in followThis(2) do + for f1 in followThis(0) do + for f2 in followThis(1) do yield advance(); } else { // we don't care how many times to do this @@ -1095,10 +1095,10 @@ proc replicateU(abIx) { here.id, " expecting ", fromLocale.id, " ", (fromLocId1, fromLocId2)); } - ref myLocalArr = replU._value.localArrs[here.id].arrLocalRep; + ref myLocalArr = replU._value.localArrs[here.id]!.arrLocalRep; coforall targloc in targetLocales[fromLocId1, ..] do if targloc != here then - replU._value.localArrs[targloc.id].arrLocalRep = myLocalArr; + replU._value.localArrs[targloc.id]!.arrLocalRep = myLocalArr; } proc targetLocalesIndexForAbIndex(param dim, abIx) do @@ -1163,7 +1163,7 @@ proc gaxpyMinus(A: [], // This confirms that our intentions, for efficiency, are fulfilled. proc ensureDR(value, param msg) { - proc etest(type t) param do where t : DefaultRectangularArr return true; + proc etest(type t) param where t : DefaultRectangularArr do return true; proc etest(type t) param do return false; compilerAssert(etest(value.type), "ensureDR ", msg, 2); } diff --git a/test/studies/prk/Stencil/stencil.chpl b/test/studies/prk/Stencil/stencil.chpl index b7c161a978b8..4aa0187b3830 100644 --- a/test/studies/prk/Stencil/stencil.chpl +++ b/test/studies/prk/Stencil/stencil.chpl @@ -92,7 +92,7 @@ proc main() { /* Flavors of parallelism, by distribution */ const blockDist = new Block(localDom), - stencilDist = new dmap(new Stencil(innerLocalDom, fluff=(R,R))), + stencilDist = new Stencil(innerLocalDom, fluff=(R,R)), noDist = defaultDist; /* Set distribution based on configs */ From 9a331fa20060c9097ff06b99b6039e3119000ab6 Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Mon, 21 Aug 2023 10:44:15 -0700 Subject: [PATCH 2/5] Add a test I missed --- Signed-off-by: Brad Chamberlain --- test/studies/prk/Stencil/optimized/stencil-opt.chpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/studies/prk/Stencil/optimized/stencil-opt.chpl b/test/studies/prk/Stencil/optimized/stencil-opt.chpl index 3f62b7e24aa5..72ae957547a4 100644 --- a/test/studies/prk/Stencil/optimized/stencil-opt.chpl +++ b/test/studies/prk/Stencil/optimized/stencil-opt.chpl @@ -69,8 +69,8 @@ proc main() { /* Domain over entire 'active' region, where Output will be updated */ innerLocalDom = localDom.expand(-R); - const outDist = new dmap(new Stencil(innerLocalDom)); - const fluffDist = new dmap(new Stencil(innerLocalDom, fluff=(R,R))); + const outDist = new Stencil(innerLocalDom); + const fluffDist = new Stencil(innerLocalDom, fluff=(R,R)); const Dom = localDom dmapped fluffDist, innerDom = innerLocalDom dmapped fluffDist; From 361b85eab0d6d876eac18105df0ecbaba89193c0 Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Mon, 21 Aug 2023 16:05:26 -0700 Subject: [PATCH 3/5] Add in a test that locks in the 'const ref' intent changes I made in this PR Specifically, to `chpl__buildDist[DMap]Value()` --- Signed-off-by: Brad Chamberlain --- test/deprecated/dmapConstRecord.chpl | 6 ++++++ test/deprecated/dmapConstRecord.good | 1 + 2 files changed, 7 insertions(+) create mode 100644 test/deprecated/dmapConstRecord.chpl create mode 100644 test/deprecated/dmapConstRecord.good diff --git a/test/deprecated/dmapConstRecord.chpl b/test/deprecated/dmapConstRecord.chpl new file mode 100644 index 000000000000..a8eb43a71833 --- /dev/null +++ b/test/deprecated/dmapConstRecord.chpl @@ -0,0 +1,6 @@ +use BlockDist; + +config const n = 10; + +const block = new Block({1..n}); +var D = {1..n} dmapped new dmap(block); diff --git a/test/deprecated/dmapConstRecord.good b/test/deprecated/dmapConstRecord.good new file mode 100644 index 000000000000..5109f8eb3ca7 --- /dev/null +++ b/test/deprecated/dmapConstRecord.good @@ -0,0 +1 @@ +dmapConstRecord.chpl:6: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new ())' with 'new ()' From 778adfcce5b0ffa8d68657f4b4c52947fc6bc927 Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Mon, 21 Aug 2023 16:10:20 -0700 Subject: [PATCH 4/5] Fix an apparent bug that the compiler didn't complain about... A 'const ref' argument was returned by 'ref' intent. --- Signed-off-by: Brad Chamberlain --- modules/internal/ChapelArray.chpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index bb27694e6356..21f3c50e6f91 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -691,7 +691,7 @@ module ChapelArray { proc chpl__buildDistValue(in x:owned) where isSubtype(x.borrow().type, BaseDist) { return new _distribution(owned.release(x)); } - proc chpl__buildDistValue(const ref x:record) ref { + proc chpl__buildDistValue(const ref x:record) const ref { return x; } @@ -699,7 +699,7 @@ module ChapelArray { compilerError("illegal domain map value specifier - must be a subclass of BaseDist"); } - proc chpl__buildDistDMapValue(const 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 ())' with 'new ()'"); return chpl__buildDistValue(x); } From 1d068179aac76e35878fd7ef0758060eac6dab45 Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Mon, 21 Aug 2023 16:18:30 -0700 Subject: [PATCH 5/5] Add future showing that 'const ref' actuals can be returned by 'ref' (and the original 'const' thing modified as a result...) --- Signed-off-by: Brad Chamberlain --- test/functions/intents/constRefArgRefReturn.bad | 2 ++ test/functions/intents/constRefArgRefReturn.chpl | 9 +++++++++ test/functions/intents/constRefArgRefReturn.future | 3 +++ test/functions/intents/constRefArgRefReturn.good | 1 + 4 files changed, 15 insertions(+) create mode 100644 test/functions/intents/constRefArgRefReturn.bad create mode 100644 test/functions/intents/constRefArgRefReturn.chpl create mode 100644 test/functions/intents/constRefArgRefReturn.future create mode 100644 test/functions/intents/constRefArgRefReturn.good diff --git a/test/functions/intents/constRefArgRefReturn.bad b/test/functions/intents/constRefArgRefReturn.bad new file mode 100644 index 000000000000..6cc8c52533f9 --- /dev/null +++ b/test/functions/intents/constRefArgRefReturn.bad @@ -0,0 +1,2 @@ +43 +43 diff --git a/test/functions/intents/constRefArgRefReturn.chpl b/test/functions/intents/constRefArgRefReturn.chpl new file mode 100644 index 000000000000..71ca20320b80 --- /dev/null +++ b/test/functions/intents/constRefArgRefReturn.chpl @@ -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); diff --git a/test/functions/intents/constRefArgRefReturn.future b/test/functions/intents/constRefArgRefReturn.future new file mode 100644 index 000000000000..aa899a4e908d --- /dev/null +++ b/test/functions/intents/constRefArgRefReturn.future @@ -0,0 +1,3 @@ +bug: compiler permits 'const ref' formals to be returned as 'ref' + +(resulting in the ability to change 'const' values) diff --git a/test/functions/intents/constRefArgRefReturn.good b/test/functions/intents/constRefArgRefReturn.good new file mode 100644 index 000000000000..091d04f72c99 --- /dev/null +++ b/test/functions/intents/constRefArgRefReturn.good @@ -0,0 +1 @@ +constRefArgRefReturn.chpl:1: error: Cannot return a const expression as a non-const reference.