[flang-commits] [flang] [flang][OpenMP] Diagnose invalid capture statements in ATOMIC (PR #233330)
via flang-commits
flang-commits at lists.llvm.org
Sun Oct 18 20:36:27 PDT 2026
https://github.com/Ritanya-B-Bharadwaj created https://github.com/llvm/llvm-project/pull/233330
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
>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] [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 7e8a3ffef8909c..7f2234bf21a770 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 530ad90abcefc6..fd93959ec7a3f9 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 00000000000000..3b6327e21d6e64
--- /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 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