[flang][OpenMP] Add diagnostic for ATOMIC WRITE with pointer to non-intrinsic type #162364

KrxGu · 2025-10-07T20:16:29Z

Fixes #161932

Problem

When using !$omp atomic write with a POINTER variable to a derived type in intrinsic assignment, Flang would crash with a FIR verifier error instead of providing a clear semantic diagnostic.

Example that previously crashed:

module p
    type t
    end type
    contains
    subroutine s(a1, a2)
        type(t), pointer :: a1, a2
        !$omp atomic write
        a1 = a2  ! Crashed with FIR verifier error
    end subroutine
end module

Solution
Added a semantic check in CheckAtomicWriteAssignment() that detects when:

The assignment is an intrinsic assignment (not pointer assignment =>)
The variable has the POINTER attribute
The pointee type is non-intrinsic (derived type, character, etc.)
The check now emits a clear error message before reaching the FIR stage:
error: ATOMIC WRITE requires an intrinsic scalar variable; 'a1' has the POINTER attribute and derived type 't'

What remains valid
According to OpenMP 6.0 spec, the following cases are still allowed:

✅ integer, pointer :: p; p = 5 - pointer to intrinsic type with intrinsic assignment
✅ integer, pointer :: p, q; p => q - pointer assignment
✅ type(t), pointer :: a, b; a => b - pointer assignment with derived type
What is now rejected
❌ type(t), pointer :: a, b; a = b - intrinsic assignment where the dereferenced value is non-intrinsic

Testing
All existing atomic tests pass (15/15)
New test validates the error diagnostic
Verified the original crash scenario now produces a clear error instead

llvmbot · 2025-10-07T20:16:59Z

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Krish Gupta (KrxGu)

Changes

Fixes #161932

Problem

When using !$omp atomic write with a POINTER variable to a derived type in intrinsic assignment, Flang would crash with a FIR verifier error instead of providing a clear semantic diagnostic.

Example that previously crashed:

module p
    type t
    end type
    contains
    subroutine s(a1, a2)
        type(t), pointer :: a1, a2
        !$omp atomic write
        a1 = a2  ! Crashed with FIR verifier error
    end subroutine
end module

Solution
Added a semantic check in CheckAtomicWriteAssignment() that detects when:

The assignment is an intrinsic assignment (not pointer assignment =>)
The variable has the POINTER attribute
The pointee type is non-intrinsic (derived type, character, etc.)
The check now emits a clear error message before reaching the FIR stage:
error: ATOMIC WRITE requires an intrinsic scalar variable; 'a1' has the POINTER attribute and derived type 't'

What remains valid
According to OpenMP 6.0 spec, the following cases are still allowed:

✅ integer, pointer :: p; p = 5 — pointer to intrinsic type with intrinsic assignment
✅ integer, pointer :: p, q; p => q — pointer assignment
✅ type(t), pointer :: a, b; a => b — pointer assignment with derived type
What is now rejected
❌ type(t), pointer :: a, b; a = b — intrinsic assignment where the dereferenced value is non-intrinsic

Testing
All existing atomic tests pass (15/15)
New test validates the error diagnostic
Verified the original crash scenario now produces a clear error instead

Full diff: https://github.com/llvm/llvm-project/pull/162364.diff

3 Files Affected:

(modified) flang/lib/Semantics/check-omp-atomic.cpp (+25-1)
(added) flang/test/Semantics/OpenMP/omp-atomic-write-pointer-derived.f90 (+8)
(added) flang/test/Semantics/OpenMP/omp-common-fp-lp.f90 (+13)

diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index 351af5c099aee..00c6a2d2b5fe3 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -539,7 +539,6 @@ void OmpStructureChecker::CheckAtomicType(
     return;
   }
 
-  // Variable is a pointer.
   if (typeSpec->IsPolymorphic()) {
     context_.Say(source,
         "Atomic variable %s cannot be a pointer to a polymorphic type"_err_en_US,
@@ -829,6 +828,31 @@ void OmpStructureChecker::CheckAtomicWriteAssignment(
   if (!IsVarOrFunctionRef(atom)) {
     ErrorShouldBeVariable(atom, rsrc);
   } else {
+    // For intrinsic assignment (x = expr), check if the variable is a pointer
+    // to a non-intrinsic type, which is not allowed in ATOMIC WRITE
+    if (!IsPointerAssignment(write)) {
+      std::vector<SomeExpr> dsgs{GetAllDesignators(atom)};
+      if (!dsgs.empty()) {
+        evaluate::SymbolVector syms{evaluate::GetSymbolVector(dsgs.front())};
+        if (!syms.empty() && IsPointer(syms.back())) {
+          SymbolRef sym = syms.back();
+          if (const DeclTypeSpec *typeSpec{sym->GetType()}) {
+            using Category = DeclTypeSpec::Category;
+            Category cat{typeSpec->category()};
+            if (cat != Category::Numeric && cat != Category::Logical) {
+              std::string details = " has the POINTER attribute";
+              if (const auto *derived{typeSpec->AsDerived()}) {
+                details += " and derived type '"s + derived->name().ToString() + "'";
+              }
+              context_.Say(lsrc,
+                  "ATOMIC WRITE requires an intrinsic scalar variable; '%s'%s"_err_en_US,
+                  sym->name(), details);
+              return;
+            }
+          }
+        }
+      }
+    }
     CheckAtomicVariable(atom, lsrc);
     CheckStorageOverlap(atom, {write.rhs}, source);
   }
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-write-pointer-derived.f90 b/flang/test/Semantics/OpenMP/omp-atomic-write-pointer-derived.f90
new file mode 100644
index 0000000000000..d1ca2308047ad
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/omp-atomic-write-pointer-derived.f90
@@ -0,0 +1,8 @@
+! RUN: not %flang_fc1 -fopenmp -fsyntax-only %s 2>&1 | FileCheck %s
+type t
+end type
+type(t), pointer :: a1, a2
+!$omp atomic write
+a1 = a2
+! CHECK: error: ATOMIC WRITE requires an intrinsic scalar variable; 'a1' has the POINTER attribute and derived type 't'
+end
diff --git a/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90 b/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90
new file mode 100644
index 0000000000000..3d6ba1547a09e
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -fopenmp -fsyntax-only %s 2>&1 | FileCheck %s --allow-empty
+! CHECK-NOT: error:
+! CHECK-NOT: warning:
+
+subroutine sub1()
+  common /com/ j
+  j = 10
+!$omp parallel do firstprivate(j) lastprivate(/com/)
+  do i = 1, 10
+     j = j + 1
+  end do
+!$omp end parallel do
+end

github-actions · 2025-10-07T20:18:34Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kparzysz · 2025-10-07T20:35:06Z

flang/lib/Semantics/check-omp-atomic.cpp

@@ -539,7 +539,6 @@ void OmpStructureChecker::CheckAtomicType(
    return;
  }

-  // Variable is a pointer.


Unrelated change

kparzysz · 2025-10-07T20:37:42Z

flang/lib/Semantics/check-omp-atomic.cpp

@@ -829,6 +828,32 @@ void OmpStructureChecker::CheckAtomicWriteAssignment(
  if (!IsVarOrFunctionRef(atom)) {
    ErrorShouldBeVariable(atom, rsrc);
  } else {
+    // For intrinsic assignment (x = expr), check if the variable is a pointer
+    // to a non-intrinsic type, which is not allowed in ATOMIC WRITE


This isn't specific to WRITE. It should be checked in CheckAtomicType.

kparzysz · 2025-10-07T20:38:39Z

If you want to work on an issue that is already assigned to someone, at least let them know.

KrxGu · 2025-10-07T21:13:34Z

If you want to work on an issue that is already assigned to someone, at least let them know.

Hi @kparzysz — apologies for the oversight🙏. I missed that this was already self-assigned to you. I’ll make sure to check assignments and coordinate with the assignee before picking up work in the future.

I’ve opened #162364 with a semantics fix and tests. If you feel it addresses the issue, please go ahead and merge it. If there are minor changes needed, I’m happy to make them quickly. Otherwise, if it’s not up to the mark or conflicts with your approach, I can close the PR. Your call, and thank you for the guidance.

kparzysz · 2025-10-11T13:25:40Z

Did you miss my first review comment by any chance?

…ntrinsic type Fixes llvm#161932

KrxGu · 2025-10-12T06:25:33Z

Did you miss my first review comment by any chance?

Thank you for the feedback! I've addressed both of your comments:

Unrelated change: The deleted comment has been restored.
Centralization: I've moved the pointer-to-non-intrinsic check to a centralized location. Instead of modifying [CheckAtomicType](which doesn't have assignment context), I created an overload of [CheckAtomicVariable] that accepts the assignment parameter. This overload is now called by all atomic operations (WRITE, UPDATE, READ, CAPTURE, and CONDITIONAL UPDATE), ensuring the check applies universally.

The changes have been tested and all atomic operations now correctly detect pointers to non-intrinsic types.

KrxGu · 2025-10-12T08:32:16Z

Updated the FileCheck pattern to match the generic “ATOMIC operation …” diagnostic emitted from the centralized checker. The test remains a negative case for intrinsic assignment to a pointer-to-derived.

CI all green, @kparzysz. good to merge!!

kparzysz · 2025-10-13T16:22:39Z

This looks good! I have a suggestion:

Remove the CheckAtomicVariable overload, instead add a parameter e.g. "bool CheckTypeOnPointer = true". Pass "!IsPointerAssignment(...)" at the call sites where needed. Add the same parameter to CheckAtomicType and pass it from CheckAtomicVariable. Then put the actual check into CheckAtomicType.

This way the checks are done where they logically belong.

KrxGu · 2025-10-14T20:38:40Z

This looks good! I have a suggestion:

Remove the CheckAtomicVariable overload, instead add a parameter e.g. "bool CheckTypeOnPointer = true". Pass "!IsPointerAssignment(...)" at the call sites where needed. Add the same parameter to CheckAtomicType and pass it from CheckAtomicVariable. Then put the actual check into CheckAtomicType.

This way the checks are done where they logically belong.

Thank you for the feedback! I've implemented your suggestions:

Removed the overload - Deleted the CheckAtomicVariable(atom, source, assign) overload
Added bool parameter - Added bool checkTypeOnPointer = true to both CheckAtomicVariable and CheckAtomicType
Updated call sites - All atomic assignment functions now pass !IsPointerAssignment(...) to control the check:
- CheckAtomicWriteAssignment
- CheckAtomicReadAssignment
- CheckAtomicCaptureAssignment
- CheckAtomicUpdateAssignment
- CheckAtomicConditionalUpdateAssignment
Moved logic to CheckAtomicType - The pointer-to-non-intrinsic check now lives in CheckAtomicType, guarded by the checkTypeOnPointer flag

The check now applies uniformly to all atomic operations while properly excluding pointer assignments (=>). Tests pass locally with the updated implementation.

@kparzysz CI is all green, good to merge!!

kparzysz · 2025-10-15T12:26:14Z

The commit message is too verbose, plus it contains outdated information. I plan to reduce it when merging. Let me know if you're ok with the text below.

Added a check for non-intrinsic types in non-pointer assignments.

Added bool checkTypeOnPointer = true to both CheckAtomicVariable and CheckAtomicType. All atomic assignment functions now pass !IsPointerAssignment(...) to control the check. The pointer-to-non-intrinsic check lives in CheckAtomicType, guarded by the checkTypeOnPointer flag. The check now applies uniformly to all atomic operations while properly excluding pointer assignments (=>).

KrxGu · 2025-10-15T18:29:04Z

The commit message is too verbose, plus it contains outdated information. I plan to reduce it when merging. Let me know if you're ok with the text below.

Added a check for non-intrinsic types in non-pointer assignments.

Added bool checkTypeOnPointer = true to both CheckAtomicVariable and CheckAtomicType. All atomic assignment functions now pass !IsPointerAssignment(...) to control the check. The pointer-to-non-intrinsic check lives in CheckAtomicType, guarded by the checkTypeOnPointer flag. The check now applies uniformly to all atomic operations while properly excluding pointer assignments (=>).

@kparzysz Yes, SGTM - please go ahead with your version of the commit message. I appreciate you cleaning it up!

llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Oct 7, 2025

KrxGu force-pushed the flang-omp-atomic-write-diag branch 2 times, most recently from 40781a0 to 28e3606 Compare October 7, 2025 20:25

kparzysz reviewed Oct 7, 2025

View reviewed changes

[flang][OpenMP] Add diagnostic for ATOMIC WRITE with pointer to non-i…

022dd65

…ntrinsic type Fixes llvm#161932

KrxGu force-pushed the flang-omp-atomic-write-diag branch from 28e3606 to 022dd65 Compare October 12, 2025 05:57

[flang][OpenMP] Centralize pointer-to-non-intrinsic check via bool flag

65472a5

KrxGu force-pushed the flang-omp-atomic-write-diag branch from 6a88852 to 65472a5 Compare October 14, 2025 20:12

kparzysz approved these changes Oct 14, 2025

View reviewed changes

kparzysz merged commit 59ab4d4 into llvm:main Oct 15, 2025
10 checks passed

kparzysz mentioned this pull request Oct 30, 2025

[Flang][OpenMP] Compilation error when statement in ATOMIC construct is MAX intrinsic function whose argument is CHARACTER variable with POINTER attribute #156268

Closed

[flang][OpenMP] Add diagnostic for ATOMIC WRITE with pointer to non-intrinsic type #162364

[flang][OpenMP] Add diagnostic for ATOMIC WRITE with pointer to non-intrinsic type #162364

Uh oh!

Conversation

KrxGu commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Uh oh!

llvmbot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Uh oh!

github-actions bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kparzysz Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

kparzysz Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

kparzysz commented Oct 7, 2025

Uh oh!

KrxGu commented Oct 7, 2025

Uh oh!

kparzysz commented Oct 11, 2025

Uh oh!

KrxGu commented Oct 12, 2025

Uh oh!

KrxGu commented Oct 12, 2025

Uh oh!

kparzysz commented Oct 13, 2025

Uh oh!

KrxGu commented Oct 14, 2025

Uh oh!

kparzysz commented Oct 15, 2025

Uh oh!

KrxGu commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KrxGu commented Oct 7, 2025 •

edited

Loading

llvmbot commented Oct 7, 2025 •

edited

Loading

github-actions bot commented Oct 7, 2025 •

edited

Loading