[flang-commits] [flang] [llvm] [OpenMP][Flang] Fix atomic operations on complex types (PR #165366)
Krish Gupta via flang-commits
flang-commits at lists.llvm.org
Tue Oct 28 22:14:47 PDT 2025
https://github.com/KrxGu updated https://github.com/llvm/llvm-project/pull/165366
>From 5524ecde3ccd8a34c27201073ee2b86e33b680e8 Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Tue, 28 Oct 2025 19:52:38 +0530
Subject: [PATCH 1/2] [OpenMP][Flang] Fix atomic size for complex types
Fixes #165184
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.
Changed both functions to use XElemTy (element type) instead of the
pointer type when computing LoadSize. This ensures the full struct
is transferred.
---
.../test/Lower/OpenMP/atomic-read-complex.f90 | 34 +++++++++++++++++++
.../Lower/OpenMP/atomic-write-complex.f90 | 34 +++++++++++++++++++
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 10 +++---
3 files changed, 72 insertions(+), 6 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/atomic-read-complex.f90
create mode 100644 flang/test/Lower/OpenMP/atomic-write-complex.f90
diff --git a/flang/test/Lower/OpenMP/atomic-read-complex.f90 b/flang/test/Lower/OpenMP/atomic-read-complex.f90
new file mode 100644
index 0000000000000..2f51f03820926
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-read-complex.f90
@@ -0,0 +1,34 @@
+! Test lowering of atomic read to LLVM IR for complex types.
+! This is a regression test for issue #165184.
+
+! RUN: %flang_fc1 -emit-llvm -fopenmp -o - %s | FileCheck %s
+
+! Test that atomic read operations with complex types emit the correct
+! size parameter to __atomic_load:
+! - complex(4) (8 bytes total): should call __atomic_load(i64 8, ...)
+! - complex(8) (16 bytes total): should call __atomic_load(i64 16, ...)
+
+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/Lower/OpenMP/atomic-write-complex.f90 b/flang/test/Lower/OpenMP/atomic-write-complex.f90
new file mode 100644
index 0000000000000..48cfe26ca5a49
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-write-complex.f90
@@ -0,0 +1,34 @@
+! Test lowering of atomic write to LLVM IR for complex types.
+! This is a regression test for issue #165184.
+
+! RUN: %flang_fc1 -emit-llvm -fopenmp -o - %s | FileCheck %s
+
+! Test that atomic write operations with complex types emit the correct
+! size parameter to __atomic_store:
+! - complex(4) (8 bytes total): should call __atomic_store(i64 8, ...)
+! - complex(8) (16 bytes total): should call __atomic_store(i64 16, ...)
+
+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/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);
>From 81e5343a70cea784c969c906eac8ad71ab7f7c77 Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Wed, 29 Oct 2025 10:44:25 +0530
Subject: [PATCH 2/2] [OpenMP][Flang] Fix atomic operations size for complex
types
OpenMP atomic operations on complex(8) were using pointer size (8 bytes)
instead of the actual element size (16 bytes), causing only 8 bytes to
be stored/loaded instead of the full 16 bytes.
Fixed by using DL.getTypeStoreSize(XElemTy) instead of pointer type size
in createAtomicRead and createAtomicWrite for both atomic operations.
Added Flang IR tests to verify correct size generation for complex(4)
and complex(8) atomic operations, and unit test to verify struct types
with __atomic_store/__atomic_load.
Fixes #165184
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 5 +-
.../Frontend/OpenMPIRBuilderTest.cpp | 82 ++++++++++++++++++-
2 files changed, 83 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index c71b4a1b6eaa1..0e5926ff0fb18 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -5473,7 +5473,8 @@ OpenMPIRBuilder::collapseLoops(DebugLoc DL, ArrayRef<CanonicalLoopInfo *> Loops,
}
// TODO: Enable UndefinedSanitizer to diagnose an overflow here.
- CollapsedTripCount = Builder.CreateNUWMul(CollapsedTripCount, OrigTripCount);
+ CollapsedTripCount =
+ Builder.CreateNUWMul(CollapsedTripCount, OrigTripCount);
}
// Create the collapsed loop control flow.
@@ -9579,7 +9580,7 @@ Expected<std::pair<Value *, Value *>> OpenMPIRBuilder::emitAtomicUpdate(
OldVal->setAtomic(AO);
// CurBB
// | /---\
- // ContBB |
+ // ContBB |
// | \---/
// ExitBB
BasicBlock *CurBB = Builder.GetInsertBlock();
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index e56872320b4ac..0b3ae643e1494 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4534,6 +4534,85 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicCompareCapture) {
EXPECT_FALSE(verifyModule(*M, &errs()));
}
+TEST_F(OpenMPIRBuilderTest, OMPAtomicRWStructType) {
+ // Test for issue #165184: atomic read/write on struct types should use
+ // element type size, not pointer size.
+ OpenMPIRBuilder OMPBuilder(*M);
+ OMPBuilder.initialize();
+ F->setName("func");
+ IRBuilder<> Builder(BB);
+
+ OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+ BasicBlock *EntryBB = BB;
+ OpenMPIRBuilder::InsertPointTy AllocaIP(EntryBB,
+ EntryBB->getFirstInsertionPt());
+
+ LLVMContext &Ctx = M->getContext();
+
+ // Create a struct type {double, double} to simulate complex(8) - 16 bytes
+ StructType *Complex8Ty = StructType::create(
+ Ctx, {Type::getDoubleTy(Ctx), Type::getDoubleTy(Ctx)}, "complex");
+
+ AllocaInst *XVal = Builder.CreateAlloca(Complex8Ty);
+ XVal->setName("AtomicVar");
+ OpenMPIRBuilder::AtomicOpValue X = {XVal, Complex8Ty, false, false};
+ AtomicOrdering AO = AtomicOrdering::SequentiallyConsistent;
+
+ // Create value to write: {1.0, 1.0}
+ Constant *Real = ConstantFP::get(Type::getDoubleTy(Ctx), 1.0);
+ Constant *Imag = ConstantFP::get(Type::getDoubleTy(Ctx), 1.0);
+ Constant *ValToWrite = ConstantStruct::get(Complex8Ty, {Real, Imag});
+
+ // Test atomic write
+ Builder.restoreIP(
+ OMPBuilder.createAtomicWrite(Loc, X, ValToWrite, AO, AllocaIP));
+
+ // Test atomic read
+ AllocaInst *VVal = Builder.CreateAlloca(Complex8Ty);
+ VVal->setName("ReadDest");
+ OpenMPIRBuilder::AtomicOpValue V = {VVal, Complex8Ty, false, false};
+
+ Builder.restoreIP(OMPBuilder.createAtomicRead(Loc, X, V, AO, AllocaIP));
+
+ Builder.CreateRetVoid();
+ OMPBuilder.finalize();
+ EXPECT_FALSE(verifyModule(*M, &errs()));
+
+ // Verify that __atomic_store and __atomic_load are called with size 16
+ bool FoundAtomicStore = false;
+ bool FoundAtomicLoad = false;
+
+ for (Function &Fn : *M) {
+ if (Fn.getName().starts_with("__atomic_store")) {
+ // Check that first call to __atomic_store has size argument = 16
+ for (User *U : Fn.users()) {
+ if (auto *CB = dyn_cast<CallBase>(U)) {
+ if (auto *SizeArg = dyn_cast<ConstantInt>(CB->getArgOperand(0))) {
+ EXPECT_EQ(SizeArg->getZExtValue(), 16U);
+ FoundAtomicStore = true;
+ break;
+ }
+ }
+ }
+ }
+ if (Fn.getName().starts_with("__atomic_load")) {
+ // Check that first call to __atomic_load has size argument = 16
+ for (User *U : Fn.users()) {
+ if (auto *CB = dyn_cast<CallBase>(U)) {
+ if (auto *SizeArg = dyn_cast<ConstantInt>(CB->getArgOperand(0))) {
+ EXPECT_EQ(SizeArg->getZExtValue(), 16U);
+ FoundAtomicLoad = true;
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ EXPECT_TRUE(FoundAtomicStore) << "Did not find __atomic_store call";
+ EXPECT_TRUE(FoundAtomicLoad) << "Did not find __atomic_load call";
+}
+
TEST_F(OpenMPIRBuilderTest, CreateTeams) {
using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
OpenMPIRBuilder OMPBuilder(*M);
@@ -7576,8 +7655,7 @@ TEST_F(OpenMPIRBuilderTest, CreateTaskgroup) {
// Checking the general structure of the IR generated is same as expected.
Instruction *GeneratedStoreInst = TaskgroupCall->getNextNode();
EXPECT_EQ(GeneratedStoreInst, InternalStoreInst);
- Instruction *GeneratedLoad32 =
- GeneratedStoreInst->getNextNode();
+ Instruction *GeneratedLoad32 = GeneratedStoreInst->getNextNode();
EXPECT_EQ(GeneratedLoad32, InternalLoad32);
Instruction *GeneratedLoad128 = GeneratedLoad32->getNextNode();
EXPECT_EQ(GeneratedLoad128, InternalLoad128);
More information about the flang-commits
mailing list