thomasrolinger / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
0 stars 1 forks source link

Task: Use custom parallel iterator instead of dummy forall #19

Closed thomasrolinger closed 2 years ago

thomasrolinger commented 2 years ago

Our 2-D array tests are not working correctly with the coforall-based inspector. It comes down to the fact that the dummy loop we start with is set up for a single i index in the for-loop. Our examples expect a tuple (i,j) as the index.

I think we can make this easier by creating a custom iterator and just replacing the original forall loop iterator with that: https://chapel-lang.org/docs/primers/parIters.html

This is a cleaner approach overall, not just for the 2D case. I can try to code up the iterator in Chapel to make sure it works. Then the question is how to replace the iterator within the AST.

We can probably make two versions of the iterator, one that takes in arrays and one that takes in domains. Not sure if that’ll be necessary but we’ll see as we start coding it up.

thomasrolinger commented 2 years ago

This has been addressed: https://github.com/thomasrolinger/chapel/commit/54fa458f028e0278f3281f4ea5f2c5841ef4b8c4

We created the parallel iterator in modules/internal/InspectorExecutor.chpl. We then modified our "transform forall to coforall" function in forallOptimizations.cpp to generate a call to our parallel iterator and then add it to the original forall. It was a simple transformation of:

Expr *origIterExpr = inspectorLoop->firstIteratedExpr();
CallExpr *inspectorIter = new CallExpr("inspectorIter");
origIterExpr->insertBefore(inspectorIter);
inspectorIter->insertAtTail(origIterExpr->remove());

Note that we are assuming that we don't have a zippered forall. For more robustness, we should eventually add checks to abort if we detect such a case. Furthermore, we still need to add a check that the array/domain that the forall iterates over supports the localSubdomain query. If it does not, we should fall back to the original inspector loop.

In regards to the 2-D scenario that we started with, what we fixed above was not the only problem we had. We were passing in an incorrect index type into our ReplicatedElems initializer, or rather, we thought what we were passing it was something that represented both the dimension and type of the index. However, for a 2-D array arr, arr.idxType will not be something like 2*int(64), but instead just int(64).

The fix this, we added a param-based conditional to the call to ReplicatedElems():

var replArray : if rank == 1 then ReplicatedElems(eltType, idxType, rank) 
                             else ReplicatedElems(eltType, rank*idxType, rank);

In that way, we will get what we expect for multi-dimensional arrays.