Cactus: New cGH fields tile_min, tile_max

[Erik Schnetter]

CarpetX’s OpenMP parallelization scheme tiles components, and calls scheduled functions once per tile. The new cGH fields tile_min and tile_max describe the local tile.

See https://bitbucket.org/cactuscode/cactus/pull-requests/104/cactus-new-cgh-fields-tile_min-tile_max

[Roland Haas]

Currently this seems not ready to be applied.

[Erik Schnetter]

I corrected the code.

[Roland Haas]

I am afraid this may require more work. Sorry for the piecemeal review (though some of the comments were present before):

• this changes the omp singleclause by removing the copyprivate(cctki3_bndsize, cctki3_is_physbnd) which will leave cctki3_bndsize uninitialized in the threads that do not execute the single clause it seems since those arrays are automatic, private variables. They could be made static though it seems, which hopefully won’t make them threadprivate.
• the prototype of GetBoundarySizesAndTypes should not be duplicated. Thorns using CCTK_LOOPX_INT must themselves USE or REQUIRE the aliased function at which point a prototype will be available when the macro is expanded
• the

‌

#else
#define CCTK_LOOP_TILE_MIN(cctkGH, d) 0
#define CCTK_LOOP_TILE_MAX(cctkGH, d) (cctkGH)->cctk_lsh[d]

‌

• does not seem to be required since both cctk_Loop.h and cctk_GroupsOnGH.h are in the same repository so will be kept in sync by git
• no support for Fortran is provided (F90 is still in use, see http://lists.einsteintoolkit.org/pipermail/users/2020-February/007298.html)

‌

[Roland Haas]

Looking at the omp single and in particular its implied omp barrier at the end, I suspect the current code may also run the risk of creating a deadlock when the scheduled functions are called in a multi-threaded way in CarpetX. Namely since the barier assumes that all threads will encounter it, there would be a deadlock if the the number of tiles processed by each thread is not identical to that of any of the other threads. A possible workaround seems to be to remove the barrier by using nowait which however makes copyprivate impossible requiring that the cctki_bndsize arrays be static so that they are shared among the threads.

[Erik Schnetter]

Code is even moar correct now.

• Removed prototype for GetBoundarySizesAndTypes

• Removed parallelization for GetBoundarySizesAndTypes

• Removed CCTK_LOOP_TILE_* macros
• Added Fortran support

‌

[Roland Haas]

I like the new pull request with more correctness than ever.

Please apply.

[Roland Haas]

Applied as git hash aa1dd19f "Merged in eschnett/tiles (pull request #104)" of cactus

[Roland Haas]

Having approved and merged I now (naturally) notice this in cGH:

  /* unused */
  int *cctk_to;
  int *cctk_from;

which could have been used instead of tile_min and tile_max.

[Erik Schnetter]

I knew about these, and I looked at using them instead of defining new fields. I decided it was not possible. Unfortunately I don’t recall the reason why.