From 98b98ccd007ed71ab5c1def239d83cb1c8dd1f5a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 03:58:10 +0000 Subject: [PATCH 1/3] Make OpenMP no_create a documented no-op instead of aborting fypp OMP_NOCREATE_STR called #:stop whenever no_create was set. GPU_PARALLEL, GPU_PARALLEL_LOOP and GPU_DATA expand both the ACC and OMP directive strings at fypp time and let the compile-time #if select one, so the abort fired on OpenACC-only builds too -- making a documented parameter unusable on every backend. OpenMP has no no_create equivalent, and OMP_DEFAULT_STR already emits a present-by-default mapping, so emitting nothing is the closest safe behavior. Document the clause as OpenACC-only, matching the existing GPU_CACHE note. Generated Fortran is byte-identical across all 109 .fpp files in the three targets: every call site passes no_create=None, which already took the empty-string path. Closes #1687 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016xc51kikzrKdHDfXf8VV5p --- docs/documentation/gpuParallelization.md | 6 +++--- src/common/include/omp_macros.fpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/documentation/gpuParallelization.md b/docs/documentation/gpuParallelization.md index 3023ac7880..a83eb47247 100644 --- a/docs/documentation/gpuParallelization.md +++ b/docs/documentation/gpuParallelization.md @@ -73,7 +73,7 @@ This wraps the lines in `code` with parallelization calls to openACC or openMP, | `copyinReadOnly` | string list | None | Allocates and copies readonly data to GPU and then deallocated on exit | | `copyout` | string list | None | Allocates data on GPU on entrance and then deallocates and copies to CPU on exit | | `create` | string list | None | Allocates data on GPU on entrance and then deallocates on exit | -| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory | +| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory (OpenACC only) | | `present` | string list | None | Data that must be present in GPU memory. Increment counter on entrance, decrement on exit | | `deviceptr` | string list | None | Pointer variables that are already allocated on GPU memory | | `attach` | string list | None | Attaches device pointer to device targets on entrance, then detach on exit | @@ -184,7 +184,7 @@ Uses FYPP call directive using `#:call` | `copyinReadOnly` | string list | None | Allocates and copies readonly data to GPU and then deallocated on exit | | `copyout` | string list | None | Allocates data on GPU on entrance and then deallocates and copies to CPU on exit | | `create` | string list | None | Allocates data on GPU on entrance and then deallocates on exit | -| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory | +| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory (OpenACC only) | | `present` | string list | None | Data that must be present in GPU memory. Increment counter on entrance, decrement on exit | | `deviceptr` | string list | None | Pointer variables that are already allocated on GPU memory | | `attach` | string list | None | Attaches device pointer to device targets on entrance, then detach on exit | @@ -247,7 +247,7 @@ Uses FYPP call directive using `#:call` | `copyinReadOnly` | string list | None | Allocates and copies a readonly variable to GPU and then deallocated on exit | | `copyout` | string list | None | Allocates data on GPU on entrance and then deallocates and copies to CPU on exit | | `create` | string list | None | Allocates data on GPU on entrance and then deallocates on exit | -| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory | +| `no_create` | string list | None | Use data in CPU memory unless data is already in GPU memory (OpenACC only) | | `present` | string list | None | Data that must be present in GPU memory. Increment counter on entrance, decrement on exit | | `deviceptr` | string list | None | Pointer variables that are already allocated on GPU memory | | `attach` | string list | None | Attaches device pointer to device targets on entrance, then detach on exit | diff --git a/src/common/include/omp_macros.fpp b/src/common/include/omp_macros.fpp index ff3d97fe5c..b03bd6583e 100644 --- a/src/common/include/omp_macros.fpp +++ b/src/common/include/omp_macros.fpp @@ -67,9 +67,9 @@ #:enddef #:def OMP_NOCREATE_STR(no_create) - #:if no_create is not None - #:stop 'no_create is not supported yet' - #:endif + #! OpenMP has no no_create equivalent; OMP_DEFAULT_STR already maps + #! present-by-default, so emit nothing. Aborting here would also break + #! OpenACC builds, which expand both backends before #if selects one. #:set no_create_val = '' $:no_create_val #:enddef From 89c0f71ac8d7d9d64e8ba3ef50d86d24e975ac08 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Tue, 18 Aug 2026 11:22:48 -0400 Subject: [PATCH 2/3] fix(omp): do not emit a clause-less `!$omp target data`, and correct the no_create comment Removing the #:stop makes no_create usable, but it leaves OMP_DATA able to produce a directive with no clauses at all. OMP_NOCREATE_STR now returns '', and OMP_DATA's `default` is None by default so OMP_DEFAULT_STR contributes nothing either, so GPU_DATA(no_create='[a]') expanded to: !$omp target data a = 1 !$omp end target data OpenMP requires `target data` to carry at least one map, use_device_ptr or use_device_addr clause, so an OpenMP build got a Fortran compile error at a generated line -- replacing the old, clear fypp message with a worse one. Verified by expansion on NVHPC, Cray and LLVMFlang. Emit the body alone when there are no clauses, matching what GPU_DATA's own #else branch does when neither backend is enabled. A #:stop cannot be used here: GPU_DATA expands both backends before #if selects one, which is the bug this no-op exists to fix in the first place. Also correct the justifying comment. "OMP_DEFAULT_STR already maps present-by-default" holds only for CCE, and only when the caller passes default='present'; in OMP_DATA `default` is None, and NVHPC/PGI and the fallback emit defaultmap(tofrom:...), which copies rather than reuses. The no-op is fine and now documented, but the comment promised semantics the code does not deliver. Generated code is byte-identical to the previous commit for every existing call site (3 compilers x 7 source files, 29785 lines). --- src/common/include/omp_macros.fpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/common/include/omp_macros.fpp b/src/common/include/omp_macros.fpp index b03bd6583e..f09df75233 100644 --- a/src/common/include/omp_macros.fpp +++ b/src/common/include/omp_macros.fpp @@ -67,9 +67,13 @@ #:enddef #:def OMP_NOCREATE_STR(no_create) - #! OpenMP has no no_create equivalent; OMP_DEFAULT_STR already maps - #! present-by-default, so emit nothing. Aborting here would also break - #! OpenACC builds, which expand both backends before #if selects one. + #! OpenMP has no no_create equivalent, so this is a documented no-op: the variable is + #! left to whatever mapping the enclosing region already applies to it. Note that is + #! NOT equivalent to no_create on most targets -- OMP_DEFAULT_STR emits nothing unless + #! the caller passes default='present', and even then only CCE maps present; NVHPC/PGI + #! and the fallback emit defaultmap(tofrom:...), which copies rather than reuses. + #! Do NOT #:stop here: GPU_DATA expands both backends before #if selects one, so + #! aborting would break OpenACC builds, where no_create is supported natively. #:set no_create_val = '' $:no_create_val #:enddef @@ -275,11 +279,23 @@ & no_create_val.strip('\n') + present_val.strip('\n') + & & deviceptr_val.strip('\n') + attach_val.strip('\n') + & & default_val.strip('\n') + #! An OpenMP `target data` region must carry at least one map, use_device_ptr or + #! use_device_addr clause. no_create contributes none (it is a no-op here), and + #! `default` is None by default, so GPU_DATA(no_create=...) with no other clause left + #! clause_val empty and emitted a bare `!$omp target data` that no compiler accepts. + #! Emit the body alone in that case -- a data region with nothing to map has nothing + #! to do, which is what GPU_DATA's own #else branch already does when neither backend + #! is enabled. A #:stop is not an option here, for the reason in OMP_NOCREATE_STR. + #:set has_clauses = clause_val.strip() != '' or extraOmpArgs_val.strip() != '' #:set omp_directive = '!$omp target data ' + clause_val + extraOmpArgs_val.strip('\n') #:set end_omp_directive = '!$omp end target data' - $:omp_directive + #:if has_clauses + $:omp_directive + #:endif $:code - $:end_omp_directive + #:if has_clauses + $:end_omp_directive + #:endif #:enddef #:def OMP_ENTER_DATA(copyin=None, copyinReadOnly=None, create=None, attach=None, extraOmpArgs=None) From d384a0d10c35d39bffc12965b36d3bac5e75506e Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Tue, 18 Aug 2026 11:51:02 -0400 Subject: [PATCH 3/3] Drop the ellipses from the new fypp comments; lint_source flags them as placeholder text --- src/common/include/omp_macros.fpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/include/omp_macros.fpp b/src/common/include/omp_macros.fpp index f09df75233..fe2dc78e17 100644 --- a/src/common/include/omp_macros.fpp +++ b/src/common/include/omp_macros.fpp @@ -71,7 +71,7 @@ #! left to whatever mapping the enclosing region already applies to it. Note that is #! NOT equivalent to no_create on most targets -- OMP_DEFAULT_STR emits nothing unless #! the caller passes default='present', and even then only CCE maps present; NVHPC/PGI - #! and the fallback emit defaultmap(tofrom:...), which copies rather than reuses. + #! and the fallback emit a defaultmap(tofrom:scalar) clause, which copies rather than reuses. #! Do NOT #:stop here: GPU_DATA expands both backends before #if selects one, so #! aborting would break OpenACC builds, where no_create is supported natively. #:set no_create_val = '' @@ -281,7 +281,7 @@ & default_val.strip('\n') #! An OpenMP `target data` region must carry at least one map, use_device_ptr or #! use_device_addr clause. no_create contributes none (it is a no-op here), and - #! `default` is None by default, so GPU_DATA(no_create=...) with no other clause left + #! `default` is None by default, so GPU_DATA(no_create=x) with no other clause left #! clause_val empty and emitted a bare `!$omp target data` that no compiler accepts. #! Emit the body alone in that case -- a data region with nothing to map has nothing #! to do, which is what GPU_DATA's own #else branch already does when neither backend