Revert "[Flang][OpenMP] Add lowering support for is_device_ptr clause" (#170778)

Reverts llvm/llvm-project#169331

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

@@ -44,6 +44,15 @@ mlir::omp::ReductionModifier translateReductionModifier(ReductionModifier mod) {
   return mlir::omp::ReductionModifier::defaultmod;
 }
 
+/// Check for unsupported map operand types.
+static void checkMapType(mlir::Location location, mlir::Type type) {
+  if (auto refType = mlir::dyn_cast<fir::ReferenceType>(type))
+    type = refType.getElementType();
+  if (auto boxType = mlir::dyn_cast_or_null<fir::BoxType>(type))
+    if (!mlir::isa<fir::PointerType>(boxType.getElementType()))
+      TODO(location, "OMPD_target_data MapOperand BoxType");
+}
+
 static mlir::omp::ScheduleModifier
 translateScheduleModifier(const omp::clause::Schedule::OrderingModifier &m) {
   switch (m) {
@@ -202,6 +211,18 @@ getIfClauseOperand(lower::AbstractConverter &converter,
                                     ifVal);
 }
 
+static void addUseDeviceClause(
+    lower::AbstractConverter &converter, const omp::ObjectList &objects,
+    llvm::SmallVectorImpl<mlir::Value> &operands,
+    llvm::SmallVectorImpl<const semantics::Symbol *> &useDeviceSyms) {
+  genObjectList(objects, converter, operands);
+  for (mlir::Value &operand : operands)
+    checkMapType(operand.getLoc(), operand.getType());
+
+  for (const omp::Object &object : objects)
+    useDeviceSyms.push_back(object.sym());
+}
+
 //===----------------------------------------------------------------------===//
 // ClauseProcessor unique clauses
 //===----------------------------------------------------------------------===//
@@ -1204,26 +1225,14 @@ bool ClauseProcessor::processInReduction(
 }
 
 bool ClauseProcessor::processIsDevicePtr(
-    lower::StatementContext &stmtCtx, mlir::omp::IsDevicePtrClauseOps &result,
+    mlir::omp::IsDevicePtrClauseOps &result,
     llvm::SmallVectorImpl<const semantics::Symbol *> &isDeviceSyms) const {
-  std::map<Object, OmpMapParentAndMemberData> parentMemberIndices;
-  bool clauseFound = findRepeatableClause<omp::clause::IsDevicePtr>(
-      [&](const omp::clause::IsDevicePtr &clause,
-          const parser::CharBlock &source) {
-        mlir::Location location = converter.genLocation(source);
-        // Force a map so the descriptor is materialized on the device with the
-        // device address inside.
-        mlir::omp::ClauseMapFlags mapTypeBits =
-            mlir::omp::ClauseMapFlags::is_device_ptr |
-            mlir::omp::ClauseMapFlags::to;
-        processMapObjects(stmtCtx, location, clause.v, mapTypeBits,
-                          parentMemberIndices, result.isDevicePtrVars,
-                          isDeviceSyms);
+  return findRepeatableClause<omp::clause::IsDevicePtr>(
+      [&](const omp::clause::IsDevicePtr &devPtrClause,
+          const parser::CharBlock &) {
+        addUseDeviceClause(converter, devPtrClause.v, result.isDevicePtrVars,
+                           isDeviceSyms);
       });
-
-  insertChildMapInfoIntoParent(converter, semaCtx, stmtCtx, parentMemberIndices,
-                               result.isDevicePtrVars, isDeviceSyms);
-  return clauseFound;
 }
 
 bool ClauseProcessor::processLinear(mlir::omp::LinearClauseOps &result) const {

flang/lib/Lower/OpenMP/ClauseProcessor.h

@@ -134,7 +134,7 @@ public:
       mlir::Location currentLocation, mlir::omp::InReductionClauseOps &result,
       llvm::SmallVectorImpl<const semantics::Symbol *> &outReductionSyms) const;
   bool processIsDevicePtr(
-      lower::StatementContext &stmtCtx, mlir::omp::IsDevicePtrClauseOps &result,
+      mlir::omp::IsDevicePtrClauseOps &result,
       llvm::SmallVectorImpl<const semantics::Symbol *> &isDeviceSyms) const;
   bool processLinear(mlir::omp::LinearClauseOps &result) const;
   bool

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -1672,7 +1672,7 @@ static void genTargetClauses(
     hostEvalInfo->collectValues(clauseOps.hostEvalVars);
   }
   cp.processIf(llvm::omp::Directive::OMPD_target, clauseOps);
-  cp.processIsDevicePtr(stmtCtx, clauseOps, isDevicePtrSyms);
+  cp.processIsDevicePtr(clauseOps, isDevicePtrSyms);
   cp.processMap(loc, stmtCtx, clauseOps, llvm::omp::Directive::OMPD_unknown,
                 &mapSyms);
   cp.processNowait(clauseOps);
@@ -2488,15 +2488,13 @@ static bool isDuplicateMappedSymbol(
     const semantics::Symbol &sym,
     const llvm::SetVector<const semantics::Symbol *> &privatizedSyms,
     const llvm::SmallVectorImpl<const semantics::Symbol *> &hasDevSyms,
-    const llvm::SmallVectorImpl<const semantics::Symbol *> &mappedSyms,
-    const llvm::SmallVectorImpl<const semantics::Symbol *> &isDevicePtrSyms) {
+    const llvm::SmallVectorImpl<const semantics::Symbol *> &mappedSyms) {
   llvm::SmallVector<const semantics::Symbol *> concatSyms;
   concatSyms.reserve(privatizedSyms.size() + hasDevSyms.size() +
-                     mappedSyms.size() + isDevicePtrSyms.size());
+                     mappedSyms.size());
   concatSyms.append(privatizedSyms.begin(), privatizedSyms.end());
   concatSyms.append(hasDevSyms.begin(), hasDevSyms.end());
   concatSyms.append(mappedSyms.begin(), mappedSyms.end());
-  concatSyms.append(isDevicePtrSyms.begin(), isDevicePtrSyms.end());
 
   auto checkSymbol = [&](const semantics::Symbol &checkSym) {
     return std::any_of(concatSyms.begin(), concatSyms.end(),
@@ -2536,38 +2534,6 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
                    loc, clauseOps, defaultMaps, hasDeviceAddrSyms,
                    isDevicePtrSyms, mapSyms);
 
-  if (!isDevicePtrSyms.empty()) {
-    // is_device_ptr maps get duplicated so the clause and synthesized
-    // has_device_addr entry each own a unique MapInfoOp user, keeping
-    // MapInfoFinalization happy while still wiring the symbol into
-    // has_device_addr when the user didn’t spell it explicitly.
-    auto insertionPt = firOpBuilder.saveInsertionPoint();
-    auto alreadyPresent = [&](const semantics::Symbol *sym) {
-      return llvm::any_of(hasDeviceAddrSyms, [&](const semantics::Symbol *s) {
-        return s && sym && s->GetUltimate() == sym->GetUltimate();
-      });
-    };
-
-    for (auto [idx, sym] : llvm::enumerate(isDevicePtrSyms)) {
-      mlir::Value mapVal = clauseOps.isDevicePtrVars[idx];
-      assert(sym && "expected symbol for is_device_ptr");
-      assert(mapVal && "expected map value for is_device_ptr");
-      auto mapInfo = mapVal.getDefiningOp<mlir::omp::MapInfoOp>();
-      assert(mapInfo && "expected map info op");
-
-      if (!alreadyPresent(sym)) {
-        clauseOps.hasDeviceAddrVars.push_back(mapVal);
-        hasDeviceAddrSyms.push_back(sym);
-      }
-
-      firOpBuilder.setInsertionPointAfter(mapInfo);
-      mlir::Operation *clonedOp = firOpBuilder.clone(*mapInfo.getOperation());
-      auto clonedMapInfo = mlir::cast<mlir::omp::MapInfoOp>(clonedOp);
-      clauseOps.isDevicePtrVars[idx] = clonedMapInfo.getResult();
-    }
-    firOpBuilder.restoreInsertionPoint(insertionPt);
-  }
-
   DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
                            /*shouldCollectPreDeterminedSymbols=*/
                            lower::omp::isLastItemInQueue(item, queue),
@@ -2607,7 +2573,7 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       return;
 
     if (!isDuplicateMappedSymbol(sym, dsp.getAllSymbolsToPrivatize(),
-                                 hasDeviceAddrSyms, mapSyms, isDevicePtrSyms)) {
+                                 hasDeviceAddrSyms, mapSyms)) {
       if (const auto *details =
               sym.template detailsIf<semantics::HostAssocDetails>())
         converter.copySymbolBinding(details->symbol(), sym);

flang/test/Integration/OpenMP/map-types-and-sizes.f90

@@ -33,15 +33,6 @@ subroutine mapType_array
   !$omp end target
 end subroutine mapType_array
 
-!CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [1 x i64] [i64 8]
-!CHECK: @.offload_maptypes{{.*}} = private unnamed_addr constant [1 x i64] [i64 33]
-subroutine mapType_is_device_ptr
-  use iso_c_binding, only : c_ptr
-  type(c_ptr) :: p
-  !$omp target is_device_ptr(p)
-  !$omp end target
-end subroutine mapType_is_device_ptr
-
 !CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [5 x i64] [i64 0, i64 0, i64 0, i64 8, i64 0]
 !CHECK: @.offload_maptypes{{.*}} = private unnamed_addr constant [5 x i64] [i64 32, i64 281474976711173, i64 281474976711173, i64 281474976711171, i64 281474976711187]
 subroutine mapType_ptr

flang/test/Lower/OpenMP/target.f90

@@ -566,36 +566,6 @@ subroutine omp_target_device_addr
 end subroutine omp_target_device_addr
 
 
-!===============================================================================
-! 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
-   implicit none
-   integer :: i
-   integer :: arr(4)
-   type(c_ptr) :: p
-
-   i = 0
-   arr = 0
-
-   !CHECK: %[[P_STORAGE:.*]] = omp.map.info {{.*}}{name = "p"}
-   !CHECK: %[[P_IS:.*]] = omp.map.info {{.*}}{name = "p"}
-   !CHECK: %[[ARR_MAP:.*]] = omp.map.info {{.*}}{name = "arr"}
-   !CHECK: omp.target is_device_ptr(%[[P_IS]] :
-   !CHECK-SAME: has_device_addr(%[[P_STORAGE]] ->
-   !CHECK-SAME: map_entries({{.*}}%[[ARR_MAP]] ->
-   !$omp target is_device_ptr(p)
-      i = i + 1
-      arr(1) = i
-   !$omp end target
-   !CHECK: omp.terminator
-   !CHECK: }
-end subroutine omp_target_is_device_ptr
-
-
 !===============================================================================
 ! Target Data with unstructured code
 !===============================================================================

mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td

@@ -128,7 +128,6 @@ def ClauseMapFlagsAttachAuto : I32BitEnumAttrCaseBit<"attach_auto", 15>;
 def ClauseMapFlagsRefPtr : I32BitEnumAttrCaseBit<"ref_ptr", 16>;
 def ClauseMapFlagsRefPtee : I32BitEnumAttrCaseBit<"ref_ptee", 17>;
 def ClauseMapFlagsRefPtrPtee : I32BitEnumAttrCaseBit<"ref_ptr_ptee", 18>;
-def ClauseMapFlagsIsDevicePtr : I32BitEnumAttrCaseBit<"is_device_ptr", 19>;
 
 def ClauseMapFlags : OpenMP_BitEnumAttr<
     "ClauseMapFlags",
@@ -152,8 +151,7 @@ def ClauseMapFlags : OpenMP_BitEnumAttr<
       ClauseMapFlagsAttachAuto,
       ClauseMapFlagsRefPtr,
       ClauseMapFlagsRefPtee,
-      ClauseMapFlagsRefPtrPtee,
-      ClauseMapFlagsIsDevicePtr
+      ClauseMapFlagsRefPtrPtee
     ]>;
 
 def ClauseMapFlagsAttr : OpenMP_EnumAttr<ClauseMapFlags,

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

@@ -1818,9 +1818,6 @@ static ParseResult parseMapClause(OpAsmParser &parser,
     if (mapTypeMod == "ref_ptr_ptee")
       mapTypeBits |= ClauseMapFlags::ref_ptr_ptee;
 
-    if (mapTypeMod == "is_device_ptr")
-      mapTypeBits |= ClauseMapFlags::is_device_ptr;
-
     return success();
   };
 
@@ -1890,8 +1887,6 @@ static void printMapClause(OpAsmPrinter &p, Operation *op,
     mapTypeStrs.push_back("ref_ptee");
   if (mapTypeToBool(mapFlags, ClauseMapFlags::ref_ptr_ptee))
     mapTypeStrs.push_back("ref_ptr_ptee");
-  if (mapTypeToBool(mapFlags, ClauseMapFlags::is_device_ptr))
-    mapTypeStrs.push_back("is_device_ptr");
   if (mapFlags == ClauseMapFlags::none)
     mapTypeStrs.push_back("none");
 

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

@@ -330,6 +330,10 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         op.getInReductionSyms())
       result = todo("in_reduction");
   };
+  auto checkIsDevicePtr = [&todo](auto op, LogicalResult &result) {
+    if (!op.getIsDevicePtrVars().empty())
+      result = todo("is_device_ptr");
+  };
   auto checkLinear = [&todo](auto op, LogicalResult &result) {
     if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
       result = todo("linear");
@@ -437,6 +441,7 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkBare(op, result);
         checkDevice(op, result);
         checkInReduction(op, result);
+        checkIsDevicePtr(op, result);
       })
       .Default([](Operation &) {
         // Assume all clauses for an operation can be translated unless they are
@@ -3954,9 +3959,6 @@ convertClauseMapFlags(omp::ClauseMapFlags mlirFlags) {
   auto mapTypeToBool = [&mlirFlags](omp::ClauseMapFlags flag) {
     return (mlirFlags & flag) == flag;
   };
-  const bool hasExplicitMap =
-      (mlirFlags & ~omp::ClauseMapFlags::is_device_ptr) !=
-      omp::ClauseMapFlags::none;
 
   llvm::omp::OpenMPOffloadMappingFlags mapType =
       llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE;
@@ -3997,12 +3999,6 @@ convertClauseMapFlags(omp::ClauseMapFlags mlirFlags) {
   if (mapTypeToBool(omp::ClauseMapFlags::attach))
     mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH;
 
-  if (mapTypeToBool(omp::ClauseMapFlags::is_device_ptr)) {
-    mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM;
-    if (!hasExplicitMap)
-      mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL;
-  }
-
   return mapType;
 }
 
@@ -4126,9 +4122,6 @@ static void collectMapDataFromMapOperands(
     llvm::Value *origValue = moduleTranslation.lookupValue(offloadPtr);
     auto mapType = convertClauseMapFlags(mapOp.getMapType());
     auto mapTypeAlways = llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS;
-    bool isDevicePtr =
-        (mapOp.getMapType() & omp::ClauseMapFlags::is_device_ptr) !=
-        omp::ClauseMapFlags::none;
 
     mapData.OriginalValue.push_back(origValue);
     mapData.BasePointers.push_back(origValue);
@@ -4155,18 +4148,14 @@ static void collectMapDataFromMapOperands(
         mapData.Mappers.push_back(nullptr);
       }
     } else {
-      // For is_device_ptr we need the map type to propagate so the runtime
-      // can materialize the device-side copy of the pointer container.
       mapData.Types.push_back(
-          isDevicePtr ? mapType
-                      : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL);
+          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL);
       mapData.Mappers.push_back(nullptr);
     }
     mapData.Names.push_back(LLVM::createMappingInformation(
         mapOp.getLoc(), *moduleTranslation.getOpenMPBuilder()));
     mapData.DevicePointers.push_back(
-        isDevicePtr ? llvm::OpenMPIRBuilder::DeviceInfoTy::Pointer
-                    : llvm::OpenMPIRBuilder::DeviceInfoTy::Address);
+        llvm::OpenMPIRBuilder::DeviceInfoTy::Address);
     mapData.IsAMapping.push_back(false);
     mapData.IsAMember.push_back(checkIsAMember(hasDevAddrOperands, mapOp));
   }

mlir/test/Target/LLVMIR/omptarget-llvm.mlir

@@ -622,20 +622,3 @@ module attributes {omp.target_triples = ["amdgcn-amd-amdhsa"]} {
 // CHECK:         br label %[[VAL_40]]
 // CHECK:       omp.done:                                         ; preds = %[[VAL_68]], %[[VAL_63]], %[[VAL_32]]
 // CHECK:         ret void
-
-// -----
-
-module attributes {omp.target_triples = ["amdgcn-amd-amdhsa"]} {
-  llvm.func @_QPomp_target_is_device_ptr(%arg0 : !llvm.ptr) {
-    %map = omp.map.info var_ptr(%arg0 : !llvm.ptr, !llvm.ptr)
-        map_clauses(is_device_ptr) capture(ByRef) -> !llvm.ptr {name = ""}
-    omp.target map_entries(%map -> %ptr_arg : !llvm.ptr) {
-      omp.terminator
-    }
-    llvm.return
-  }
-}
-
-// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 8]
-// CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 288]
-// CHECK-LABEL: define void @_QPomp_target_is_device_ptr

mlir/test/Target/LLVMIR/openmp-todo.mlir

@@ -225,6 +225,17 @@ llvm.func @target_in_reduction(%x : !llvm.ptr) {
 
 // -----
 
+llvm.func @target_is_device_ptr(%x : !llvm.ptr) {
+  // expected-error@below {{not yet implemented: Unhandled clause is_device_ptr in omp.target operation}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.target}}
+  omp.target is_device_ptr(%x : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @target_enter_data_depend(%x: !llvm.ptr) {
   // expected-error@below {{not yet implemented: Unhandled clause depend in omp.target_enter_data operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target_enter_data}}

offload/test/offloading/fortran/target-is-device-ptr.f90

@@ -1,60 +0,0 @@
-! Validate that a device pointer obtained via omp_get_mapped_ptr can be used
-! inside a TARGET region with the is_device_ptr clause.
-! REQUIRES: flang, amdgcn-amd-amdhsa
-
-! RUN: %libomptarget-compile-fortran-run-and-check-generic
-
-module mod
-  implicit none
-  integer, parameter :: n = 4
-contains
-  subroutine kernel(dptr)
-    use iso_c_binding, only : c_ptr, c_f_pointer
-    implicit none
-
-    type(c_ptr) :: dptr
-    integer, dimension(:), pointer :: b
-    integer :: i
-
-    b => null()
-
-    !$omp target is_device_ptr(dptr)
-      call c_f_pointer(dptr, b, [n])
-      do i = 1, n
-        b(i) = b(i) + 1
-      end do
-    !$omp end target
-  end subroutine kernel
-end module mod
-
-program is_device_ptr_target
-  use iso_c_binding, only : c_ptr, c_loc, c_f_pointer
-  use omp_lib, only: omp_get_default_device, omp_get_mapped_ptr
-  use mod, only: kernel, n
-  implicit none
-
-  integer, dimension(n), target :: a
-  integer :: dev
-  type(c_ptr) :: dptr
-
-  a = [2, 4, 6, 8]
-  print '("BEFORE:", I3)', a
-
-  dev = omp_get_default_device()
-
-  !$omp target data map(tofrom: a)
-    dptr = omp_get_mapped_ptr(c_loc(a), dev)
-    call kernel(dptr)
-  !$omp end target data
-
-  print '("AFTER: ", I3)', a
-
-  if (all(a == [3, 5, 7, 9])) then
-    print '("PASS")'
-  else
-    print '("FAIL   ", I3)', a
-  end if
-
-end program is_device_ptr_target
-
-!CHECK: PASS