[llvm] 22a10c8 - [OpenMP][Flang] Fix atomic operations on complex types (#165366)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 29 06:55:56 PDT 2025
Author: Krish Gupta
Date: 2025-10-29T08:55:52-05:00
New Revision: 22a10c8c2141b1a2794f475272281b1ad64f56cb
URL: https://github.com/llvm/llvm-project/commit/22a10c8c2141b1a2794f475272281b1ad64f56cb
DIFF: https://github.com/llvm/llvm-project/commit/22a10c8c2141b1a2794f475272281b1ad64f56cb.diff
LOG: [OpenMP][Flang] Fix atomic operations on complex types (#165366)
Fixes https://github.com/llvm/llvm-project/issues/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.
Added:
flang/test/Lower/OpenMP/atomic-read-complex.f90
flang/test/Lower/OpenMP/atomic-write-complex.f90
Modified:
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
Removed:
################################################################################
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..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.
@@ -9338,9 +9339,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 +9384,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);
@@ -9581,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 llvm-commits
mailing list