Revert "[Flang] [OpenMP] [Semantics] [MLIR] [Lowering] Add lowering support for IS_DEVICE_PTR and HAS_DEVICE_ADDR clauses on OMP TARGET directive." #88198
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…upport f…" This reverts commit 9d9560f.
llvmbot
added
mlir
flang
|
@llvm/pr-subscribers-mlir-openmp @llvm/pr-subscribers-mlir Author: Raghu Maddhipatla (raghavendhra) ChangesReverts llvm/llvm-project#74187 Full diff: https://github.com/llvm/llvm-project/pull/88198.diff 7 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index fb24c8d1fe3eb0..0a57a1496289f4 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -751,20 +751,6 @@ bool ClauseProcessor::processDepend(
});
}
-bool ClauseProcessor::processHasDeviceAddr(
- llvm::SmallVectorImpl<mlir::Value> &operands,
- llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
- llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &isDeviceSymbols)
- const {
- return findRepeatableClause<omp::clause::HasDeviceAddr>(
- [&](const omp::clause::HasDeviceAddr &devAddrClause,
- const Fortran::parser::CharBlock &) {
- addUseDeviceClause(converter, devAddrClause.v, operands, isDeviceTypes,
- isDeviceLocs, isDeviceSymbols);
- });
-}
-
bool ClauseProcessor::processIf(
omp::clause::If::DirectiveNameModifier directiveName,
mlir::Value &result) const {
@@ -785,20 +771,6 @@ bool ClauseProcessor::processIf(
return found;
}
-bool ClauseProcessor::processIsDevicePtr(
- llvm::SmallVectorImpl<mlir::Value> &operands,
- llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
- llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &isDeviceSymbols)
- const {
- return findRepeatableClause<omp::clause::IsDevicePtr>(
- [&](const omp::clause::IsDevicePtr &devPtrClause,
- const Fortran::parser::CharBlock &) {
- addUseDeviceClause(converter, devPtrClause.v, operands, isDeviceTypes,
- isDeviceLocs, isDeviceSymbols);
- });
-}
-
bool ClauseProcessor::processLink(
llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const {
return findRepeatableClause<omp::clause::Link>(
@@ -1021,7 +993,6 @@ bool ClauseProcessor::processUseDevicePtr(
useDeviceLocs, useDeviceSymbols);
});
}
-
} // namespace omp
} // namespace lower
} // namespace Fortran
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index df8f4f5310fcb6..d31d6a5c20623a 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -66,12 +66,6 @@ class ClauseProcessor {
bool processDeviceType(mlir::omp::DeclareTargetDeviceType &result) const;
bool processFinal(Fortran::lower::StatementContext &stmtCtx,
mlir::Value &result) const;
- bool
- processHasDeviceAddr(llvm::SmallVectorImpl<mlir::Value> &operands,
- llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
- llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
- &isDeviceSymbols) const;
bool processHint(mlir::IntegerAttr &result) const;
bool processMergeable(mlir::UnitAttr &result) const;
bool processNowait(mlir::UnitAttr &result) const;
@@ -110,12 +104,6 @@ class ClauseProcessor {
bool processIf(omp::clause::If::DirectiveNameModifier directiveName,
mlir::Value &result) const;
bool
- processIsDevicePtr(llvm::SmallVectorImpl<mlir::Value> &operands,
- llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
- llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
- &isDeviceSymbols) const;
- bool
processLink(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
// This method is used to process a map clause.
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 50ad889052ab05..340921c867246c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1294,11 +1294,6 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
llvm::SmallVector<mlir::Type> mapSymTypes;
llvm::SmallVector<mlir::Location> mapSymLocs;
llvm::SmallVector<const Fortran::semantics::Symbol *> mapSymbols;
- llvm::SmallVector<mlir::Value> devicePtrOperands, deviceAddrOperands;
- llvm::SmallVector<mlir::Type> devicePtrTypes, deviceAddrTypes;
- llvm::SmallVector<mlir::Location> devicePtrLocs, deviceAddrLocs;
- llvm::SmallVector<const Fortran::semantics::Symbol *> devicePtrSymbols,
- deviceAddrSymbols;
ClauseProcessor cp(converter, semaCtx, clauseList);
cp.processIf(llvm::omp::Directive::OMPD_target, ifClauseOperand);
@@ -1308,15 +1303,11 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
cp.processNowait(nowaitAttr);
cp.processMap(currentLocation, directive, stmtCtx, mapOperands, &mapSymTypes,
&mapSymLocs, &mapSymbols);
- cp.processIsDevicePtr(devicePtrOperands, devicePtrTypes, devicePtrLocs,
- devicePtrSymbols);
- cp.processHasDeviceAddr(deviceAddrOperands, deviceAddrTypes, deviceAddrLocs,
- deviceAddrSymbols);
- cp.processTODO<clause::Private, clause::Firstprivate, clause::Reduction,
- clause::InReduction, clause::Allocate, clause::UsesAllocators,
- clause::Defaultmap>(currentLocation,
- llvm::omp::Directive::OMPD_target);
+ cp.processTODO<clause::Private, clause::Firstprivate, clause::IsDevicePtr,
+ clause::HasDeviceAddr, clause::Reduction, clause::InReduction,
+ clause::Allocate, clause::UsesAllocators, clause::Defaultmap>(
+ currentLocation, llvm::omp::Directive::OMPD_target);
// 5.8.1 Implicit Data-Mapping Attribute Rules
// The following code follows the implicit data-mapping rules to map all the
@@ -1409,8 +1400,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
? nullptr
: mlir::ArrayAttr::get(converter.getFirOpBuilder().getContext(),
dependTypeOperands),
- dependOperands, nowaitAttr, devicePtrOperands, deviceAddrOperands,
- mapOperands);
+ dependOperands, nowaitAttr, mapOperands);
genBodyOfTargetOp(converter, semaCtx, eval, genNested, targetOp, mapSymTypes,
mapSymLocs, mapSymbols, currentLocation);
@@ -2069,8 +2059,6 @@ genOMP(Fortran::lower::AbstractConverter &converter,
!std::get_if<Fortran::parser::OmpClause::Map>(&clause.u) &&
!std::get_if<Fortran::parser::OmpClause::UseDevicePtr>(&clause.u) &&
!std::get_if<Fortran::parser::OmpClause::UseDeviceAddr>(&clause.u) &&
- !std::get_if<Fortran::parser::OmpClause::IsDevicePtr>(&clause.u) &&
- !std::get_if<Fortran::parser::OmpClause::HasDeviceAddr>(&clause.u) &&
!std::get_if<Fortran::parser::OmpClause::ThreadLimit>(&clause.u) &&
!std::get_if<Fortran::parser::OmpClause::NumTeams>(&clause.u)) {
TODO(clauseLocation, "OpenMP Block construct clause");
diff --git a/flang/test/Lower/OpenMP/FIR/target.f90 b/flang/test/Lower/OpenMP/FIR/target.f90
index 022327f9c25daf..821196b83c3b99 100644
--- a/flang/test/Lower/OpenMP/FIR/target.f90
+++ b/flang/test/Lower/OpenMP/FIR/target.f90
@@ -506,45 +506,4 @@ subroutine omp_target_parallel_do
!CHECK: omp.terminator
!CHECK: }
!$omp end target parallel do
-end subroutine omp_target_parallel_do
-
-!===============================================================================
-! Target `is_device_ptr` clause
-!===============================================================================
-
-!CHECK-LABEL: func.func @_QPomp_target_is_device_ptr() {
-subroutine omp_target_is_device_ptr
- use iso_c_binding, only : c_ptr, c_loc
- !CHECK: %[[VAL_0:.*]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}> {bindc_name = "a", uniq_name = "_QFomp_target_is_device_ptrEa"}
- type(c_ptr) :: a
- !CHECK: %[[VAL_1:.*]] = fir.alloca i32 {bindc_name = "b", fir.target, uniq_name = "_QFomp_target_is_device_ptrEb"}
- integer, target :: b
- !CHECK: %[[MAP_0:.*]] = omp.map.info var_ptr(%[[DEV_PTR:.*]] : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>) map_clauses(tofrom) capture(ByRef) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>> {name = "a"}
- !CHECK: %[[MAP_1:.*]] = omp.map.info var_ptr(%[[VAL_0:.*]] : !fir.ref<i32>, i32) map_clauses(tofrom) capture(ByRef) -> !fir.ref<i32> {name = "b"}
- !CHECK: %[[MAP_2:.*]] = omp.map.info var_ptr(%[[DEV_PTR:.*]] : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByRef) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>> {name = "a"}
- !CHECK: omp.target is_device_ptr(%[[DEV_PTR:.*]] : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) map_entries(%[[MAP_0:.*]] -> %[[ARG0:.*]], %[[MAP_1:.*]] -> %[[ARG1:.*]], %[[MAP_2:.*]] -> %[[ARG2:.*]] : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.ref<i32>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) {
- !CHECK: ^bb0(%[[ARG0]]: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, %[[ARG1]]: !fir.ref<i32>, %[[ARG2]]: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>):
- !$omp target map(tofrom: a,b) is_device_ptr(a)
- !CHECK: {{.*}} = fir.coordinate_of %[[VAL_0:.*]], {{.*}} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
- a = c_loc(b)
- !CHECK: omp.terminator
- !$omp end target
- !CHECK: }
-end subroutine omp_target_is_device_ptr
-
- !===============================================================================
- ! Target `has_device_addr` clause
- !===============================================================================
-
- !CHECK-LABEL: func.func @_QPomp_target_has_device_addr() {
- subroutine omp_target_has_device_addr
- !CHECK: %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "a", uniq_name = "_QFomp_target_has_device_addrEa"}
- integer, pointer :: a
- !CHECK: omp.target has_device_addr(%[[VAL_0:.*]] : !fir.ref<!fir.box<!fir.ptr<i32>>>) map_entries({{.*}} -> {{.*}}, {{.*}} -> {{.*}} : !fir.llvm_ptr<!fir.ref<i32>>, !fir.ref<!fir.box<!fir.ptr<i32>>>) {
- !$omp target has_device_addr(a)
- !CHECK: {{.*}} = fir.load %[[VAL_0:.*]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
- a = 10
- !CHECK: omp.terminator
- !$omp end target
- !CHECK: }
-end subroutine omp_target_has_device_addr
+ end subroutine omp_target_parallel_do
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 2a8582be383311..a38a82f9cc6073 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1678,23 +1678,14 @@ def TargetOp : OpenMP_Op<"target", [IsolatedFromAbove, MapClauseOwningOpInterfac
The optional $thread_limit specifies the limit on the number of threads
- The optional $nowait eliminates the implicit barrier so the parent task can make progress
+ The optional $nowait elliminates the implicit barrier so the parent task can make progress
even if the target task is not yet completed.
The `depends` and `depend_vars` arguments are variadic lists of values
that specify the dependencies of this particular target task in relation to
other tasks.
- The optional $is_device_ptr indicates list items are device pointers.
-
- The optional $has_device_addr indicates that list items already have device
- addresses, so they may be directly accessed from the target device. This
- includes array sections.
-
- The optional $map_operands maps data from the task’s environment to the
- device environment.
-
- TODO: defaultmap, in_reduction
+ TODO: is_device_ptr, defaultmap, in_reduction
}];
@@ -1704,9 +1695,8 @@ def TargetOp : OpenMP_Op<"target", [IsolatedFromAbove, MapClauseOwningOpInterfac
OptionalAttr<TaskDependArrayAttr>:$depends,
Variadic<OpenMP_PointerLikeType>:$depend_vars,
UnitAttr:$nowait,
- Variadic<OpenMP_PointerLikeType>:$is_device_ptr,
- Variadic<OpenMP_PointerLikeType>:$has_device_addr,
Variadic<AnyType>:$map_operands);
+
let regions = (region AnyRegion:$region);
let builders = [
@@ -1718,8 +1708,6 @@ def TargetOp : OpenMP_Op<"target", [IsolatedFromAbove, MapClauseOwningOpInterfac
| `device` `(` $device `:` type($device) `)`
| `thread_limit` `(` $thread_limit `:` type($thread_limit) `)`
| `nowait` $nowait
- | `is_device_ptr` `(` $is_device_ptr `:` type($is_device_ptr) `)`
- | `has_device_addr` `(` $has_device_addr `:` type($has_device_addr) `)`
| `map_entries` `(` custom<MapEntries>($map_operands, type($map_operands)) `)`
| `depend` `(` custom<DependVarList>($depend_vars, type($depend_vars), $depends) `)`
) $region attr-dict
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 27a440b3f97ca8..1134db77d5baa8 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -1809,7 +1809,7 @@ func.func @omp_target_depend(%data_var: memref<i32>) {
// expected-error @below {{op expected as many depend values as depend variables}}
"omp.target"(%data_var) ({
"omp.terminator"() : () -> ()
- }) {depends = [], operandSegmentSizes = array<i32: 0, 0, 0, 1, 0, 0, 0>} : (memref<i32>) -> ()
+ }) {depends = [], operandSegmentSizes = array<i32: 0, 0, 0, 1, 0>} : (memref<i32>) -> ()
"func.return"() : () -> ()
}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index ad5b74b84ac701..e2c255c7a3ccc3 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -510,22 +510,22 @@ return
// CHECK-LABEL: omp_target
-func.func @omp_target(%if_cond : i1, %device : si32, %num_threads : i32, %device_ptr: memref<i32>, %device_addr: memref<?xi32>, %map1: memref<?xi32>, %map2: memref<?xi32>) -> () {
+func.func @omp_target(%if_cond : i1, %device : si32, %num_threads : i32, %map1: memref<?xi32>, %map2: memref<?xi32>) -> () {
// Test with optional operands; if_expr, device, thread_limit, private, firstprivate and nowait.
// CHECK: omp.target if({{.*}}) device({{.*}}) thread_limit({{.*}}) nowait
"omp.target"(%if_cond, %device, %num_threads) ({
// CHECK: omp.terminator
omp.terminator
- }) {nowait, operandSegmentSizes = array<i32: 1,1,1,0,0,0,0>} : ( i1, si32, i32 ) -> ()
+ }) {nowait, operandSegmentSizes = array<i32: 1,1,1,0,0>} : ( i1, si32, i32 ) -> ()
// Test with optional map clause.
// CHECK: %[[MAP_A:.*]] = omp.map.info var_ptr(%[[VAL_1:.*]] : memref<?xi32>, tensor<?xi32>) map_clauses(tofrom) capture(ByRef) -> memref<?xi32> {name = ""}
// CHECK: %[[MAP_B:.*]] = omp.map.info var_ptr(%[[VAL_2:.*]] : memref<?xi32>, tensor<?xi32>) map_clauses(exit_release_or_enter_alloc) capture(ByRef) -> memref<?xi32> {name = ""}
- // CHECK: omp.target is_device_ptr(%[[VAL_4:.*]] : memref<i32>) has_device_addr(%[[VAL_5:.*]] : memref<?xi32>) map_entries(%[[MAP_A]] -> {{.*}}, %[[MAP_B]] -> {{.*}} : memref<?xi32>, memref<?xi32>) {
+ // CHECK: omp.target map_entries(%[[MAP_A]] -> {{.*}}, %[[MAP_B]] -> {{.*}} : memref<?xi32>, memref<?xi32>) {
%mapv1 = omp.map.info var_ptr(%map1 : memref<?xi32>, tensor<?xi32>) map_clauses(tofrom) capture(ByRef) -> memref<?xi32> {name = ""}
%mapv2 = omp.map.info var_ptr(%map2 : memref<?xi32>, tensor<?xi32>) map_clauses(exit_release_or_enter_alloc) capture(ByRef) -> memref<?xi32> {name = ""}
- omp.target map_entries(%mapv1 -> %arg0, %mapv2 -> %arg1 : memref<?xi32>, memref<?xi32>) is_device_ptr(%device_ptr : memref<i32>) has_device_addr(%device_addr : memref<?xi32>) {
+ omp.target map_entries(%mapv1 -> %arg0, %mapv2 -> %arg1 : memref<?xi32>, memref<?xi32>) {
^bb0(%arg0: memref<?xi32>, %arg1: memref<?xi32>):
omp.terminator
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #74187