[llvm-branch-commits] [Clang][OpenMP] Restrict 'saved' firstprivate modifier to admissible directives (PR #200407)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri May 29 06:56:00 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Julian Brown (jtb20)
<details>
<summary>Changes</summary>
Per OpenMP 6.0 [7.2], the 'saved' modifier on a data-environment
attribute clause has effect only when that clause appears on a
replayable construct. Per [14.3] and [14.6], the only directives
that admit the 'replayable' clause are: 'target',
'target enter data', 'target exit data', 'target update', 'task',
'taskloop' and 'taskwait'. Of those seven, only 'target', 'task'
and 'taskloop' also admit a 'firstprivate' clause; the remaining
four cannot syntactically carry 'firstprivate' and need no extra
sema work. A directive outside the admitted set can never make
'firstprivate(saved: ...)' meaningful.
We do not try to flag every dead-modifier case statically:
'saved' on a directive in the admitted set but without
'replayable' (and without lexical nesting in a taskgraph) is
well-formed per [7.2] and silently has no effect. Detecting that
would require inspecting the full clause list, and is invalidated
as soon as the user adds the clause or moves the construct inside
a taskgraph.
What we can and do diagnose is the case where the host directive
itself cannot ever be a replayable construct: 'saved' on a
'parallel', 'for', 'sections', 'single', 'distribute', 'teams',
etc. firstprivate clause has no path to replay semantics at all.
Reject that in ActOnOpenMPFirstprivateClause when CurDir is
neither an OpenMP tasking directive
(isOpenMPTaskingDirective, which covers OMPD_task and every
OMPD_*taskloop* variant) nor an OpenMP target execution directive
(isOpenMPTargetExecutionDirective, which covers OMPD_target and
every combined directive that has OMPD_target as a leaf
construct), with a new dedicated diagnostic
err_omp_firstprivate_saved_wrong_directive that points at the
modifier location and lists the three admissible directives.
The existing ast-print test is extended to cover these cases, and other
new tests have been added.
Assisted-By: Claude Opus 4.7
---
Full diff: https://github.com/llvm/llvm-project/pull/200407.diff
4 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
- (modified) clang/lib/Sema/SemaOpenMP.cpp (+14)
- (modified) clang/test/OpenMP/taskgraph_firstprivate_saved_ast_print.cpp (+45-7)
- (added) clang/test/OpenMP/taskgraph_firstprivate_saved_messages.cpp (+115)
``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fd442a7b93492..ce6e7f2182d13 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12646,6 +12646,9 @@ def err_omp_one_defaultmap_each_category: Error<
def err_omp_lastprivate_conditional_non_scalar : Error<
"expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier"
>;
+def err_omp_firstprivate_saved_wrong_directive : Error<
+ "'saved' modifier on 'firstprivate' clause is only allowed on a 'task', "
+ "'taskloop', or 'target' construct">;
def err_omp_flush_order_clause_and_list : Error<
"'flush' directive with memory order clause '%0' cannot have the list">;
def note_omp_flush_order_clause_here : Note<
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 51a5c3522a5db..bf888892f8aab 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -19648,6 +19648,20 @@ OMPClause *SemaOpenMP::ActOnOpenMPFirstprivateClause(
return nullptr;
}
+ // OpenMP 6.0 [7.2]: the 'saved' modifier has effect only on a clause
+ // that appears on a replayable construct. Per [14.6] the directives
+ // that admit a 'replayable' clause and that also admit 'firstprivate'
+ // are 'task', 'taskloop', and 'target' (the other replayable-eligible
+ // directives -- 'target_enter_data' etc. -- do not admit
+ // 'firstprivate' at all). Reject 'saved' on any other directive, as
+ // it can never become a replayable construct.
+ OpenMPDirectiveKind CurDir = DSAStack->getCurrentDirective();
+ if (FPKind == OMPC_FIRSTPRIVATE_saved && !isOpenMPTaskingDirective(CurDir) &&
+ !isOpenMPTargetExecutionDirective(CurDir)) {
+ Diag(FPKindLoc, diag::err_omp_firstprivate_saved_wrong_directive);
+ return nullptr;
+ }
+
SmallVector<Expr *, 8> Vars;
SmallVector<Expr *, 8> PrivateCopies;
SmallVector<Expr *, 8> Inits;
diff --git a/clang/test/OpenMP/taskgraph_firstprivate_saved_ast_print.cpp b/clang/test/OpenMP/taskgraph_firstprivate_saved_ast_print.cpp
index faf6b7a2936ae..df358df14c82f 100644
--- a/clang/test/OpenMP/taskgraph_firstprivate_saved_ast_print.cpp
+++ b/clang/test/OpenMP/taskgraph_firstprivate_saved_ast_print.cpp
@@ -21,21 +21,59 @@ void firstprivate_saved() {
#pragma omp task firstprivate(a)
{ (void)a; }
+ // 'saved' on an omp task lexically inside a taskgraph.
+ // CHECK: #pragma omp taskgraph
// CHECK: #pragma omp task firstprivate(saved: a)
- #pragma omp task firstprivate(saved: a)
- { (void)a; }
+ #pragma omp taskgraph
+ {
+ #pragma omp task firstprivate(saved: a)
+ { (void)a; }
+ }
+ // Multiple variables.
+ // CHECK: #pragma omp taskgraph
// CHECK: #pragma omp task firstprivate(saved: a,b,c)
- #pragma omp task firstprivate(saved: a, b, c)
- { (void)a; (void)b; (void)c; }
+ #pragma omp taskgraph
+ {
+ #pragma omp task firstprivate(saved: a, b, c)
+ { (void)a; (void)b; (void)c; }
+ }
+ // Mixed with another clause.
+ // CHECK: #pragma omp taskgraph
// CHECK: #pragma omp task firstprivate(saved: a) shared(b)
- #pragma omp task firstprivate(saved: a) shared(b)
- { (void)a; (void)b; }
+ #pragma omp taskgraph
+ {
+ #pragma omp task firstprivate(saved: a) shared(b)
+ { (void)a; (void)b; }
+ }
+ // 'saved' on an omp taskloop lexically inside a taskgraph.
+ // CHECK: #pragma omp taskgraph
// CHECK: #pragma omp taskloop firstprivate(saved: a)
- #pragma omp taskloop firstprivate(saved: a)
+ #pragma omp taskgraph
+ {
+ #pragma omp taskloop firstprivate(saved: a)
+ for (int i = 0; i < 4; ++i) (void)a;
+ }
+
+ // 'saved' on a replayable omp task outside any taskgraph - also legal.
+ // CHECK: #pragma omp task replayable firstprivate(saved: a)
+ #pragma omp task replayable firstprivate(saved: a)
+ { (void)a; }
+
+ // 'saved' on a replayable omp taskloop outside any taskgraph - also legal.
+ // CHECK: #pragma omp taskloop replayable firstprivate(saved: a)
+ #pragma omp taskloop replayable firstprivate(saved: a)
for (int i = 0; i < 4; ++i) (void)a;
+
+ // 'saved' on a non-lexically-nested task (dynamic nesting via a call into
+ // a function from a taskgraph region is the runtime use case) - we accept
+ // any task/taskloop construct since the static check cannot prove dynamic
+ // nesting.
+ // CHECK: #pragma omp task firstprivate(saved: a)
+ #pragma omp task firstprivate(saved: a)
+ { (void)a; }
}
#endif
diff --git a/clang/test/OpenMP/taskgraph_firstprivate_saved_messages.cpp b/clang/test/OpenMP/taskgraph_firstprivate_saved_messages.cpp
new file mode 100644
index 0000000000000..5197d16cd0421
--- /dev/null
+++ b/clang/test/OpenMP/taskgraph_firstprivate_saved_messages.cpp
@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=60 -fsyntax-only -verify %s
+
+// Tests the OpenMP 6.0 'saved' modifier on the 'firstprivate' clause. The
+// modifier is meaningful only on constructs that create tasks or taskloops
+// (the units of work that can participate in taskgraph replay). Every other
+// directive that admits a 'firstprivate' clause must reject it.
+
+void unknown_modifier() {
+ int a = 0;
+ // The diagnostic comes from the generic "expected <list> in OpenMP clause"
+ // path and enumerates the legal modifier names ('saved' in OpenMP 6.0).
+ #pragma omp task firstprivate(bogus: a) // expected-error {{expected 'saved' in OpenMP clause 'firstprivate'}}
+ { (void)a; }
+}
+
+void rejected_on_non_tasking_constructs() {
+ int a = 0;
+ int b[8];
+
+ // parallel
+ #pragma omp parallel firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ { (void)a; }
+
+ // for (worksharing)
+ #pragma omp parallel
+ {
+ #pragma omp for firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ for (int i = 0; i < 4; ++i) (void)a;
+ }
+
+ // sections
+ #pragma omp parallel
+ {
+ #pragma omp sections firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ {
+ (void)a;
+ }
+ }
+
+ // single
+ #pragma omp parallel
+ {
+ #pragma omp single firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ { (void)a; }
+ }
+
+ // teams (inside target -- the inner directive is a standalone 'teams',
+ // not a 'target teams' combined directive, so it is not a target
+ // execution directive in its own right).
+ #pragma omp target
+ #pragma omp teams firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ { (void)a; }
+
+ // distribute (inside teams) -- standalone 'distribute' is not a target
+ // execution directive either.
+ #pragma omp target teams
+ #pragma omp distribute firstprivate(saved: a) // expected-error {{'saved' modifier on 'firstprivate' clause is only allowed on a 'task', 'taskloop', or 'target' construct}}
+ for (int i = 0; i < 4; ++i) (void)a;
+}
+
+void accepted_on_task_taskloop_and_target() {
+ int a = 0;
+
+ // Bare task (no enclosing taskgraph, no replayable clause): accepted.
+ // Per OpenMP 6.0 [7.2] the 'saved' modifier silently has no effect on a
+ // non-replayable construct; we only enforce the directive-kind check
+ // statically. In this implementation a bare task / taskloop is never
+ // recorded into a taskgraph -- recording requires either lexical
+ // nesting inside a '#pragma omp taskgraph' or an explicit 'replayable'
+ // clause -- so the modifier is a well-defined no-op here.
+ #pragma omp task firstprivate(saved: a)
+ { (void)a; }
+
+ // Bare taskloop: accepted, same rationale.
+ #pragma omp taskloop firstprivate(saved: a)
+ for (int i = 0; i < 4; ++i) (void)a;
+
+ // Replayable task: explicitly opted-in for replay.
+ #pragma omp task replayable firstprivate(saved: a)
+ { (void)a; }
+
+ // Replayable taskloop.
+ #pragma omp taskloop replayable firstprivate(saved: a)
+ for (int i = 0; i < 4; ++i) (void)a;
+
+ // Task lexically nested inside a taskgraph.
+ #pragma omp taskgraph
+ {
+ #pragma omp task firstprivate(saved: a)
+ { (void)a; }
+ }
+
+ // Bare target: accepted on the same well-formed-but-no-effect grounds
+ // as a bare task. Per OpenMP 6.0 [14.6] the 'target' construct admits
+ // both 'firstprivate' and 'replayable', so 'saved' is meaningful as
+ // soon as a 'replayable' clause is added or the construct is nested
+ // inside a 'taskgraph' region.
+ #pragma omp target firstprivate(saved: a)
+ { (void)a; }
+
+ // Replayable target: explicitly opted-in for replay.
+ #pragma omp target replayable firstprivate(saved: a)
+ { (void)a; }
+
+ // Combined target construct (target + parallel): accepted because the
+ // composite directive is a target execution directive in its own
+ // right, so the captured snapshot at the target boundary belongs to a
+ // construct that may participate in taskgraph replay.
+ #pragma omp target parallel firstprivate(saved: a)
+ { (void)a; }
+
+ // Combined target teams distribute parallel for: same rationale.
+ #pragma omp target teams distribute parallel for firstprivate(saved: a)
+ for (int i = 0; i < 4; ++i) (void)a;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/200407
More information about the llvm-branch-commits
mailing list