[Flang][OpenMP] Add lowering support for is_device_ptr clause

[TIFitis]

Add support for OpenMP is_device_ptr clause for target directives.

[Copilot]

Pull request overview

This PR adds lowering support for the OpenMP is_device_ptr clause in target directives for Fortran. The implementation follows established patterns from similar clauses like has_device_addr and use_device_ptr.

• Updates processIsDevicePtr to use the standard processMapObjects and insertChildMapInfoIntoParent helper functions
• Implements map duplication logic in genTargetOp to create separate MapInfoOp instances for is_device_ptr and the synthesized has_device_addr entry
• Extends duplicate symbol checking to include is_device_ptr symbols

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
flang/test/Lower/OpenMP/target.f90	Adds test case verifying is_device_ptr clause generates correct map operations and is wired to both is_device_ptr and has_device_addr operands
flang/lib/Lower/OpenMP/ClauseProcessor.h	Updates processIsDevicePtr signature to accept StatementContext parameter
flang/lib/Lower/OpenMP/ClauseProcessor.cpp	Refactors processIsDevicePtr to use processMapObjects pattern; removes now-unused addUseDeviceClause helper function
flang/lib/Lower/OpenMP/OpenMP.cpp	Updates call site for processIsDevicePtr, implements map duplication logic for is_device_ptr symbols, and extends isDuplicateMappedSymbol to check for is_device_ptr symbols

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🐧 Linux x64 Test Results

• 7424 tests passed
• 596 tests skipped

✅ The build succeeded and all tests passed.

[llvmbot]

@llvm/pr-subscribers-offload
@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Akash Banerjee (TIFitis)

Changes

Add support for OpenMP is_device_ptr clause for target directives.

---

Full diff: https://github.com/llvm/llvm-project/pull/169331.diff

4 Files Affected:

• (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+15-27)
• (modified) flang/lib/Lower/OpenMP/ClauseProcessor.h (+1-1)
• (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+39-4)
• (modified) flang/test/Lower/OpenMP/target.f90 (+30)

diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 1c163e6de7e5a..81a47e20b2a88 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -42,15 +42,6 @@ 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) {
@@ -209,18 +200,6 @@ 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
 //===----------------------------------------------------------------------===//
@@ -1159,14 +1138,23 @@ bool ClauseProcessor::processInReduction(
 }
 
 bool ClauseProcessor::processIsDevicePtr(
-    mlir::omp::IsDevicePtrClauseOps &result,
+    lower::StatementContext &stmtCtx, mlir::omp::IsDevicePtrClauseOps &result,
     llvm::SmallVectorImpl<const semantics::Symbol *> &isDeviceSyms) const {
-  return findRepeatableClause<omp::clause::IsDevicePtr>(
-      [&](const omp::clause::IsDevicePtr &devPtrClause,
-          const parser::CharBlock &) {
-        addUseDeviceClause(converter, devPtrClause.v, result.isDevicePtrVars,
-                           isDeviceSyms);
+  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);
+        mlir::omp::ClauseMapFlags mapTypeBits =
+            mlir::omp::ClauseMapFlags::storage;
+        processMapObjects(stmtCtx, location, clause.v, mapTypeBits,
+                          parentMemberIndices, result.isDevicePtrVars,
+                          isDeviceSyms);
       });
+
+  insertChildMapInfoIntoParent(converter, semaCtx, stmtCtx, parentMemberIndices,
+                               result.isDevicePtrVars, isDeviceSyms);
+  return clauseFound;
 }
 
 bool ClauseProcessor::processLinear(mlir::omp::LinearClauseOps &result) const {
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index 6452e39b97551..4d2b684c51f60 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -130,7 +130,7 @@ class ClauseProcessor {
       mlir::Location currentLocation, mlir::omp::InReductionClauseOps &result,
       llvm::SmallVectorImpl<const semantics::Symbol *> &outReductionSyms) const;
   bool processIsDevicePtr(
-      mlir::omp::IsDevicePtrClauseOps &result,
+      lower::StatementContext &stmtCtx, mlir::omp::IsDevicePtrClauseOps &result,
       llvm::SmallVectorImpl<const semantics::Symbol *> &isDeviceSyms) const;
   bool processLinear(mlir::omp::LinearClauseOps &result) const;
   bool
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 71067283d13f7..52e482002840d 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1673,7 +1673,7 @@ static void genTargetClauses(
     hostEvalInfo->collectValues(clauseOps.hostEvalVars);
   }
   cp.processIf(llvm::omp::Directive::OMPD_target, clauseOps);
-  cp.processIsDevicePtr(clauseOps, isDevicePtrSyms);
+  cp.processIsDevicePtr(stmtCtx, clauseOps, isDevicePtrSyms);
   cp.processMap(loc, stmtCtx, clauseOps, llvm::omp::Directive::OMPD_unknown,
                 &mapSyms);
   cp.processNowait(clauseOps);
@@ -2485,13 +2485,15 @@ 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 *> &mappedSyms,
+    const llvm::SmallVectorImpl<const semantics::Symbol *> &isDevicePtrSyms) {
   llvm::SmallVector<const semantics::Symbol *> concatSyms;
   concatSyms.reserve(privatizedSyms.size() + hasDevSyms.size() +
-                     mappedSyms.size());
+                     mappedSyms.size() + isDevicePtrSyms.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(),
@@ -2538,6 +2540,39 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
                            /*isTargetPrivitization=*/true);
   dsp.processStep1(&clauseOps);
 
+  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.
+    fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+    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];
+      if (!sym || !mapVal)
+        continue;
+      auto mapInfo = mapVal.getDefiningOp<mlir::omp::MapInfoOp>();
+      if (!mapInfo)
+        continue;
+
+      if (!alreadyPresent(sym)) {
+        clauseOps.hasDeviceAddrVars.push_back(mapVal);
+        hasDeviceAddrSyms.push_back(sym);
+      }
+
+      builder.setInsertionPointAfter(mapInfo);
+      auto clonedOp = builder.clone(*mapInfo.getOperation());
+      auto clonedMapInfo = mlir::dyn_cast<mlir::omp::MapInfoOp>(clonedOp);
+      assert(clonedMapInfo && "expected cloned map info op");
+      clauseOps.isDevicePtrVars[idx] = clonedMapInfo.getResult();
+    }
+  }
+
   // 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
@@ -2570,7 +2605,7 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       return;
 
     if (!isDuplicateMappedSymbol(sym, dsp.getAllSymbolsToPrivatize(),
-                                 hasDeviceAddrSyms, mapSyms)) {
+                                 hasDeviceAddrSyms, mapSyms, isDevicePtrSyms)) {
       if (const auto *details =
               sym.template detailsIf<semantics::HostAssocDetails>())
         converter.copySymbolBinding(details->symbol(), sym);
diff --git a/flang/test/Lower/OpenMP/target.f90 b/flang/test/Lower/OpenMP/target.f90
index 26bd62edf9d0c..1e62adc0f6f98 100644
--- a/flang/test/Lower/OpenMP/target.f90
+++ b/flang/test/Lower/OpenMP/target.f90
@@ -566,6 +566,36 @@ 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_associated, 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)
+      if (c_associated(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
 !===============================================================================

[ergawy]

Thanks Akash! Just a few stop-by comments.

[ergawy]

LGTM, thanks Akash. Please wait for another approval before merging.

[TIFitis]

Ping for review. (Only the FIR lowering changes are awaiting approval.)

[mjklemm]

LGTM