[flang-commits] [flang] 8325eca - [flang][OpenMP] Diagnose invalid capture statements in ATOMIC (#233330)
via flang-commits
flang-commits at lists.llvm.org
Wed Oct 21 00:50:16 PDT 2026
Author: Ritanya-B-Bharadwaj
Date: 2026-10-21T13:20:10+05:30
New Revision: 8325eca76045387b0b8271d777e56f8b9fc8a430
URL: https://github.com/llvm/llvm-project/commit/8325eca76045387b0b8271d777e56f8b9fc8a430
DIFF: https://github.com/llvm/llvm-project/commit/8325eca76045387b0b8271d777e56f8b9fc8a430.diff
LOG: [flang][OpenMP] Diagnose invalid capture statements in ATOMIC (#233330)
OpenMP doesn't allow v and x in an atomic construct to access the same storage location ([6.0:194:32-33]). For ATOMIC READ and CAPTURE, flang only warned about this, and the code then failed in lowering. Make it an
error.
In ATOMIC COMPARE CAPTURE, also check that the capture statement has the form v = x ([6.0:192:26-28], [6.0:193:9-14]), and, from OpenMP 5.2, that v and e don't access the same storage when v gets the original value of
x ([6.0:195:1-2]).
Fixes #232594, fixes #221013
Added:
flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
flang/test/Semantics/OpenMP/atomic-capture-same-storage-v52.f90
flang/test/Semantics/OpenMP/atomic-compare-capture-invalid.f90
Modified:
flang/lib/Semantics/check-omp-atomic.cpp
flang/lib/Semantics/check-omp-structure.h
Removed:
################################################################################
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index 7e8a3ffef8909c..db413dafcce47f 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
//
diff erent variables and
diff erent 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,10 @@ void OmpStructureChecker::CheckAtomicCaptureAssignment(
assert(GetConvertInput(capture.rhs) &&
IsSameAtomicVariable(*GetConvertInput(capture.rhs), atom) &&
"This cannot be a capture assignment");
- CheckStorageOverlap(atom, {cap}, source);
+ // [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);
}
}
@@ -860,7 +863,10 @@ void OmpStructureChecker::CheckAtomicReadAssignment(
} else {
CheckAtomicVariable(
atom, rsrc, /*checkTypeOnPointer=*/!IsPointerAssignment(read));
- CheckStorageOverlap(atom, {read.lhs}, source);
+ // [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 {
ErrorShouldBeVariable(read.rhs, rsrc);
@@ -1407,6 +1413,15 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, update.ift.source);
if (auto maybeCapture{GetEvaluateAssignment(capture.stmt())}) {
+ // [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,
+ "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 +1430,26 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
"The update and capture assignments should both be pointer-assignments or both be non-pointer-assignments"_err_en_US);
return;
}
+
+ // [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)};
+ 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 038ba804638795..59c731b4d4840e 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -269,7 +269,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-v51.f90 b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
new file mode 100644
index 00000000000000..4faf54f8ef5128
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic-capture-same-storage-v51.f90
@@ -0,0 +1,55 @@
+! 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.
+! 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
+ 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
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 00000000000000..90319a3b1ea403
--- /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
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 00000000000000..092d2045c0cff3
--- /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
More information about the flang-commits
mailing list