Convert the BlockCyclic class to a record blockCycDist (chapel-la…

…ng#23399)

[reviewed by @arezaii ]

This follows in the footsteps of other recent PRs, converting the
`BlockCyclic` class to a record `blockCycDist`. I'm not crazy about the
name (would probably choose `blkCycDist` for brevity and symmetry), but
figured I'd follow the module name as we do with the other cases, and
that we could rename things as we stabilize this distribution and
module.

Changes here are fairly standard:
* introduce record that wraps our helper record and defines the things
it must
* rename the class to `BlockCyclicImpl`
* add a deprecated type alias for `BlockCyclic`
* add the dsiGetDist() method to `BlockCyclicDom`
* add a case for BlockCyclic to my deprecation tests
* update the tests

Because this distribution is not stabilized, I spent less care with the
documentation than with the others, just renaming things that were
clearly wrong. Also fixed an issue in ReplicatedDist that Jade had
pointed out earlier, but that I'd apparently never fixed?

modules/dists/BlockCycDist.chpl

@@ -23,7 +23,7 @@ prototype module BlockCycDist {
 //
 // BlockCyclic Distribution
 //
-//      BlockCyclic    BlockCyclicDom     BlockCyclicArr
+//  BlockCyclicImpl    BlockCyclicDom     BlockCyclicArr
 //
 //   LocBlockCyclic    LocBlockCyclicDom  LocBlockCyclicArr
 //
@@ -63,11 +63,11 @@ proc _ensureTuple(arg) {
 
 
 ////////////////////////////////////////////////////////////////////////////////
-// BlockCyclic Distribution Class
+// BlockCyclicImpl Distribution Class
 //
 /*
 
-This Block-Cyclic distribution maps maps blocks of indices to locales in a
+The ``blockCycDist`` distribution maps blocks of indices to locales in a
 round-robin pattern according to a given block size and start index.
 
 Formally, consider a Block-Cyclic distribution with:
@@ -111,12 +111,12 @@ to the ID of the locale to which it is mapped.
 
     const Space = {1..8, 1..8};
     const D: domain(2)
-      dmapped BlockCyclic(startIdx=Space.lowBound,blocksize=(2,3))
+      dmapped blockCycDist(startIdx=Space.lowBound,blocksize=(2,3))
       = Space;
     var A: [D] int;
 
     forall a in A do
-      a = a.locale.id;
+      a = a.here.id;
 
     writeln(A);
 
@@ -136,11 +136,11 @@ When run on 6 locales, the output is:
 
 **Initializer Arguments**
 
-The ``BlockCyclic`` class initializer is defined as follows:
+The ``blockCycDist`` initializer is defined as follows:
 
   .. code-block:: chapel
 
-    proc BlockCyclic.init(
+    proc blockCycDist.init(
       startIdx,
       blocksize,
       targetLocales: [] locale = Locales,
@@ -172,7 +172,7 @@ of tasks on each Locale for data parallelism.
 The ``rank`` and ``idxType`` arguments are inferred from the
 ``startIdx`` argument unless explicitly set.
 They must match the rank and index type of the domains
-"dmapped" using that BlockCyclic instance.
+"dmapped" using that blockCycDist instance.
 
 
 **Data-Parallel Iteration**
@@ -182,7 +182,107 @@ executes each iteration on the locale where that iteration's index
 is mapped to.
 
 */
-class BlockCyclic : BaseDist, writeSerializable {
+
+
+record blockCycDist: writeSerializable {
+  param rank: int;
+  type idxType = int;
+
+  forwarding const chpl_distHelp: chpl_PrivatizedDistHelper(unmanaged BlockCyclicImpl(rank, idxType));
+
+  proc init(startIdx,
+            blocksize,
+            targetLocales: [] locale = Locales,
+            dataParTasksPerLocale=getDataParTasksPerLocale(),
+            param rank = _determineRankFromArg(startIdx),
+            type idxType = _determineIdxTypeFromArg(startIdx)) {
+    const value = new unmanaged BlockCyclicImpl(startIdx, blocksize,
+                                                targetLocales,
+                                          dataParTasksPerLocale,
+                                          rank, idxType);
+    this.rank = rank;
+    this.idxType = idxType;
+    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.chpl_distHelp = new chpl_PrivatizedDistHelper(_pid,
+                                                         _instance,
+                                                         _unowned);
+    }
+
+    proc init(value) {
+      this.rank = value.rank;
+      this.idxType = value.idxType;
+      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 : blockCycDist(?)) {
+      this.init(other._value.dsiClone());
+    }
+
+    proc clone() {
+      return new blockCycDist(this._value.dsiClone());
+    }
+
+  @chpldoc.nodoc
+  inline operator ==(d1: blockCycDist(?), d2: blockCycDist(?)) {
+    if (d1._value == d2._value) then
+      return true;
+    return d1._value.dsiEqualDMaps(d2._value);
+  }
+
+  @chpldoc.nodoc
+  inline operator !=(d1: blockCycDist(?), d2: blockCycDist(?)) {
+    return !(d1 == d2);
+  }
+
+  proc writeThis(x) {
+    chpl_distHelp.writeThis(x);
+  }
+
+  proc serialize(writer, ref serializer) throws {
+    writeThis(writer);
+  }
+}
+
+
+@chpldoc.nodoc
+@unstable(category="experimental", reason="assignment between distributions is currently unstable due to lack of testing")
+operator =(ref a: blockCycDist(?), b: blockCycDist(?)) {
+  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);
+  }
+}
+
+
+@deprecated("'BlockCyclic' is deprecated, please use 'blockCycDist' instead")
+type BlockCyclic = blockCycDist;
+
+
+class BlockCyclicImpl : BaseDist, writeSerializable {
   param rank: int;
   type idxType = int;
 
@@ -246,7 +346,7 @@ class BlockCyclic : BaseDist, writeSerializable {
   }
 
   // copy initializer for privatization
-  proc init(param rank: int, type idxType, other: unmanaged BlockCyclic(rank, idxType)) {
+  proc init(param rank: int, type idxType, other: unmanaged BlockCyclicImpl(rank, idxType)) {
     this.rank = rank;
     this.idxType = idxType;
     lowIdx = other.lowIdx;
@@ -258,7 +358,7 @@ class BlockCyclic : BaseDist, writeSerializable {
   }
 
   proc dsiClone() {
-    return new unmanaged BlockCyclic(lowIdx, blocksize, targetLocales, dataParTasksPerLocale);
+    return new unmanaged BlockCyclicImpl(lowIdx, blocksize, targetLocales, dataParTasksPerLocale);
   }
 
   override proc dsiDestroyDist() {
@@ -268,7 +368,7 @@ class BlockCyclic : BaseDist, writeSerializable {
     }
   }
 
-  proc dsiEqualDMaps(that: BlockCyclic(?)) {
+  proc dsiEqualDMaps(that: BlockCyclicImpl(?)) {
     //
     // TODO: In retrospect, I think that this equality check
     // is too simple.  Since a distribution distributes the
@@ -290,7 +390,7 @@ class BlockCyclic : BaseDist, writeSerializable {
   }
 }
 
-proc BlockCyclic._locsize {
+proc BlockCyclicImpl._locsize {
   var ret : rank*int;
   for param i in 0..rank-1 {
     ret(i) = targetLocDom.dim(i).sizeAs(int);
@@ -301,7 +401,7 @@ proc BlockCyclic._locsize {
 //
 // create a new rectangular domain over this distribution
 //
-override proc BlockCyclic.dsiNewRectangularDom(param rank: int, type idxType,
+override proc BlockCyclicImpl.dsiNewRectangularDom(param rank: int, type idxType,
                                       param strides: strideKind, inds) {
   if idxType != this.idxType then
     compilerError("BlockCyclic domain index type does not match distribution's");
@@ -318,9 +418,9 @@ override proc BlockCyclic.dsiNewRectangularDom(param rank: int, type idxType,
 //
 // output distribution
 //
-proc BlockCyclic.writeThis(x) throws {
-  x.writeln("BlockCyclic");
-  x.writeln("-------");
+proc BlockCyclicImpl.writeThis(x) throws {
+  x.writeln("blockCycDist");
+  x.writeln("------------");
   x.writeln("distributes: ", lowIdx, "...");
   x.writeln("in chunks of: ", blocksize);
   x.writeln("across locales: ", targetLocales);
@@ -329,26 +429,26 @@ proc BlockCyclic.writeThis(x) throws {
   for locid in targetLocDom do
     x.writeln("  [", locid, "] ", locDist(locid));
 }
-override proc BlockCyclic.serialize(writer, ref serializer) throws {
+override proc BlockCyclicImpl.serialize(writer, ref serializer) throws {
   writeThis(writer);
 }
 
 //
 // convert an index into a locale value
 //
-proc BlockCyclic.dsiIndexToLocale(ind: idxType) where rank == 1 {
+proc BlockCyclicImpl.dsiIndexToLocale(ind: idxType) where rank == 1 {
   return targetLocales(idxToLocaleInd(ind));
 }
 
-proc BlockCyclic.dsiIndexToLocale(ind: rank*idxType) {
+proc BlockCyclicImpl.dsiIndexToLocale(ind: rank*idxType) {
   return targetLocales(idxToLocaleInd(ind));
 }
 
 //
 // compute what chunk of inds is owned by a given locale -- assumes
 // it's being called on the locale in question
 //
-proc BlockCyclic.getStarts(inds, locid) {
+proc BlockCyclicImpl.getStarts(inds, locid) {
   // use domain slicing to get the intersection between what the
   // locale owns and the domain's index set
   //
@@ -403,17 +503,17 @@ proc BlockCyclic.getStarts(inds, locid) {
 // someone else can help me remember it (since it was probably someone
 // else's suggestion).
 //
-proc BlockCyclic.idxToLocaleInd(ind: idxType) where rank == 1 {
+proc BlockCyclicImpl.idxToLocaleInd(ind: idxType) where rank == 1 {
   const ind0 = ind - lowIdx(0);
   //  compilerError((ind0/blocksize(0)%targetLocDom.dim(0).type:string);
   return (ind0 / blocksize(0)) % targetLocDom.dim(0).size;
 }
 
-proc BlockCyclic.idxToLocaleInd(ind: rank*idxType) where rank == 1 {
+proc BlockCyclicImpl.idxToLocaleInd(ind: rank*idxType) where rank == 1 {
   return idxToLocaleInd(ind(0));
 }
 
-proc BlockCyclic.idxToLocaleInd(ind: rank*idxType) where rank != 1 {
+proc BlockCyclicImpl.idxToLocaleInd(ind: rank*idxType) where rank != 1 {
   var locInd: rank*int;
   for param i in 0..rank-1 {
     const ind0 = ind(i) - lowIdx(i);
@@ -422,7 +522,7 @@ proc BlockCyclic.idxToLocaleInd(ind: rank*idxType) where rank != 1 {
   return locInd;
 }
 
-proc BlockCyclic.init(other: BlockCyclic, privatizeData,
+proc BlockCyclicImpl.init(other: BlockCyclicImpl, privatizeData,
                       param rank = other.rank, type idxType = other.idxType) {
   this.rank = rank;
   this.idxType = idxType;
@@ -434,14 +534,14 @@ proc BlockCyclic.init(other: BlockCyclic, privatizeData,
   dataParTasksPerLocale = privatizeData[3];
 }
 
-override proc BlockCyclic.dsiSupportsPrivatization() param do return true;
+override proc BlockCyclicImpl.dsiSupportsPrivatization() param do return true;
 
-proc BlockCyclic.dsiGetPrivatizeData() {
+proc BlockCyclicImpl.dsiGetPrivatizeData() {
   return (lowIdx, blocksize, targetLocDom.dims(), dataParTasksPerLocale);
 }
 
-proc BlockCyclic.dsiPrivatize(privatizeData) {
-  return new unmanaged BlockCyclic(_to_unmanaged(this), privatizeData);
+proc BlockCyclicImpl.dsiPrivatize(privatizeData) {
+  return new unmanaged BlockCyclicImpl(_to_unmanaged(this), privatizeData);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -506,7 +606,7 @@ class BlockCyclicDom: BaseRectangularDom(?) {
   //
   // LEFT LINK: a pointer to the parent distribution
   //
-  const dist: unmanaged BlockCyclic(rank, idxType);
+  const dist: unmanaged BlockCyclicImpl(rank, idxType);
 
   //
   // DOWN LINK: an array of local domain class descriptors -- set up in
@@ -743,6 +843,13 @@ proc BlockCyclicDom.dsiReprivatize(other, reprivatizeData) {
   whole = other.whole;
 }
 
+proc BlockCyclicDom.dsiGetDist() {
+  if _isPrivatized(dist) then
+    return new blockCycDist(dist.pid, dist, _unowned=true);
+  else
+    return new blockCycDist(nullPid, dist, _unowned=true);
+}
+
 
 ////////////////////////////////////////////////////////////////////////////////
 // BlockCyclic Local Domain Class
@@ -1058,7 +1165,7 @@ proc BlockCyclicArr.dsiTargetLocales() const ref {
 proc BlockCyclicDom.dsiTargetLocales() const ref {
   return dist.targetLocales;
 }
-proc BlockCyclic.dsiTargetLocales() const ref {
+proc BlockCyclicImpl.dsiTargetLocales() const ref {
   return targetLocales;
 }
 

modules/dists/ReplicatedDist.chpl

@@ -96,7 +96,7 @@ distribution.
 
 **Initializer Arguments**
 
-The ``replicatedDist`` class initializer is defined as follows:
+The ``replicatedDist`` initializer is defined as follows:
 
   .. code-block:: chapel
 

modules/dists/dims/BlockCycDim.chpl

@@ -45,7 +45,7 @@ This Block-Cyclic dimension specifier is for use with the
 :class:`DimensionalDist2D` distribution.
 
 It specifies the mapping of indices in its dimension
-that would be produced by a 1D :class:`~BlockCycDist.BlockCyclic` distribution.
+that would be produced by a 1D :class:`~BlockCycDist.blockCycDist` distribution.
 
 **Initializer Arguments**
 
@@ -66,7 +66,7 @@ The arguments are as follows:
       to be distributed over
   ``lowIdx``, ``blockSize``
       are the counterparts to ``startIdx`` and ``blocksize``
-      in the :class:`~BlockCycDist.BlockCyclic` distribution
+      in the :class:`~BlockCycDist.blockCycDist` distribution
   ``cycleSizePos``
       is used internally by the implementation and
       should not be specified by the user code

test/arrays/ferguson/array-initialization-patterns/array-initialization-patterns-dists.chpl

@@ -65,7 +65,7 @@ proc makeInitialArray() {
   } else if distType == DistType.cyclic {
     return cyclicDist.createArray(1..1, int);
   } else if distType == DistType.blockcyclic {
-    var D = {1..1} dmapped BlockCyclic(startIdx=(1,), (3,));
+    var D = {1..1} dmapped blockCycDist(startIdx=(1,), (3,));
     var ret: [D] int;
     return ret;
   } else if distType == DistType.replicated {

test/arrays/ferguson/array-initialization-patterns/post-alloc-dists.chpl

@@ -22,7 +22,7 @@ proc makeA() {
   } else if distType == DistType.cyclic {
     return cyclicDist.createArray(1..1, int);
   } else if distType == DistType.blockcyclic {
-    var D = {1..1} dmapped BlockCyclic(startIdx=(1,), (3,));
+    var D = {1..1} dmapped blockCycDist(startIdx=(1,), (3,));
     var ret: [D] int;
     return ret;
   } else if distType == DistType.replicated {

test/deprecated/dmapType.chpl

@@ -62,3 +62,14 @@ config var n = 10, addLeaks=false;
     writeln(Dist2.type:string);
   }
 }
+
+{
+  use BlockCycDist;
+  var Dist = new dmap(new BlockCyclic(1, 2));
+  var Dom = {1..n} dmapped Dist;
+  var A: [Dom] real;
+  writeln(A);
+  const Dist2: dmap(BlockCyclic(1)) = new BlockCyclic(1, 2);
+  writeln(Dist2.type:string);
+}
+