Oops, the Michael Ferguson bug bit me and I forgot the patch. Attached
now.
-Brad
On Fri, 24 Jan 2014, Brad Chamberlain wrote:
Hi Chapel Devs --
This is a request for a proposed change to the interpretation of Chapel
programs, which anyone invested in the language should give a quick look at
and then respond to two questions below. There's also a patch to implement
the change (which I'll need a reviewer for as soon as we verify that we have
consensus on it).
For some time now, Chapel has permitted a domain to be specified either
by naming it, e.g.:
const D = {1..n};
const DistD = D dmapped Block(...);
forall i in DistD ...
or by wrapping it in an extraneous set of curly brackets:
const D = {1..n};
const DistD = {D} dmapped Block(...);
forall i in {DistD} ...
The reasons for this are somewhat obscure (and arguably indefensible), but
relate to (1) a precedent set in ZPL, (2) the days when we used square
brackets for domain literals, and (3) to the use of anonymous domains in
array declarations without curly brackets (i.e., a pure implementation of
Chapel would require
var A: [{1..n, 1..n}] real;
rather than permitting the shorthand:
var A: [1..n, 1..n] real;
I'd forgotten about this anomaly until doing cleanup on iterations over
anonymous domains the other week when I encountered some instances of it.
Over lunch, Vass pointed out that cleaning this up should be fairly easy so I
took it on as a five-minute fix (TM).
I'm proposing that instead, we interpret {D} (where 'D' is a domain) as an
associative domain of domain values, containing one index 'D' which seems
more consistent with the current language definition.
In implementing this, I also realized for the first time (or re-realized
something I'd forgotten), that as an extension to the sugaring of anonymous
domains in array declarations, that we permit:
var A: [1, 2, 4] real;
to mean "A is declared over an anonymous associative domain {1, 2, 4}. I'm
not sure whether this is a good thing or not. I think the argument for
sugaring this case is weaker since programmers from C and Fortran won't have
a corresponding concept that they'll expect to work in Chapel, and I think in
this case the curly brackets make the intention clearer:
var A: [{1, 2, 4}] real;
That said, my patch does not impact this behavior -- it's more a question for
future five-minute fixes (TM) because it would be similarly straightforward.
So, the two questions are:
1) Should we interpret {D} as an anonymous associative domain of domains
rather than simply a reference to domain 'D'?
2) Should we require curly brackets for anonymous domains in array
types that are not rectangular / defined by a list of ranges?
And if you're not interested in reviewing the patch, you can stop there.
-----
The heart of the patch is a simple change: We have traditionally supported a
routine, chpl__buildDomainExpr() which creates a domain from whatever is
passed it. One of the overloads took a domain and simply returned it which
is what caused '{D}' to evaluate to 'D'. So the idea behind this change is
to remove that overload.
A little more work is required, however, because the same call is used to
build the domain for an array type like '[D] real' and in this case, we want
to permit a domain like 'D' to be used. So what I did was to split
this call into two calls:
chpl__buildDomainExpr() : does what it has, but without the domain
overload now
chpl_ensureDomainExpr() : used in the array type signature context
to make sure that the expression is a
domain. In this context, if the thing
is a domain, we simply return it. In
all other cases, we forward the args to
chpl__buildDomainExpr()
From there, the only other change was to switch calls to
chpl__buildDomainExpr() in the array type signature case (which was most of
them) over to chpl__ensureDomainExpr(), and to rewrite any tests that were
relying on '{D}' evaluating to 'D'.
I also added a test to demonstrate the "anonymous associative domain in an
array declaration" issue for consideration.
Note that to only permit rectangular domains to have their curly brackets
dropped in the array declaration context, we'd simply need to restrict the
type signatures permitted for chpl_ensureDomainExpr(), I believe. And we
could generate an error overload for the other cases.
Thanks,
-Brad
Index: test/release/examples/benchmarks/ssca2/SSCA2_Modules/SSCA2_kernels.chpl
===================================================================
--- test/release/examples/benchmarks/ssca2/SSCA2_Modules/SSCA2_kernels.chpl (revision 22577)
+++ test/release/examples/benchmarks/ssca2/SSCA2_Modules/SSCA2_kernels.chpl (working copy)
@@ -169,7 +169,7 @@
config const defaultNumTPVs = 16;
config var numTPVs = min(defaultNumTPVs, numLocales);
// Would be nice to use PriavteDist, but aliasing is not supported (yet)
- const PrivateSpace = {LocaleSpace} dmapped Block(boundingBox={LocaleSpace});
+ const PrivateSpace = LocaleSpace dmapped Block(boundingBox=LocaleSpace);
// ==================================================================
// KERNEL 4
@@ -244,8 +244,8 @@
TPVLocales[t] = Locales[((t-1)/numTPVs)/numLocales];
}
}
- const TPVLocaleSpace = {TPVSpace} dmapped Block(boundingBox={TPVSpace},
- targetLocales=TPVLocales);
+ const TPVLocaleSpace = TPVSpace dmapped Block(boundingBox=TPVSpace,
+ targetLocales=TPVLocales);
// There will be numTPVs copies of the temps, thus throttling the
// number of starting vertices being considered simultaneously.
Index: test/distributions/diten/Dimensional.chpl
===================================================================
--- test/distributions/diten/Dimensional.chpl (revision 22577)
+++ test/distributions/diten/Dimensional.chpl (working copy)
@@ -168,7 +168,7 @@
var localeDom: domain(nDims) = {0..#nLocRows, 0..#nLocCols};
var locales: [localeDom] locale;
- for (i,j) in {localeDom} do
+ for (i,j) in localeDom do
locales(i,j) = Locales((i*nLocCols + j)%numLocales);
var dims: nDims*DimensionDistributor = (new Cyclic(1), new Cyclic(2));
Index: test/studies/ssca2/rachels/SSCA2_kernels.chpl
===================================================================
--- test/studies/ssca2/rachels/SSCA2_kernels.chpl (revision 22577)
+++ test/studies/ssca2/rachels/SSCA2_kernels.chpl (working copy)
@@ -171,7 +171,7 @@
config const defaultNumTPVs = 16;
config var numTPVs = min(defaultNumTPVs, numLocales);
// Would be nice to use PriavteDist, but aliasing is not supported (yet)
- const PrivateSpace = {LocaleSpace} dmapped Block(boundingBox={LocaleSpace});
+ const PrivateSpace = LocaleSpace dmapped Block(boundingBox=LocaleSpace);
// ==================================================================
// KERNEL 4
@@ -240,8 +240,8 @@
TPVLocales[t] = Locales[_computeChunkStartEnd(numLocales,
numTPVs, t+1)[1]-1];
}
- const TPVLocaleSpace = {TPVSpace} dmapped Block(boundingBox={TPVSpace},
- targetLocales=TPVLocales);
+ const TPVLocaleSpace = TPVSpace dmapped Block(boundingBox=TPVSpace,
+ targetLocales=TPVLocales);
// There will be numTPVs copies of the temps, thus throttling the
// number of starting vertices being considered simultaneously.
Index: test/studies/parsec/blackscholes_promote.chpl
===================================================================
--- test/studies/parsec/blackscholes_promote.chpl (revision 22577)
+++ test/studies/parsec/blackscholes_promote.chpl (working copy)
@@ -128,7 +128,7 @@
data.r, data.v, data.t, otype);
}
if ERR_CHK then
- numError = + reduce [i in {Dom}] errChk(i, data(i).DGrefval, prices(i));
+ numError = + reduce [i in Dom] errChk(i, data(i).DGrefval, prices(i));
}
proc main() {
Index: test/studies/hpcc/HPL/stonea/serial/hplExample3.chpl
===================================================================
--- test/studies/hpcc/HPL/stonea/serial/hplExample3.chpl (revision 22577)
+++ test/studies/hpcc/HPL/stonea/serial/hplExample3.chpl (working copy)
@@ -98,8 +98,8 @@
(ABlkDmn.dim(2))((blkCol-1)*blkSize+1..#blkSize)};
const bBlkD = {(ABlkDmn.dim(1))((blkRow-1)*blkSize+1..#blkSize),
(ADmn.dim(2))((blkCol-1)*blkSize+1..#blkSize)};
- const cBlkD = {ADmn[(blkRow-1)*blkSize+1..#aBlkD.dim(1).length,
- (blkCol-1)*blkSize+1..#bBlkD.dim(2).length]};
+ const cBlkD = ADmn[(blkRow-1)*blkSize+1..#aBlkD.dim(1).length,
+ (blkCol-1)*blkSize+1..#bBlkD.dim(2).length];
local {
dgemm(
@@ -235,11 +235,11 @@
const trailingCols = blk+crntBlkSize..ACols.high;
const unfactoredCols = blk..ACols.high;
- var tl = {A.domain[blockRange, blockRange]};
- var tr = {A.domain[blockRange, trailingCols]};
- var bl = {A.domain[trailingRows, blockRange]};
- var br = {A.domain[trailingRows, trailingCols]};
- var l = {A.domain[unfactoredRows, blockRange]};
+ var tl = A.domain[blockRange, blockRange];
+ var tr = A.domain[blockRange, trailingCols];
+ var bl = A.domain[trailingRows, blockRange];
+ var br = A.domain[trailingRows, trailingCols];
+ var l = A.domain[unfactoredRows, blockRange];
// Now that we've sliced and diced A properly do the blocked-LU
// computation:
Index: test/arrays/bradc/anonAssoc.chpl
===================================================================
--- test/arrays/bradc/anonAssoc.chpl (revision 0)
+++ test/arrays/bradc/anonAssoc.chpl (revision 0)
@@ -0,0 +1,11 @@
+var x: [1, 2, 4] string;
+
+x[1] = "one";
+x[2] = "two";
+x[4] = "four";
+
+writeln("x[1] is ", x[1]);
+writeln("x[4] is ", x[4]);
+writeln("x[2] is ", x[2]);
+
+x[3] = "three";
Index: test/arrays/bradc/anonAssoc.good
===================================================================
--- test/arrays/bradc/anonAssoc.good (revision 0)
+++ test/arrays/bradc/anonAssoc.good (revision 0)
@@ -0,0 +1,4 @@
+x[1] is one
+x[4] is four
+x[2] is two
+anonAssoc.chpl:11: error: halt reached - array index out of bounds: 3
Index: test/parallel/taskPar/rachels/Barrier.chpl
===================================================================
--- test/parallel/taskPar/rachels/Barrier.chpl (revision 22577)
+++ test/parallel/taskPar/rachels/Barrier.chpl (working copy)
@@ -15,7 +15,7 @@
use BlockDist;
// use this instead of a PrivateDist for performance reasons (PrivateDists are expensive!)
-const PrivateSpace = {LocaleSpace} dmapped Block(boundingBox={LocaleSpace});
+const PrivateSpace = LocaleSpace dmapped Block(boundingBox=LocaleSpace);
record Barrier {
param reusable = true;
Index: modules/internal/ChapelArray.chpl
===================================================================
--- modules/internal/ChapelArray.chpl (revision 22577)
+++ modules/internal/ChapelArray.chpl (working copy)
@@ -349,9 +349,6 @@
//
// Support for domain expressions, e.g., {1..3, 1..3}
//
- proc chpl__buildDomainExpr(x: domain)
- return x;
-
proc chpl__buildDomainExpr(ranges: range(?) ...?rank) {
for param i in 2..rank do
if ranges(1).idxType != ranges(i).idxType then
@@ -383,6 +380,17 @@
return D;
}
+
+ //
+ // Support for domain expressions within array types, e.g. [1..n], [D]
+ //
+ proc chpl__ensureDomainExpr(x: domain) {
+ return x;
+ }
+
+ proc chpl__ensureDomainExpr(x...) {
+ return chpl__buildDomainExpr((...x));
+ }
//
// Support for distributed domain expression e.g. {1..3, 1..3} dmapped Dist()
Index: compiler/AST/build.cpp
===================================================================
--- compiler/AST/build.cpp (revision 22577)
+++ compiler/AST/build.cpp (working copy)
@@ -171,16 +171,16 @@
if (indexCall->numActuals() != 1)
USR_FATAL(iterator, "invalid index expression");
return new CallExpr("chpl__buildArrayRuntimeType",
- new CallExpr("chpl__buildDomainExpr", iterator),
+ new CallExpr("chpl__ensureDomainExpr", iterator),
eltType, indexCall->get(1)->remove(),
- new CallExpr("chpl__buildDomainExpr", iterator->copy()));
+ new CallExpr("chpl__ensureDomainExpr", iterator->copy()));
} else {
CallExpr* call = toCallExpr(iterator);
if (call->numActuals() == 1 && isDefExpr(call->get(1))) {
return new CallExpr("chpl__buildArrayRuntimeType", call->get(1)->remove(), eltType);
} else
return new CallExpr("chpl__buildArrayRuntimeType",
- new CallExpr("chpl__buildDomainExpr", iterator), eltType);
+ new CallExpr("chpl__ensureDomainExpr", iterator), eltType);
}
}
@@ -710,7 +710,7 @@
// counting domains within runtime array types
thenStmt->insertAtTail(new CallExpr(PRIM_MOVE, domain,
new CallExpr("chpl__autoCopy",
- new CallExpr("chpl__buildDomainExpr",
+ new CallExpr("chpl__ensureDomainExpr",
iteratorExpr->copy()))));
if (hasSpecifiedIndices) {
// we want to swap something like the below commented-out
Index: compiler/AST/expr.cpp
===================================================================
--- compiler/AST/expr.cpp (revision 22577)
+++ compiler/AST/expr.cpp (working copy)
@@ -3370,7 +3370,7 @@
argList.last()->prettyPrint(o);
unusual = true;
} else if (strcmp(expr->unresolved,
- "chpl__buildDomainExpr") == 0) {
+ "chpl__ensureDomainExpr") == 0) {
unusual = true;
for_alist(expr, argList) {
if (expr != argList.first()) {
Index: compiler/parser/chapel.ypp
===================================================================
--- compiler/parser/chapel.ypp (revision 22577)
+++ compiler/parser/chapel.ypp (working copy)
@@ -340,7 +340,7 @@
| TLSBR expr_ls TRSBR stmt
{
if ($2->argList.length > 1)
- $$ = buildForallLoopStmt(NULL, new CallExpr("chpl__buildDomainExpr", $2), new BlockStmt($4));
+ $$ = buildForallLoopStmt(NULL, new CallExpr("chpl__ensureDomainExpr", $2), new BlockStmt($4));
else
$$ = buildForallLoopStmt(NULL, $2->get(1)->remove(), new BlockStmt($4));
}
@@ -806,7 +806,7 @@
opt_reindex_expr:
{ $$ = NULL; }
| TCOLON TLSBR expr_ls TRSBR
- { $$ = new CallExpr("chpl__buildDomainExpr", $3); }
+ { $$ = new CallExpr("chpl__ensureDomainExpr", $3); }
;
opt_type:
@@ -835,19 +835,19 @@
array_type:
TLSBR expr_ls TRSBR type_level_expr
{ $$ = new CallExpr("chpl__buildArrayRuntimeType",
- new CallExpr("chpl__buildDomainExpr", $2), $4);
+ new CallExpr("chpl__ensureDomainExpr", $2), $4);
}
| TLSBR expr_ls TRSBR array_type
{ $$ = new CallExpr("chpl__buildArrayRuntimeType",
- new CallExpr("chpl__buildDomainExpr", $2), $4);
+ new CallExpr("chpl__ensureDomainExpr", $2), $4);
}
| TLSBR expr_ls TIN expr TRSBR type_level_expr
{
if ($2->argList.length != 1)
USR_FATAL($4, "invalid index expression");
$$ = new CallExpr("chpl__buildArrayRuntimeType",
- new CallExpr("chpl__buildDomainExpr", $4), $6, $2->get(1)->remove(),
- new CallExpr("chpl__buildDomainExpr", $4->copy()));
+ new CallExpr("chpl__ensureDomainExpr", $4), $6, $2->get(1)->remove(),
+ new CallExpr("chpl__ensureDomainExpr", $4->copy()));
}
| error {printf("bad array type specification"); clean_exit(1); }
;
@@ -984,7 +984,7 @@
| TLSBR expr_ls TRSBR expr %prec TFOR
{
if ($2->argList.length > 1)
- $$ = buildForallLoopExpr(NULL, new CallExpr("chpl__buildDomainExpr", $2), $4, NULL, true);
+ $$ = buildForallLoopExpr(NULL, new CallExpr("chpl__ensureDomainExpr", $2), $4, NULL, true);
else
$$ = buildForallLoopExpr(NULL, $2->get(1)->remove(), $4, NULL, true);
}
Index: compiler/passes/normalize.cpp
===================================================================
--- compiler/passes/normalize.cpp (revision 22577)
+++ compiler/passes/normalize.cpp (working copy)
@@ -867,12 +867,6 @@
CallExpr* call = toCallExpr(arg->typeExpr->body.tail);
// Not sure why we select the tail here....
- //if (call && call->isNamed("chpl__buildDomainExpr")) {
- //CallExpr* arrayTypeCall = new CallExpr("chpl__buildArrayRuntimeType");
- //call->insertBefore(arrayTypeCall);
- //arrayTypeCall->insertAtTail(call->remove());
- //call = arrayTypeCall;
- //}
if (call && call->isNamed("chpl__buildArrayRuntimeType")) {
// We are building an array type.
bool noDomain = (isSymExpr(call->get(1))) ? toSymExpr(call->get(1))->var == gNil : false;
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
