Cactus: New cGH fields tile_min, tile_max

Create issue
Issue #2446 resolved
Erik Schnetter created an issue

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

Comments (11)

  1. 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]
    

  2. 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.

  3. Erik Schnetter reporter

    Code is even moar correct now.

    • Removed prototype for GetBoundarySizesAndTypes

    • Removed parallelization for GetBoundarySizesAndTypes

    • Removed CCTK_LOOP_TILE_* macros
    • Added Fortran support

  4. 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.

  5. Erik Schnetter reporter

    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.

  6. Log in to comment