[flang-commits] [flang] [Flang][OpenMP] Diagnose invalid op sequence in atomic capture (no crash) (PR #162884)
Krish Gupta via flang-commits
flang-commits at lists.llvm.org
Fri Oct 10 09:59:32 PDT 2025
https://github.com/KrxGu created https://github.com/llvm/llvm-project/pull/162884
### Problem:
The Flang frontend's semantic analysis can produce invalid operation sequences for OpenMP atomic capture constructs. Specifically, it may generate a WRITE→READ sequence instead of the valid READ→UPDATE or UPDATE→WRITE sequences required by the OpenMP specification. When this invalid sequence reaches the MLIR lowering stage, verification fails with a cryptic error message and the compiler crashes.
Example that triggers the bug:
```fortran
type :: t
integer :: a(10)
end type
type(t) :: x, y
!$omp atomic capture
x%a(i) = y%a(i)
v = y%a(i)
!$omp end atomic capture
```
**Solution:**
Added validation in lowerAtomic() to check the operation sequence for atomic capture constructs before generating FIR/MLIR operations. If an invalid sequence is detected, the compiler now emits a clear diagnostic message:
This prevents the crash and provides actionable feedback to users or downstream bug reporters.
**Changes:**
[Atomic.cpp](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html): Added validation check using typed enum constants for operation types
[atomic-capture-derived-array-diag.f90](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html): Negative test case verifying the diagnostic is emitted
[atomic-capture-same-element.f90](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html): Positive control test ensuring valid atomic capture constructs still work
Both tests pass, confirming the diagnostic is emitted for invalid sequences while valid constructs compile successfully.
<img width="1192" height="142" alt="image" src="https://github.com/user-attachments/assets/391aebaa-f211-4540-982b-39978147cb48" />
<img width="1192" height="142" alt="image" src="https://github.com/user-attachments/assets/def96bc5-8989-45e7-84f0-ee0ec3de63b1" />
**Notes:**
The root cause lies in the semantic analysis phase, which should be addressed in a future PR
This is a defensive fix to prevent crashes until the semantic analysis bug is resolved
The diagnostic message is intentionally generic to cover all invalid sequences, not just derived-type cases
>From 066ad69fc1b2f6b83dfef9219709f98af4c46be7 Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Fri, 10 Oct 2025 19:10:05 +0530
Subject: [PATCH] [Flang][OpenMP] Diagnose invalid op sequence in atomic
capture (no crash)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When analysis yields WRITE→READ for capture, lowering built an invalid
FIR sequence and tripped verification. Emit a clear diagnostic instead.
Adds a regression test and positive control test.
Fixes #161934
---
flang/lib/Lower/OpenMP/Atomic.cpp | 18 ++++++++++++++
.../atomic-capture-derived-array-diag.f90 | 24 +++++++++++++++++++
.../OpenMP/atomic-capture-same-element.f90 | 21 ++++++++++++++++
3 files changed, 63 insertions(+)
create mode 100644 flang/test/Lower/OpenMP/atomic-capture-derived-array-diag.f90
create mode 100644 flang/test/Lower/OpenMP/atomic-capture-same-element.f90
diff --git a/flang/lib/Lower/OpenMP/Atomic.cpp b/flang/lib/Lower/OpenMP/Atomic.cpp
index ff82a36951bfa..48016b5c21ab8 100644
--- a/flang/lib/Lower/OpenMP/Atomic.cpp
+++ b/flang/lib/Lower/OpenMP/Atomic.cpp
@@ -19,6 +19,7 @@
#include "flang/Lower/SymbolMap.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/type.h"
@@ -459,6 +460,23 @@ void Fortran::lower::omp::lowerAtomic(
} else {
int action0 = analysis.op0.what & analysis.Action;
int action1 = analysis.op1.what & analysis.Action;
+
+ // Check for invalid atomic capture sequence.
+ // Valid sequences: (READ, UPDATE) or (UPDATE, WRITE)
+ if (construct.IsCapture()) {
+ using Action = parser::OpenMPAtomicConstruct::Analysis;
+ const bool validSequence =
+ (action0 == Action::Read && action1 == Action::Update) ||
+ (action0 == Action::Update && action1 == Action::Write);
+
+ if (!validSequence) {
+ mlir::emitError(loc,
+ "OpenMP atomic capture produced an invalid operation "
+ "sequence (expected read+update or update+write)");
+ return;
+ }
+ }
+
mlir::Operation *captureOp = nullptr;
fir::FirOpBuilder::InsertPoint preAt = builder.saveInsertionPoint();
fir::FirOpBuilder::InsertPoint atomicAt, postAt;
diff --git a/flang/test/Lower/OpenMP/atomic-capture-derived-array-diag.f90 b/flang/test/Lower/OpenMP/atomic-capture-derived-array-diag.f90
new file mode 100644
index 0000000000000..d22126c04005d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-capture-derived-array-diag.f90
@@ -0,0 +1,24 @@
+! Test diagnostic for atomic capture with invalid operation sequence
+! RUN: not %flang_fc1 -emit-fir -fopenmp %s 2>&1 | FileCheck %s
+
+! This test verifies that a clear diagnostic is emitted when atomic capture
+! produces an invalid operation sequence. This can occur with derived-type
+! component array elements where different indices are used.
+
+program test_atomic_capture_invalid_sequence
+ type t1
+ integer :: i(1)
+ end type
+ type(t1) :: t2
+ integer :: j
+
+ t2%i = 0
+ j = 1
+
+ ! CHECK: error: OpenMP atomic capture produced an invalid operation sequence
+ !$omp atomic capture
+ t2%i(j*1) = t2%i(1) + 1
+ t2%i(1) = t2%i(j*1)
+ !$omp end atomic
+
+end program test_atomic_capture_invalid_sequence
diff --git a/flang/test/Lower/OpenMP/atomic-capture-same-element.f90 b/flang/test/Lower/OpenMP/atomic-capture-same-element.f90
new file mode 100644
index 0000000000000..208a2db362369
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-capture-same-element.f90
@@ -0,0 +1,21 @@
+! Test that atomic capture works correctly with same element (positive control)
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test verifies that atomic capture with the same array element in both
+! statements works correctly and doesn't trigger the invalid sequence diagnostic.
+
+! CHECK-LABEL: func @_QQmain
+program test_atomic_capture_same_element
+ integer :: a(10)
+ integer :: v
+
+ a = 0
+
+ ! This should work - same element a(1) in both statements
+ ! CHECK: omp.atomic.capture
+ !$omp atomic capture
+ v = a(1)
+ a(1) = a(1) + 1
+ !$omp end atomic
+
+end program test_atomic_capture_same_element
More information about the flang-commits
mailing list