[flang] [llvm] [flang][OpenMP] Map ByRef if size/alignment exceed that of a pointer (PR #130832)
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 12 08:31:41 PDT 2025
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/130832
>From 5f87ce59ec36c2f401ce492830864371b9479255 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 11 Mar 2025 14:57:22 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Map ByRef if size/alignment exceed that
of a pointer
Improve the check for whether a type can be passed by copy. Currently,
passing by copy is done via the OMP_MAP_LITERAL mapping, which can only
transfer as much data as can be contained in a pointer representation.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 20 ++++++++++-
.../test/Lower/OpenMP/target-map-complex.f90 | 33 +++++++++++++++++++
2 files changed, 52 insertions(+), 1 deletion(-)
create mode 100644 flang/test/Lower/OpenMP/target-map-complex.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 77786742b3e13..651ed3055f203 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2192,6 +2192,24 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
/*useDelayedPrivatization=*/true, symTable);
dsp.processStep1(&clauseOps);
+ // Check if a value of type `type` be passed to the kernel by value.
+ // All kernel parameters are of pointer type, so if the value can be
+ // represented inside of a pointer, then it can be passed by value.
+ auto isLiteralType = [&](mlir::Type type) {
+ if (!fir::isa_trivial(type) && !fir::isa_char(type))
+ return false;
+
+ const mlir::DataLayout &dl = firOpBuilder.getDataLayout();
+ mlir::Type ptrTy =
+ mlir::LLVM::LLVMPointerType::get(&converter.getMLIRContext());
+ uint64_t ptrSize = dl.getTypeSize(ptrTy);
+ uint64_t ptrAlign = dl.getTypePreferredAlignment(ptrTy);
+
+ auto [size, align] = fir::getTypeSizeAndAlignmentOrCrash(
+ loc, type, dl, converter.getKindMap());
+ return size <= ptrSize && align <= ptrAlign;
+ };
+
// 5.8.1 Implicit Data-Mapping Attribute Rules
// The following code follows the implicit data-mapping rules to map all the
// symbols used inside the region that do not have explicit data-environment
@@ -2268,7 +2286,7 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
}
- } else if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) {
+ } else if (isLiteralType(eleType)) {
captureKind = mlir::omp::VariableCaptureKind::ByCopy;
} else if (!fir::isa_builtin_cptr_type(eleType)) {
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
diff --git a/flang/test/Lower/OpenMP/target-map-complex.f90 b/flang/test/Lower/OpenMP/target-map-complex.f90
new file mode 100644
index 0000000000000..c15a10e4804dd
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-map-complex.f90
@@ -0,0 +1,33 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! Check that the complex*4 is passed by value. but complex*8 is passed by
+! reference
+
+!CHECK-LABEL: func.func @_QMmPbar()
+!CHECK: %[[V0:[0-9]+]]:2 = hlfir.declare {{.*}} (!fir.ref<complex<f64>>) -> (!fir.ref<complex<f64>>, !fir.ref<complex<f64>>)
+!CHECK: %[[V1:[0-9]+]]:2 = hlfir.declare {{.*}} (!fir.ref<complex<f32>>) -> (!fir.ref<complex<f32>>, !fir.ref<complex<f32>>)
+!CHECK: %[[V2:[0-9]+]] = omp.map.info var_ptr(%[[V1]]#1 : !fir.ref<complex<f32>>, complex<f32>) {{.*}} capture(ByCopy)
+!CHECK: %[[V3:[0-9]+]] = omp.map.info var_ptr(%[[V0]]#1 : !fir.ref<complex<f64>>, complex<f64>) {{.*}} capture(ByRef)
+!CHECK: omp.target map_entries(%[[V2]] -> {{.*}}, %[[V3]] -> {{.*}} : !fir.ref<complex<f32>>, !fir.ref<complex<f64>>)
+
+module m
+ implicit none
+ complex(kind=4) :: cfval = (24, 25)
+ complex(kind=8) :: cdval = (28, 29)
+ interface
+ subroutine foo(x, y)
+ complex(kind=4) :: x
+ complex(kind=8) :: y
+ !$omp declare target
+ end
+ end interface
+
+contains
+
+subroutine bar()
+!$omp target
+ call foo(cfval, cdval)
+!$omp end target
+end
+
+end module
>From e2d00bf00982c9bda6169a5409408ecf69cc972e Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 11 Mar 2025 14:57:22 -0500
Subject: [PATCH 2/2] Use TO for trivial types that are too large to be LITERAL
Make sure they are not TOFROM.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 16 +++++++++------
.../fortran/target-map-literal-write.f90 | 20 +++++++++++++++++++
2 files changed, 30 insertions(+), 6 deletions(-)
create mode 100644 offload/test/offloading/fortran/target-map-literal-write.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 651ed3055f203..2cfc1bd88dcef 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2192,13 +2192,10 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
/*useDelayedPrivatization=*/true, symTable);
dsp.processStep1(&clauseOps);
- // Check if a value of type `type` be passed to the kernel by value.
+ // Check if a value of type `type` can be passed to the kernel by value.
// All kernel parameters are of pointer type, so if the value can be
// represented inside of a pointer, then it can be passed by value.
auto isLiteralType = [&](mlir::Type type) {
- if (!fir::isa_trivial(type) && !fir::isa_char(type))
- return false;
-
const mlir::DataLayout &dl = firOpBuilder.getDataLayout();
mlir::Type ptrTy =
mlir::LLVM::LLVMPointerType::get(&converter.getMLIRContext());
@@ -2286,8 +2283,15 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
}
- } else if (isLiteralType(eleType)) {
- captureKind = mlir::omp::VariableCaptureKind::ByCopy;
+ } else if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) {
+ // Scalars behave as if they were "firstprivate".
+ // TODO: Handle objects that are shared/lastprivate or were listed
+ // in an in_reduction clause.
+ if (isLiteralType(eleType)) {
+ captureKind = mlir::omp::VariableCaptureKind::ByCopy;
+ } else {
+ mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
+ }
} else if (!fir::isa_builtin_cptr_type(eleType)) {
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
diff --git a/offload/test/offloading/fortran/target-map-literal-write.f90 b/offload/test/offloading/fortran/target-map-literal-write.f90
new file mode 100644
index 0000000000000..8f0cb95338eed
--- /dev/null
+++ b/offload/test/offloading/fortran/target-map-literal-write.f90
@@ -0,0 +1,20 @@
+!REQUIRES: flang, amdgpu
+
+!RUN: %libomptarget-compile-fortran-run-and-check-generic
+
+program m
+ complex(kind=8) :: x
+ x = (1.0, 2.0)
+!$omp target
+ x = (-1.0, -2.0)
+!$omp end target
+ print *, "x=", x
+end program
+
+! The host variable "x" should be passed to the kernel as "firstprivate",
+! hence the kernel should have its own copy of it. This is in contrast to
+! other cases where implicitly mapped variables have the TOFROM map-type.
+
+! Make sure that the target region didn't overwrite the host variable.
+
+!CHECK: x= (1.,2.)
More information about the llvm-commits
mailing list