Make it possible to disable OpenMP in the `cctk_Loop.h` loop macros

Create issue
Issue #2444 resolved
Erik Schnetter created an issue

The looping macros defined in cctk_Loop.h use OpenMP unconditionally. If a thorn is not using OpenMP, or is using OpenMP in a manner incompatible with cctk_Loop.h, then this can lead to segfaults. For example, cctk_Loop.h uses omp single (that prevents certain code from executing) or omp for (which parallelizes part of the loops). There should be a mechanism to disable this OpenMP parallelization.

Comments (10)

  1. Erik Schnetter reporter

    I forgot to say: For example, CarpetX uses OpenMP in a manner incompatible with cctk_Loop.h, leading to such segfaults.

  2. Roland Haas

    Somewhat ugly, but seems to be the only solution.

    Naming scheme (OMP or OPENMP) is consistent with the existing CCTK_DISABLE_OMP_COLLAPSE configure option.

    I would suggest to extend the ifndef to include also the case where _OPENMP is not defined (ie

    • fopenmp

    is not in effect):

    #  if(defined(_OPENMP) && !defined(CCTK_LOOP_DISABLE_OMP))
    #    define CCTK_PRAGMA_OMP(x) _Pragma(x)
    #  else
    #    define CCTK_PRAGMA_OMP(x)
    #  endif

    since this will avoid the “warning: ignoring #pragma omp for” warnings when compiling without

    • fopenmp

    . It does require, like all such checks for OPENMP, that

    • fopenmp

    is part of FPPFLAGS and CPPFLAGS (required already for MAKEDEPEND to work which uses CPP and FPP only) so runs slight risk of always disabling OpenMP in CCTK_LOOP if CPPFLAGS and FPPFLAGS are not set correctly (know-architectures does not set them, unfortunately).

    Other than that please apply.

  3. Roland Haas

    @eschnett said:

    Is that (accidentally switching off OpenMP) not quite dangerous?

    Also, not all OpenMP warnings will go away, since I left the omp critical in. These are used for error messages. (One should probably wrap the bodies of CCTK_ERROR and friends instead of asking the callers to do so, but that’s another point.) I’m inclined to leave out the defined _OPENMP.

  4. Roland Haas

    Uff, I had not realized it would leave OpenMP statements behind.

    Yes, please leave out the defined _OPENMP as a more comprehensive solution is needed.

    As for the danger of silently disabling OpenMP: yes, that was definitely a concern.

    Right now we kind of cheat since even if CPPFLAGS does not contain -fopenmp it really only affects the MAKEDEPEND call in the Makefile since C/C++ preprocessing is handled on-the-fly by the compiler wrappers to which we pass both CPPFLAGS and CFLAGS/ CXXFLAGS. If these two stages were actually separate (or if there was a wrapper that would take say --cppflags = ““ and --cflags = ““ then we’ woul be in trouble already now since LoopControl (likely, I have not checked right now) contains the same #ifdef _OPENMP constructs to wrap around omp_num_threads() calls.

    For Fortran code this is much more obvious since there FPP is indeed a separate invocation from the compiler.

    So: please apply (the original version).

  5. Roland Haas

    Actually…. since the define only affects the loops, would it be possible to name it CCTK_LOOP_PPRAGMA_OMP?

  6. Log in to comment