[flang-commits] [flang] [flang][OpenMP] Diagnose invalid capture statements in ATOMIC (PR #233330)
via flang-commits
flang-commits at lists.llvm.org
Wed Oct 21 00:09:55 PDT 2026
https://github.com/Ritanya-B-Bharadwaj updated https://github.com/llvm/llvm-project/pull/233330
>From 1aadf7f14cffb7e1c3806fe907f2ac7f09a5d099 Mon Sep 17 00:00:00 2001
From: Ritanya-B-Bharadwaj <ritanya.b.bharadwaj at gmail.com>
Date: Fri, 16 Oct 2026 12:03:44 +0530
Subject: [PATCH 1/3] [flang][OpenMP] Diagnose invalid capture statements in
ATOMIC
v and x being the same variable in ATOMIC READ/CAPTURE is now an error
instead of a warning, since lowering can't handle it. In ATOMIC COMPARE
CAPTURE, check that the capture statement reads x, and (from OpenMP 5.2)
that v and e are different when v gets the original value of x.
Fixes #232594, fixes #221013
---
flang/lib/Semantics/check-omp-atomic.cpp | 41 +++++++++++--
flang/lib/Semantics/check-omp-structure.h | 3 +-
.../OpenMP/atomic-capture-same-storage.f90 | 60 +++++++++++++++++++
.../OpenMP/atomic-compare-capture-invalid.f90 | 33 ++++++++++
4 files changed, 130 insertions(+), 7 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90
create mode 100644 flang/test/Semantics/OpenMP/atomic-compare-capture-invalid.f90
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index 7e8a3ffef8909..7f2234bf21a77 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -427,7 +427,6 @@ static bool IsMaybeAtomicWrite(const evaluate::Assignment &assign) {
// (ultimate) symbol vectors together with the source form distinguishes
// different variables and different constant subscripts (e.g. a(1) vs a(2))
// while treating host-associated and ultimate references as equal.
-#ifndef NDEBUG
static bool IsSameAtomicVariable(const SomeExpr &x, const SomeExpr &y) {
if (x == y) {
return true;
@@ -435,7 +434,6 @@ static bool IsSameAtomicVariable(const SomeExpr &x, const SomeExpr &y) {
return evaluate::GetSymbolVector(x) == evaluate::GetSymbolVector(y) &&
x.AsFortran() == y.AsFortran();
}
-#endif
static void SetExpr(parser::TypedExpr &expr, MaybeExpr value) {
if (value) {
@@ -630,10 +628,12 @@ void OmpStructureChecker::CheckAtomicVariable(
void OmpStructureChecker::CheckStorageOverlap(const SomeExpr &base,
llvm::ArrayRef<evaluate::Expr<evaluate::SomeType>> exprs,
- parser::CharBlock source) {
+ parser::CharBlock source, bool isError) {
if (auto *expr{HasStorageOverlap(base, exprs)}) {
context_.Say(source,
- "Within atomic operation %s and %s access the same storage"_warn_en_US,
+ isError
+ ? "Within atomic operation %s and %s access the same storage"_err_en_US
+ : "Within atomic operation %s and %s access the same storage"_warn_en_US,
base.AsFortran(), expr->AsFortran());
}
}
@@ -843,7 +843,8 @@ void OmpStructureChecker::CheckAtomicCaptureAssignment(
assert(GetConvertInput(capture.rhs) &&
IsSameAtomicVariable(*GetConvertInput(capture.rhs), atom) &&
"This cannot be a capture assignment");
- CheckStorageOverlap(atom, {cap}, source);
+ // Lowering can't handle v and x being the same, so make it an error.
+ CheckStorageOverlap(atom, {cap}, source, /*isError=*/true);
}
}
@@ -860,7 +861,8 @@ void OmpStructureChecker::CheckAtomicReadAssignment(
} else {
CheckAtomicVariable(
atom, rsrc, /*checkTypeOnPointer=*/!IsPointerAssignment(read));
- CheckStorageOverlap(atom, {read.lhs}, source);
+ // Lowering can't handle v and x being the same, so make it an error.
+ CheckStorageOverlap(atom, {read.lhs}, source, /*isError=*/true);
}
} else {
ErrorShouldBeVariable(read.rhs, rsrc);
@@ -1407,6 +1409,14 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, update.ift.source);
if (auto maybeCapture{GetEvaluateAssignment(capture.stmt())}) {
+ // CheckAtomicCaptureAssignment expects the capture to read x.
+ if (auto rhs{GetConvertInput(maybeCapture->rhs)};
+ !rhs || !IsSameAtomicVariable(*rhs, atom)) {
+ context_.Say(capture.source,
+ "In ATOMIC UPDATE COMPARE CAPTURE the right-hand side of the capture assignment should read %s"_err_en_US,
+ atom.AsFortran());
+ return;
+ }
CheckAtomicCaptureAssignment(*maybeCapture, atom, capture.source);
if (IsPointerAssignment(*maybeWrite) !=
@@ -1415,6 +1425,25 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
"The update and capture assignments should both be pointer-assignments or both be non-pointer-assignments"_err_en_US);
return;
}
+
+ // Since OpenMP 5.2, v and e must be different when v gets the
+ // original value of x.
+ llvm::omp::Version version{context_.langOptions().getOpenMPVersion()};
+ if (version >= 52 && captureFirst && captureAlways) {
+ auto top{GetTopLevelOperationIgnoreResizing(update.cond)};
+ if (top.first == operation::Operator::Eq ||
+ top.first == operation::Operator::Eqv) {
+ const SomeExpr &expected{IsSameOrConvertOf(top.second[0], atom)
+ ? top.second[1]
+ : top.second[0]};
+ if (HasStorageOverlap(maybeCapture->lhs, {expected})) {
+ context_.Say(capture.source,
+ "The capture variable %s and the expected value %s should not access the same storage when the original value of %s is captured"_err_en_US,
+ maybeCapture->lhs.AsFortran(), expected.AsFortran(),
+ atom.AsFortran());
+ }
+ }
+ }
} else {
if (!IsAssignment(capture.stmt())) {
context_.Say(capture.source,
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 530ad90abcefc..fd93959ec7a3f 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -268,7 +268,8 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
// check-omp-atomic.cpp
void CheckStorageOverlap(const evaluate::Expr<evaluate::SomeType> &,
- llvm::ArrayRef<evaluate::Expr<evaluate::SomeType>>, parser::CharBlock);
+ llvm::ArrayRef<evaluate::Expr<evaluate::SomeType>>, parser::CharBlock,
+ bool isError = false);
void ErrorShouldBeVariable(const MaybeExpr &expr, parser::CharBlock source);
void CheckAtomicType(SymbolRef sym, parser::CharBlock source,
std::string_view name, bool checkTypeOnPointer = true);
diff --git a/flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90 b/flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90
new file mode 100644
index 0000000000000..3b6327e21d6e6
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90
@@ -0,0 +1,60 @@
+! RUN: not %flang_fc1 %openmp_flags -fopenmp-version=51 -fsyntax-only %s 2>&1 | FileCheck %s
+! RUN: not %flang_fc1 %openmp_flags -fopenmp-version=52 -fsyntax-only %s 2>&1 | FileCheck %s --check-prefixes=CHECK,OMP52
+
+! v and x can't be the same variable in ATOMIC READ/CAPTURE. Since OpenMP
+! 5.2, v and e also can't be the same when v gets the original value of x.
+! Other overlaps are still warnings.
+
+subroutine f00(x)
+ integer :: x
+ !$omp atomic read
+ x = x
+end
+
+subroutine f01(x)
+ integer :: x
+ !$omp atomic capture
+ x = x
+ x = x + 1
+ !$omp end atomic
+end
+
+subroutine f02(x, e, d)
+ integer :: x, e, d
+ !$omp atomic compare capture
+ x = x
+ if (x == e) x = d
+ !$omp end atomic
+end
+
+subroutine f03(x)
+ integer :: x
+ !$omp atomic write
+ x = x + 1
+end
+
+! v gets the original value of x.
+subroutine f04(x, v, d)
+ integer :: x, v, d
+ !$omp atomic compare capture
+ v = x
+ if (x == v) x = d
+ !$omp end atomic
+end
+
+! v gets the final value of x, so v and e can be the same.
+subroutine f05(x, v, d)
+ integer :: x, v, d
+ !$omp atomic compare capture
+ if (x == v) x = d
+ v = x
+ !$omp end atomic
+end
+
+!CHECK: error: Semantic errors in
+!CHECK: :11:3: error: Within atomic operation x and x access the same storage
+!CHECK: :17:3: error: Within atomic operation x and x access the same storage
+!CHECK: :25:3: error: Within atomic operation x and x access the same storage
+!CHECK: :33:3: warning: Within atomic operation x and x+1_4 access the same storage
+!OMP52: :40:3: error: The capture variable v and the expected value v should not access the same storage when the original value of x is captured
+!CHECK-NOT: should not access the same storage
diff --git a/flang/test/Semantics/OpenMP/atomic-compare-capture-invalid.f90 b/flang/test/Semantics/OpenMP/atomic-compare-capture-invalid.f90
new file mode 100644
index 0000000000000..092d2045c0cff
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic-compare-capture-invalid.f90
@@ -0,0 +1,33 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=51
+
+! The capture statement in ATOMIC COMPARE CAPTURE must read x.
+
+subroutine f00(x, v, e, d)
+ integer :: x, v, e, d
+ !$omp atomic compare capture
+ !ERROR: In ATOMIC UPDATE COMPARE CAPTURE the right-hand side of the capture assignment should read x
+ v = v
+ if (x == e) x = d
+ !$omp end atomic
+end
+
+subroutine f01(x, v, e, d)
+ integer :: x, v, e, d
+ !$omp atomic compare capture
+ if (x == e) x = d
+ !ERROR: In ATOMIC UPDATE COMPARE CAPTURE the right-hand side of the capture assignment should read x
+ v = e
+ !$omp end atomic
+end
+
+subroutine f02(x, v, e, d)
+ integer :: x, v, e, d
+ !$omp atomic compare capture
+ if (x == e) then
+ x = d
+ else
+ !ERROR: In ATOMIC UPDATE COMPARE CAPTURE the right-hand side of the capture assignment should read x
+ v = d
+ end if
+ !$omp end atomic
+end
>From dafb81f27dc4e4444ee60adcdbf48d858fb08b61 Mon Sep 17 00:00:00 2001
From: Ritanya-B-Bharadwaj <ritanya.b.bharadwaj at gmail.com>
Date: Mon, 19 Oct 2026 22:25:38 +0530
Subject: [PATCH 2/3] Use OpenMP spec references in comments
---
flang/lib/Semantics/check-omp-atomic.cpp | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index 7f2234bf21a77..db413dafcce47 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -843,7 +843,9 @@ void OmpStructureChecker::CheckAtomicCaptureAssignment(
assert(GetConvertInput(capture.rhs) &&
IsSameAtomicVariable(*GetConvertInput(capture.rhs), atom) &&
"This cannot be a capture assignment");
- // Lowering can't handle v and x being the same, so make it an error.
+ // [6.0:194:32-33]
+ // None of v, x, d, r, expr, and expr-list (as applicable) may access the
+ // same storage location as any other symbol in the list.
CheckStorageOverlap(atom, {cap}, source, /*isError=*/true);
}
}
@@ -861,7 +863,9 @@ void OmpStructureChecker::CheckAtomicReadAssignment(
} else {
CheckAtomicVariable(
atom, rsrc, /*checkTypeOnPointer=*/!IsPointerAssignment(read));
- // Lowering can't handle v and x being the same, so make it an error.
+ // [6.0:194:32-33]
+ // None of v, x, d, r, expr, and expr-list (as applicable) may access the
+ // same storage location as any other symbol in the list.
CheckStorageOverlap(atom, {read.lhs}, source, /*isError=*/true);
}
} else {
@@ -1409,7 +1413,8 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, update.ift.source);
if (auto maybeCapture{GetEvaluateAssignment(capture.stmt())}) {
- // CheckAtomicCaptureAssignment expects the capture to read x.
+ // [6.0:192:26-28], [6.0:193:9-14]
+ // The capture statement has the form v = x or v => x.
if (auto rhs{GetConvertInput(maybeCapture->rhs)};
!rhs || !IsSameAtomicVariable(*rhs, atom)) {
context_.Say(capture.source,
@@ -1426,8 +1431,9 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
return;
}
- // Since OpenMP 5.2, v and e must be different when v gets the
- // original value of x.
+ // [6.0:195:1-2] (since OpenMP 5.2)
+ // In forms that capture the original value of x in v, v may not access
+ // the same storage location as e.
llvm::omp::Version version{context_.langOptions().getOpenMPVersion()};
if (version >= 52 && captureFirst && captureAlways) {
auto top{GetTopLevelOperationIgnoreResizing(update.cond)};
>From 6076176152fdbe35db22fbfc079c042edc6de5b1 Mon Sep 17 00:00:00 2001
From: Ritanya-B-Bharadwaj <ritanya.b.bharadwaj at gmail.com>
Date: Wed, 21 Oct 2026 12:16:51 +0530
Subject: [PATCH 3/3] Split the same-storage test and use test_errors.py
---
...90 => atomic-capture-same-storage-v51.f90} | 15 ++---
.../atomic-capture-same-storage-v52.f90 | 56 +++++++++++++++++++
2 files changed, 61 insertions(+), 10 deletions(-)
rename flang/test/Semantics/OpenMP/{atomic-capture-same-storage.f90 => atomic-capture-same-storage-v51.f90} (54%)
create mode 100644 flang/test/Semantics/OpenMP/atomic-capture-same-storage-v52.f90
diff --git a/flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90 b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
similarity index 54%
rename from flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90
rename to flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
index 3b6327e21d6e6..4faf54f8ef512 100644
--- a/flang/test/Semantics/OpenMP/atomic-capture-same-storage.f90
+++ b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
@@ -1,5 +1,4 @@
-! RUN: not %flang_fc1 %openmp_flags -fopenmp-version=51 -fsyntax-only %s 2>&1 | FileCheck %s
-! RUN: not %flang_fc1 %openmp_flags -fopenmp-version=52 -fsyntax-only %s 2>&1 | FileCheck %s --check-prefixes=CHECK,OMP52
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=51
! v and x can't be the same variable in ATOMIC READ/CAPTURE. Since OpenMP
! 5.2, v and e also can't be the same when v gets the original value of x.
@@ -8,12 +7,14 @@
subroutine f00(x)
integer :: x
!$omp atomic read
+ !ERROR: Within atomic operation x and x access the same storage
x = x
end
subroutine f01(x)
integer :: x
!$omp atomic capture
+ !ERROR: Within atomic operation x and x access the same storage
x = x
x = x + 1
!$omp end atomic
@@ -22,6 +23,7 @@ subroutine f01(x)
subroutine f02(x, e, d)
integer :: x, e, d
!$omp atomic compare capture
+ !ERROR: Within atomic operation x and x access the same storage
x = x
if (x == e) x = d
!$omp end atomic
@@ -30,6 +32,7 @@ subroutine f02(x, e, d)
subroutine f03(x)
integer :: x
!$omp atomic write
+ !WARNING: Within atomic operation x and x+1_4 access the same storage
x = x + 1
end
@@ -50,11 +53,3 @@ subroutine f05(x, v, d)
v = x
!$omp end atomic
end
-
-!CHECK: error: Semantic errors in
-!CHECK: :11:3: error: Within atomic operation x and x access the same storage
-!CHECK: :17:3: error: Within atomic operation x and x access the same storage
-!CHECK: :25:3: error: Within atomic operation x and x access the same storage
-!CHECK: :33:3: warning: Within atomic operation x and x+1_4 access the same storage
-!OMP52: :40:3: error: The capture variable v and the expected value v should not access the same storage when the original value of x is captured
-!CHECK-NOT: should not access the same storage
diff --git a/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v52.f90 b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v52.f90
new file mode 100644
index 0000000000000..90319a3b1ea40
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v52.f90
@@ -0,0 +1,56 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=52
+
+! v and x can't be the same variable in ATOMIC READ/CAPTURE. Since OpenMP
+! 5.2, v and e also can't be the same when v gets the original value of x.
+! Other overlaps are still warnings.
+
+subroutine f00(x)
+ integer :: x
+ !$omp atomic read
+ !ERROR: Within atomic operation x and x access the same storage
+ x = x
+end
+
+subroutine f01(x)
+ integer :: x
+ !$omp atomic capture
+ !ERROR: Within atomic operation x and x access the same storage
+ x = x
+ x = x + 1
+ !$omp end atomic
+end
+
+subroutine f02(x, e, d)
+ integer :: x, e, d
+ !$omp atomic compare capture
+ !ERROR: Within atomic operation x and x access the same storage
+ x = x
+ if (x == e) x = d
+ !$omp end atomic
+end
+
+subroutine f03(x)
+ integer :: x
+ !$omp atomic write
+ !WARNING: Within atomic operation x and x+1_4 access the same storage
+ x = x + 1
+end
+
+! v gets the original value of x.
+subroutine f04(x, v, d)
+ integer :: x, v, d
+ !$omp atomic compare capture
+ !ERROR: The capture variable v and the expected value v should not access the same storage when the original value of x is captured
+ v = x
+ if (x == v) x = d
+ !$omp end atomic
+end
+
+! v gets the final value of x, so v and e can be the same.
+subroutine f05(x, v, d)
+ integer :: x, v, d
+ !$omp atomic compare capture
+ if (x == v) x = d
+ v = x
+ !$omp end atomic
+end
More information about the flang-commits
mailing list