[flang-commits] [flang] [llvm] [OpenMP][Flang] Fix atomic operations on complex types (PR #165366)

via flang-commits flang-commits at lists.llvm.org
Tue Oct 28 03:23:22 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: Krish Gupta (KrxGu)

<details>
<summary>Changes</summary>

Fixes #<!-- -->165184

## Problem
When using OpenMP atomic read/write on Fortran `complex(8)` (16 bytes) or `complex(16)` (32 bytes), only the real part was being stored/loaded. The imaginary part remained unchanged (typically 0).

## Root Cause
In `OMPIRBuilder::createAtomicRead()` and `createAtomicWrite()`, the size parameter for `__atomic_load`/`__atomic_store` was incorrectly computed from the pointer type instead of the pointee (element) type.

On 64-bit systems, this resulted in only 8 bytes being transferred regardless of the actual struct size.

## Solution
Changed both functions to use `XElemTy` (element type) instead of the pointer type when computing `LoadSize`. This ensures the full struct is transferred:
- `complex(4)`: 8 bytes ✓
- `complex(8)`: 16 bytes ✓ (was 8 before)
- `complex(16)`: 32 bytes ✓ (was 8 before)

## Tests
Added three regression tests:
- `atomic-write-complex.f90` - Verifies IR for atomic write
<img width="1382" height="198" alt="image" src="https://github.com/user-attachments/assets/6b03b804-2c1e-4dc8-8b4e-b9a76166f648" />

- `atomic-read-complex.f90` - Verifies IR for atomic read  
<img width="1382" height="198" alt="image" src="https://github.com/user-attachments/assets/35781219-8d92-4918-a957-cf65052229b6" />

- `issue-165184-atomic-complex8.f90` - Direct reproducer from the issue

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


4 Files Affected:

- (added) flang/test/Integration/OpenMP/atomic-read-complex.f90 (+44) 
- (added) flang/test/Integration/OpenMP/atomic-write-complex.f90 (+44) 
- (added) flang/test/Integration/OpenMP/issue-165184-atomic-complex8.f90 (+32) 
- (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+4-6) 


``````````diff
diff --git a/flang/test/Integration/OpenMP/atomic-read-complex.f90 b/flang/test/Integration/OpenMP/atomic-read-complex.f90
new file mode 100644
index 0000000000000..53ad7acbfaf49
--- /dev/null
+++ b/flang/test/Integration/OpenMP/atomic-read-complex.f90
@@ -0,0 +1,44 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple
+! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
+! contain executable tests. We should only add tests here sparingly and only
+! if there is no other way to test. Repeat this message in each test that is
+! added to this directory and sub-directories.
+!===----------------------------------------------------------------------===!
+
+! REQUIRES: x86-registered-target || aarch64-registered-target
+
+! RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
+
+! Test that atomic read operations with complex types emit the correct
+! size to __atomic_load. This is a regression test for issue #165184.
+!
+! For complex(4) (8 bytes total): should call __atomic_load(i64 8, ...)
+! For complex(8) (16 bytes total): should call __atomic_load(i64 16, ...)
+! For complex(16) (32 bytes total): should call __atomic_load(i64 32, ...)
+
+program atomic_read_complex
+  implicit none
+
+  ! Test complex(4) - single precision (8 bytes)
+  complex(4) :: c41, c42
+  ! Test complex(8) - double precision (16 bytes)
+  complex(8) :: c81, c82
+  
+  c42 = (1.0_4, 1.0_4)
+  c82 = (1.0_8, 1.0_8)
+
+  ! CHECK-LABEL: define {{.*}} @_QQmain
+
+  ! Single precision complex: 8 bytes
+  ! CHECK: call void @__atomic_load(i64 8, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
+!$omp atomic read
+  c41 = c42
+  
+  ! Double precision complex: 16 bytes (this was broken before the fix)
+  ! CHECK: call void @__atomic_load(i64 16, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
+!$omp atomic read
+  c81 = c82
+
+end program atomic_read_complex
diff --git a/flang/test/Integration/OpenMP/atomic-write-complex.f90 b/flang/test/Integration/OpenMP/atomic-write-complex.f90
new file mode 100644
index 0000000000000..7302328fba3bf
--- /dev/null
+++ b/flang/test/Integration/OpenMP/atomic-write-complex.f90
@@ -0,0 +1,44 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple
+! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
+! contain executable tests. We should only add tests here sparingly and only
+! if there is no other way to test. Repeat this message in each test that is
+! added to this directory and sub-directories.
+!===----------------------------------------------------------------------===!
+
+! REQUIRES: x86-registered-target || aarch64-registered-target
+
+! RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
+
+! Test that atomic write operations with complex types emit the correct
+! size to __atomic_store. This is a regression test for issue #165184.
+!
+! For complex(4) (8 bytes total): should call __atomic_store(i64 8, ...)
+! For complex(8) (16 bytes total): should call __atomic_store(i64 16, ...)
+! For complex(16) (32 bytes total): should call __atomic_store(i64 32, ...)
+
+program atomic_write_complex
+  implicit none
+
+  ! Test complex(4) - single precision (8 bytes)
+  complex(4) :: c41, c42
+  ! Test complex(8) - double precision (16 bytes)  
+  complex(8) :: c81, c82
+  
+  c42 = (1.0_4, 1.0_4)
+  c82 = (1.0_8, 1.0_8)
+
+  ! CHECK-LABEL: define {{.*}} @_QQmain
+  
+  ! Single precision complex: 8 bytes
+  ! CHECK: call void @__atomic_store(i64 8, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
+!$omp atomic write
+  c41 = c42
+  
+  ! Double precision complex: 16 bytes (this was broken before the fix)
+  ! CHECK: call void @__atomic_store(i64 16, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
+!$omp atomic write
+  c81 = c82
+
+end program atomic_write_complex
diff --git a/flang/test/Integration/OpenMP/issue-165184-atomic-complex8.f90 b/flang/test/Integration/OpenMP/issue-165184-atomic-complex8.f90
new file mode 100644
index 0000000000000..8c930191ad233
--- /dev/null
+++ b/flang/test/Integration/OpenMP/issue-165184-atomic-complex8.f90
@@ -0,0 +1,32 @@
+! RUN: %flang_fc1 -emit-llvm -fopenmp -o - %s | FileCheck %s
+! RUN: %flang -fopenmp %s -o %t && %t | FileCheck %s --check-prefix=EXEC
+
+! Regression test for issue #165184:
+! Atomic write to complex(8) was only storing 8 bytes instead of 16,
+! causing the imaginary part to remain 0.
+
+program test_atomic_write_complex8
+  implicit none
+  complex(8) :: c81, c82
+  
+  c81 = (0.0_8, 0.0_8)
+  c82 = (0.0_8, 0.0_8)
+  
+  ! Verify the fix: __atomic_store should be called with size 16
+  ! CHECK: call void @__atomic_store(i64 16,
+  
+!$omp parallel
+!$omp atomic write
+  c81 = c82 + (1.0_8, 1.0_8)
+!$omp end parallel
+  
+  ! EXEC: c81 = (1.00000000000000,1.00000000000000)
+  write(*,*) "c81 = ", c81
+  
+  ! Verify both parts are correct
+  if (real(c81) /= 1.0_8 .or. aimag(c81) /= 1.0_8) then
+    write(*,*) "ERROR: Expected (1.0,1.0) but got", c81
+    stop 1
+  end if
+  
+end program test_atomic_write_complex8
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 286ed039b1214..c71b4a1b6eaa1 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -9338,9 +9338,8 @@ OpenMPIRBuilder::createAtomicRead(const LocationDescription &Loc,
     // target does not support `atomicrmw` of the size of the struct
     LoadInst *OldVal = Builder.CreateLoad(XElemTy, X.Var, "omp.atomic.read");
     OldVal->setAtomic(AO);
-    const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
-    unsigned LoadSize =
-        LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
+    const DataLayout &DL = OldVal->getModule()->getDataLayout();
+    unsigned LoadSize = DL.getTypeStoreSize(XElemTy);
     OpenMPIRBuilder::AtomicInfo atomicInfo(
         &Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
         OldVal->getAlign(), true /* UseLibcall */, AllocaIP, X.Var);
@@ -9384,9 +9383,8 @@ OpenMPIRBuilder::createAtomicWrite(const LocationDescription &Loc,
     XSt->setAtomic(AO);
   } else if (XElemTy->isStructTy()) {
     LoadInst *OldVal = Builder.CreateLoad(XElemTy, X.Var, "omp.atomic.read");
-    const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
-    unsigned LoadSize =
-        LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
+    const DataLayout &DL = OldVal->getModule()->getDataLayout();
+    unsigned LoadSize = DL.getTypeStoreSize(XElemTy);
     OpenMPIRBuilder::AtomicInfo atomicInfo(
         &Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
         OldVal->getAlign(), true /* UseLibcall */, AllocaIP, X.Var);

``````````

</details>


https://github.com/llvm/llvm-project/pull/165366


More information about the flang-commits mailing list