GPU macros: OpenMP renders attach as map(always,tofrom:), which is not what OpenACC's attach means #1693

Activity

1. sbryngelson

   added a commit that references this issue on Jul 29, 2026

   cce: drop ptr-component-attach -- accidentally committed WIP, does no…

   ...

   9df3407
2. sbryngelson commented on Jul 29, 2026

   

   sbryngelson

   on Jul 29, 2026

   MemberAuthor

   More actions

   Evidentiary status: code-level finding, no demonstrated runtime failure

   Recording this so nobody assumes more evidence exists than actually does.

   What this issue rests on. Reading the macro layer: attach renders as
   map(always,tofrom:) under OpenMP, while the OpenACC path performs a genuine attach. Those are
   not equivalent — one copies data, the other establishes the device-side pointer relationship
   between a parent aggregate and its pointee. That discrepancy is real and stands on its own.

   What it does not rest on. There is no reproduced wrong answer, fault, or test failure
   attributable to it. I attempted to build a standalone reproducer and failed twice:

   1. First version did not compile — #ifdef in a lowercase .f90, which ftn will not
      preprocess without -eZ. The captured "results" were build errors.
   2. After fixing that, the OpenACC arm reported a failure to attach — which contradicts MFC
      passing its full 627-test suite on --gpu acc using exactly the ACC_SETUP_SFs pattern in
      question. A third round (removing a declare create / enter data double-map, the trap
      documented in our own notes) did not resolve the disagreement.

   When a microbenchmark disagrees with a green production suite, the microbenchmark is wrong. The
   reproducer has been deleted rather than kept, since one that contradicts observed behaviour is
   worse than none — someone would eventually act on it.

   Worth noting the OpenMP arm passed both variants tested, which is weak evidence that the
   current rendering may work in practice for this pattern.

   What would settle it. The OpenMP 5.x spec text on implicit pointer attachment — specifically
   whether mapping a pointer component while its parent is already present is required to attach.
   If it is, the fix is small and well-defined: emit the parent-then-pointee sequence under OpenMP,
   matching what ACC_SETUP_SFs already does for OpenACC —

   $:GPU_ENTER_DATA(copyin=('[' + arg + ']'))          ! parent first
   if (associated(${arg}$%sf)) then
       $:GPU_ENTER_DATA(copyin=('[' + arg + '%sf]'))   ! pointee second -> attaches
   end if

   — rather than map(always,tofrom:). If the spec leaves it unspecified, then MFC is relying on
   unportable behaviour on both models and the fix is larger.

   Priority read: worth fixing on correctness grounds, because substituting a data copy for
   pointer attachment is fragile even where it happens to work. Not urgent, and should not be
   described as a known-broken path.
3. sbryngelson commented on Aug 28, 2026

   

   sbryngelson

   on Aug 28, 2026

   MemberAuthor

   More actions

   Audit: confirmed as written, but the stated impact does not occur today — this is latent, not active.

   The always,tofrom rendering is real and is at omp_macros.fpp:135 (OMP_PARALLEL_LOOP), :169
   (OMP_PARALLEL) and :270 (OMP_DATA). But none of those three paths has a caller that passes
   attach. Every attach= use in src/ — 8 of them — goes through GPU_ENTER_DATA:

   src/simulation/m_igr.fpp:215
   src/simulation/m_rhs.fpp:183, 190, 197, 203, 281, 287, 289
   

   and OMP_ENTER_DATA (omp_macros.fpp:288) already renders it as

   #:set attach_val = OMP_MAP_STR('always,to', attach)
   

   which is the always,to the commented-out OMP_ATTACH_STR intended. So no current build performs
   the unconditional round-trip copy or the stale copy-back described above.

   That reprioritizes this: it is not a live correctness or performance bug, it is a trap for the first
   person who passes attach to GPU_PARALLEL_LOOP, GPU_PARALLEL or GPU_DATA, where they will
   silently get different semantics from the same argument on the ENTER_DATA path.

   Worth noting for whoever fixes it: OMP_ENTER_DATA's always,to is the shipped, tested behaviour on
   the AMD OpenMP path, so the safe fix is to bring the three unused sites into line with it (ideally by
   restoring OMP_ATTACH_STR and calling it from all four, so there is one definition), rather than
   changing the path that currently works.