Skip to content

Commit e536d92

Browse files
Krish GuptaKrish Gupta
Krish Gupta
authored and
Krish Gupta
committed
[OpenMP][Flang] Fix atomic operations on complex types
Fixes #165184 Use element type instead of pointer type when computing size for __atomic_load/__atomic_store on struct types (including Fortran complex). This ensures the full struct is transferred, not just pointer-sized bytes.
1 parent 5cef6f3 commit e536d92

File tree

4 files changed

+124
-6
lines changed

4 files changed

+124
-6
lines changed

‎flang/test/Integration/OpenMP/atomic-read-complex.f90‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
!===----------------------------------------------------------------------===!
2+
! This directory can be used to add Integration tests involving multiple
3+
! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
4+
! contain executable tests. We should only add tests here sparingly and only
5+
! if there is no other way to test. Repeat this message in each test that is
6+
! added to this directory and sub-directories.
7+
!===----------------------------------------------------------------------===!
8+
9+
! REQUIRES: x86-registered-target || aarch64-registered-target
10+
11+
! RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
12+
! RUN: %flang_fc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
13+
14+
! Test that atomic read operations with complex types emit the correct
15+
! size to __atomic_load. This is a regression test for issue #165184.
16+
!
17+
! For complex(4) (8 bytes total): should call __atomic_load(i64 8, ...)
18+
! For complex(8) (16 bytes total): should call __atomic_load(i64 16, ...)
19+
! For complex(16) (32 bytes total): should call __atomic_load(i64 32, ...)
20+
21+
program atomic_read_complex
22+
implicit none
23+
24+
! Test complex(4) - single precision (8 bytes)
25+
complex(4) :: c41, c42
26+
! Test complex(8) - double precision (16 bytes)
27+
complex(8) :: c81, c82
28+
29+
c42 = (1.0_4, 1.0_4)
30+
c82 = (1.0_8, 1.0_8)
31+
32+
! CHECK-LABEL: define {{.*}} @_QQmain
33+
34+
! Single precision complex: 8 bytes
35+
! CHECK: call void @__atomic_load(i64 8, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
36+
!$omp atomic read
37+
c41 = c42
38+
39+
! Double precision complex: 16 bytes (this was broken before the fix)
40+
! CHECK: call void @__atomic_load(i64 16, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
41+
!$omp atomic read
42+
c81 = c82
43+
44+
end program atomic_read_complex

‎flang/test/Integration/OpenMP/atomic-write-complex.f90‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
!===----------------------------------------------------------------------===!
2+
! This directory can be used to add Integration tests involving multiple
3+
! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
4+
! contain executable tests. We should only add tests here sparingly and only
5+
! if there is no other way to test. Repeat this message in each test that is
6+
! added to this directory and sub-directories.
7+
!===----------------------------------------------------------------------===!
8+
9+
! REQUIRES: x86-registered-target || aarch64-registered-target
10+
11+
! RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
12+
! RUN: %flang_fc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fopenmp %s -o - | FileCheck %s
13+
14+
! Test that atomic write operations with complex types emit the correct
15+
! size to __atomic_store. This is a regression test for issue #165184.
16+
!
17+
! For complex(4) (8 bytes total): should call __atomic_store(i64 8, ...)
18+
! For complex(8) (16 bytes total): should call __atomic_store(i64 16, ...)
19+
! For complex(16) (32 bytes total): should call __atomic_store(i64 32, ...)
20+
21+
program atomic_write_complex
22+
implicit none
23+
24+
! Test complex(4) - single precision (8 bytes)
25+
complex(4) :: c41, c42
26+
! Test complex(8) - double precision (16 bytes)
27+
complex(8) :: c81, c82
28+
29+
c42 = (1.0_4, 1.0_4)
30+
c82 = (1.0_8, 1.0_8)
31+
32+
! CHECK-LABEL: define {{.*}} @_QQmain
33+
34+
! Single precision complex: 8 bytes
35+
! CHECK: call void @__atomic_store(i64 8, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
36+
!$omp atomic write
37+
c41 = c42
38+
39+
! Double precision complex: 16 bytes (this was broken before the fix)
40+
! CHECK: call void @__atomic_store(i64 16, ptr {{.*}}, ptr {{.*}}, i32 {{.*}})
41+
!$omp atomic write
42+
c81 = c82
43+
44+
end program atomic_write_complex

‎flang/test/Integration/OpenMP/issue-165184-atomic-complex8.f90‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
! RUN: %flang_fc1 -emit-llvm -fopenmp -o - %s | FileCheck %s
2+
! RUN: %flang -fopenmp %s -o %t && %t | FileCheck %s --check-prefix=EXEC
3+
4+
! Regression test for issue #165184:
5+
! Atomic write to complex(8) was only storing 8 bytes instead of 16,
6+
! causing the imaginary part to remain 0.
7+
8+
program test_atomic_write_complex8
9+
implicit none
10+
complex(8) :: c81, c82
11+
12+
c81 = (0.0_8, 0.0_8)
13+
c82 = (0.0_8, 0.0_8)
14+
15+
! Verify the fix: __atomic_store should be called with size 16
16+
! CHECK: call void @__atomic_store(i64 16,
17+
18+
!$omp parallel
19+
!$omp atomic write
20+
c81 = c82 + (1.0_8, 1.0_8)
21+
!$omp end parallel
22+
23+
! EXEC: c81 = (1.00000000000000,1.00000000000000)
24+
write(*,*) "c81 = ", c81
25+
26+
! Verify both parts are correct
27+
if (real(c81) /= 1.0_8 .or. aimag(c81) /= 1.0_8) then
28+
write(*,*) "ERROR: Expected (1.0,1.0) but got", c81
29+
stop 1
30+
end if
31+
32+
end program test_atomic_write_complex8

‎llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9338,9 +9338,8 @@ OpenMPIRBuilder::createAtomicRead(const LocationDescription &Loc,
93389338
// target does not support `atomicrmw` of the size of the struct
93399339
LoadInst *OldVal = Builder.CreateLoad(XElemTy, X.Var, "omp.atomic.read");
93409340
OldVal->setAtomic(AO);
9341-
const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
9342-
unsigned LoadSize =
9343-
LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
9341+
const DataLayout &DL = OldVal->getModule()->getDataLayout();
9342+
unsigned LoadSize = DL.getTypeStoreSize(XElemTy);
93449343
OpenMPIRBuilder::AtomicInfo atomicInfo(
93459344
&Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
93469345
OldVal->getAlign(), true /* UseLibcall */, AllocaIP, X.Var);
@@ -9384,9 +9383,8 @@ OpenMPIRBuilder::createAtomicWrite(const LocationDescription &Loc,
93849383
XSt->setAtomic(AO);
93859384
} else if (XElemTy->isStructTy()) {
93869385
LoadInst *OldVal = Builder.CreateLoad(XElemTy, X.Var, "omp.atomic.read");
9387-
const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
9388-
unsigned LoadSize =
9389-
LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
9386+
const DataLayout &DL = OldVal->getModule()->getDataLayout();
9387+
unsigned LoadSize = DL.getTypeStoreSize(XElemTy);
93909388
OpenMPIRBuilder::AtomicInfo atomicInfo(
93919389
&Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
93929390
OldVal->getAlign(), true /* UseLibcall */, AllocaIP, X.Var);

0 commit comments

Comments
 (0)