[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 13:16:59 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/3] [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 9f08edab0a559b871388c29615d94e2a61cbd442 Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Wed, 29 Oct 2025 00:51:47 +0530
Subject: [PATCH 2/3] Add unit test for atomic read/write on struct types
Test verifies that OpenMPIRBuilder generates correct size argument (16 bytes)
for __atomic_store/__atomic_load when operating on struct types like {double,double}.
This ensures the fix for issue #165184 is covered by unit tests.
---
.../Frontend/OpenMPIRBuilderTest.cpp | 81 +++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index e56872320b4ac..c49e69ae2364d 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4534,6 +4534,87 @@ 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};
+ Value *ReadResult = nullptr;
+
+ Builder.restoreIP(
+ OMPBuilder.createAtomicRead(Loc, X, ReadResult, 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);
>From 4941db72db99b2aa61c8d9ffff933f83ca76d41b Mon Sep 17 00:00:00 2001
From: Krish Gupta <krishgupta at Krishs-MacBook-Air.local>
Date: Wed, 29 Oct 2025 01:46:48 +0530
Subject: [PATCH 3/3] Fix Windows build error and apply code formatting
- Fix unit test to pass AtomicOpValue& instead of Value* to createAtomicRead
- Apply clang-format to fix code style violations
- Remove trailing whitespace and fix indentation
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 5 +++--
.../Frontend/OpenMPIRBuilderTest.cpp | 19 ++++++++-----------
2 files changed, 11 insertions(+), 13 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 c49e69ae2364d..0b3ae643e1494 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -4548,16 +4548,16 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicRWStructType) {
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);
@@ -4571,10 +4571,8 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicRWStructType) {
AllocaInst *VVal = Builder.CreateAlloca(Complex8Ty);
VVal->setName("ReadDest");
OpenMPIRBuilder::AtomicOpValue V = {VVal, Complex8Ty, false, false};
- Value *ReadResult = nullptr;
-
- Builder.restoreIP(
- OMPBuilder.createAtomicRead(Loc, X, ReadResult, AO, AllocaIP));
+
+ Builder.restoreIP(OMPBuilder.createAtomicRead(Loc, X, V, AO, AllocaIP));
Builder.CreateRetVoid();
OMPBuilder.finalize();
@@ -4583,7 +4581,7 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicRWStructType) {
// 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
@@ -4610,7 +4608,7 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicRWStructType) {
}
}
}
-
+
EXPECT_TRUE(FoundAtomicStore) << "Did not find __atomic_store call";
EXPECT_TRUE(FoundAtomicLoad) << "Did not find __atomic_load call";
}
@@ -7657,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