[flang-commits] [flang] [Flang][OpenMP] Implement "promotion" of use_device_ptr non-cptr arguments to use_device_addr (PR #82834)
via flang-commits
flang-commits at lists.llvm.org
Fri Feb 23 13:30:26 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
Author: None (agozillon)
<details>
<summary>Changes</summary>
This effectively implements some now deprecated OpenMP functionality that some applications (most notably at the moment GenASiS) unfortunately depend on (deprecated in specification version 5.2):
"If a list item in a use_device_ptr clause is not of type C_PTR, the behavior is as if the list item appeared in a use_device_addr clause. Support for such list items in a use_device_ptr clause is deprecated."
This PR downgrades the hard-error to a deprecated warning and "promotes" the above cases by simply moving the offending operands from the use_device_ptr value list to the back of the use_device_addr list (and moves the related symbols, locs and types that form the BlockArgs correspondingly) and then the generation of the target data construct proceeds as normal.
---
Full diff: https://github.com/llvm/llvm-project/pull/82834.diff
4 Files Affected:
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+66)
- (modified) flang/lib/Semantics/check-omp-structure.cpp (+1-1)
- (added) flang/test/Lower/OpenMP/use-device-ptr-to-use-device-addr.f90 (+72)
- (modified) flang/test/Semantics/OpenMP/use_device_ptr1.f90 (+1-1)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 7953bf83cba0fe..922831fa734c8c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -723,6 +723,58 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
/*task_reductions=*/nullptr, allocateOperands, allocatorOperands);
}
+// This helper function implements the functionality of "promoting"
+// non-CPTR arguments of use_device_ptr to use_device_addr
+// arguments (automagic conversion of use_device_ptr ->
+// use_device_addr in these cases). The way we do so currently is
+// through the shuffling of operands from the devicePtrOperands to
+// deviceAddrOperands where neccesary and re-organizing the types,
+// locations and symbols to maintain the correct ordering of ptr/addr
+// input -> BlockArg.
+//
+// This effectively implements some deprecated OpenMP functionality
+// that some legacy applications unfortunately depend on
+// (deprecated in specification version 5.2):
+//
+// "If a list item in a use_device_ptr clause is not of type C_PTR,
+// the behavior is as if the list item appeared in a use_device_addr
+// clause. Support for such list items in a use_device_ptr clause
+// is deprecated."
+static void promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(
+ llvm::SmallVector<mlir::Value> &devicePtrOperands,
+ llvm::SmallVector<mlir::Value> &deviceAddrOperands,
+ llvm::SmallVector<mlir::Type> &useDeviceTypes,
+ llvm::SmallVector<mlir::Location> &useDeviceLocs,
+ llvm::SmallVector<const Fortran::semantics::Symbol *> &useDeviceSymbols) {
+ auto moveElementToBack = [](size_t idx, auto &vector) {
+ auto *iter = std::next(vector.begin(), idx);
+ vector.push_back(*iter);
+ vector.erase(iter);
+ };
+
+ // Iterate over our use_device_ptr list and shift all non-cptr arguments into
+ // use_device_addr.
+ for (auto *it = devicePtrOperands.begin(); it != devicePtrOperands.end();) {
+ if (!fir::isa_builtin_cptr_type(fir::unwrapRefType(it->getType()))) {
+ deviceAddrOperands.push_back(*it);
+ // We have to shuffle the symbols around as well, to maintain
+ // the correct Input -> BlockArg for use_device_ptr/use_device_addr.
+ // NOTE: However, as map's do not seem to be included currently
+ // this isn't as pertinent, but we must try to maintain for
+ // future alterations. I believe the reason they are not currently
+ // is that the BlockArg assign/lowering needs to be extended
+ // to a greater set of types.
+ auto idx = std::distance(devicePtrOperands.begin(), it);
+ moveElementToBack(idx, useDeviceTypes);
+ moveElementToBack(idx, useDeviceLocs);
+ moveElementToBack(idx, useDeviceSymbols);
+ it = devicePtrOperands.erase(it);
+ continue;
+ }
+ ++it;
+ }
+}
+
static mlir::omp::DataOp
genDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
@@ -745,6 +797,20 @@ genDataOp(Fortran::lower::AbstractConverter &converter,
useDeviceSymbols);
cp.processUseDeviceAddr(deviceAddrOperands, useDeviceTypes, useDeviceLocs,
useDeviceSymbols);
+ // This function implements the deprecated functionality of use_device_ptr
+ // that allows users to provide non-CPTR arguments to it with the caveat
+ // that the compiler will treat them as use_device_addr. A lot of legacy
+ // code may still depend on this functionality, so we should support it
+ // in some manner. We do so currently by simply shifting non-cptr operands
+ // from the use_device_ptr list into the front of the use_device_addr list
+ // whilst maintaining the ordering of useDeviceLocs, useDeviceSymbols and
+ // useDeviceTypes to use_device_ptr/use_device_addr input for BlockArg
+ // ordering.
+ // TODO: Perhaps create a user provideable compiler option that will
+ // re-introduce a hard-error rather than a warning in these cases.
+ promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(
+ devicePtrOperands, deviceAddrOperands, useDeviceTypes, useDeviceLocs,
+ useDeviceSymbols);
cp.processMap(currentLocation, llvm::omp::Directive::OMPD_target_data,
stmtCtx, mapOperands);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 54101ab8a42bbf..bf4debee1df34c 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2948,7 +2948,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::UseDevicePtr &x) {
if (name->symbol) {
if (!(IsBuiltinCPtr(*(name->symbol)))) {
context_.Say(itr->second->source,
- "'%s' in USE_DEVICE_PTR clause must be of type C_PTR"_err_en_US,
+ "Use of non-C_PTR type '%s' in USE_DEVICE_PTR is deprecated, use USE_DEVICE_ADDR instead"_warn_en_US,
name->ToString());
} else {
useDevicePtrNameList.push_back(*name);
diff --git a/flang/test/Lower/OpenMP/use-device-ptr-to-use-device-addr.f90 b/flang/test/Lower/OpenMP/use-device-ptr-to-use-device-addr.f90
new file mode 100644
index 00000000000000..33b5971656010a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/use-device-ptr-to-use-device-addr.f90
@@ -0,0 +1,72 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+!RUN: bbc -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! This tests primary goal is to check the promotion of
+! non-CPTR arguments from use_device_ptr to
+! use_device_addr works, without breaking any
+! functionality
+
+!CHECK: func.func @{{.*}}only_use_device_ptr()
+!CHECK: omp.target_data use_device_ptr(%{{.*}} : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) use_device_addr(%{{.*}}, %{{.*}} : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) {
+!CHECK: ^bb0(%{{.*}}: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>):
+subroutine only_use_device_ptr
+ use iso_c_binding
+ integer, pointer, dimension(:) :: array
+ real, pointer :: pa(:)
+ type(c_ptr) :: cptr
+
+ !$omp target data use_device_ptr(pa, cptr, array)
+ !$omp end target data
+end subroutine
+
+!CHECK: func.func @{{.*}}mix_use_device_ptr_and_addr()
+!CHECK: omp.target_data use_device_ptr({{.*}} : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) use_device_addr(%{{.*}}, %{{.*}} : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) {
+!CHECK: ^bb0(%{{.*}}: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>):
+subroutine mix_use_device_ptr_and_addr
+ use iso_c_binding
+ integer, pointer, dimension(:) :: array
+ real, pointer :: pa(:)
+ type(c_ptr) :: cptr
+
+ !$omp target data use_device_ptr(pa, cptr) use_device_addr(array)
+ !$omp end target data
+end subroutine
+
+!CHECK: func.func @{{.*}}only_use_device_addr()
+!CHECK: omp.target_data use_device_addr(%{{.*}}, %{{.*}}, %{{.*}} : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) {
+!CHECK: ^bb0(%{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, %{{.*}}: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>):
+subroutine only_use_device_addr
+ use iso_c_binding
+ integer, pointer, dimension(:) :: array
+ real, pointer :: pa(:)
+ type(c_ptr) :: cptr
+
+ !$omp target data use_device_addr(pa, cptr, array)
+ !$omp end target data
+end subroutine
+
+!CHECK: func.func @{{.*}}mix_use_device_ptr_and_addr_and_map()
+!CHECK: omp.target_data map_entries(%{{.*}}, %{{.*}} : !fir.ref<i32>, !fir.ref<i32>) use_device_ptr(%{{.*}} : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) use_device_addr(%{{.*}}, %{{.*}} : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) {
+!CHECK: ^bb0(%{{.*}}: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, %{{.*}}: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>):
+subroutine mix_use_device_ptr_and_addr_and_map
+ use iso_c_binding
+ integer :: i, j
+ integer, pointer, dimension(:) :: array
+ real, pointer :: pa(:)
+ type(c_ptr) :: cptr
+
+ !$omp target data use_device_ptr(pa, cptr) use_device_addr(array) map(tofrom: i, j)
+ !$omp end target data
+end subroutine
+
+!CHECK: func.func @{{.*}}only_use_map()
+!CHECK: omp.target_data map_entries(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}} : !fir.llvm_ptr<!fir.ref<!fir.array<?xf32>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) {
+subroutine only_use_map
+ use iso_c_binding
+ integer, pointer, dimension(:) :: array
+ real, pointer :: pa(:)
+ type(c_ptr) :: cptr
+
+ !$omp target data map(pa, cptr, array)
+ !$omp end target data
+end subroutine
diff --git a/flang/test/Semantics/OpenMP/use_device_ptr1.f90 b/flang/test/Semantics/OpenMP/use_device_ptr1.f90
index af89698a5c5a99..176fb5f35a8490 100644
--- a/flang/test/Semantics/OpenMP/use_device_ptr1.f90
+++ b/flang/test/Semantics/OpenMP/use_device_ptr1.f90
@@ -27,7 +27,7 @@ subroutine omp_target_data
a = arrayB
!$omp end target data
- !ERROR: 'a' in USE_DEVICE_PTR clause must be of type C_PTR
+ !WARNING: Use of non-C_PTR type 'a' in USE_DEVICE_PTR is deprecated, use USE_DEVICE_ADDR instead
!$omp target data map(tofrom: a) use_device_ptr(a)
a = 2
!$omp end target data
``````````
</details>
https://github.com/llvm/llvm-project/pull/82834
More information about the flang-commits
mailing list