Addressed nit reviewer comments.

Author: raghavendhra

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

@@ -751,6 +751,20 @@ 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 {
@@ -771,6 +785,20 @@ 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>(
@@ -994,34 +1022,6 @@ bool ClauseProcessor::processUseDevicePtr(
       });
 }
 
-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::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);
-      });
-}
-
 } // namespace omp
 } // namespace lower
 } // namespace Fortran

flang/lib/Lower/OpenMP/ClauseProcessor.h

@@ -66,6 +66,12 @@ 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;
@@ -104,6 +110,12 @@ 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.
@@ -141,20 +153,6 @@ class ClauseProcessor {
                       llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
                           &useDeviceSymbols) 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
-  processHasDeviceAddr(llvm::SmallVectorImpl<mlir::Value> &operands,
-                       llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
-                       llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
-                       llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
-                           &isDeviceSymbols) const;
-
   template <typename T>
   bool processMotionClauses(Fortran::lower::StatementContext &stmtCtx,
                             llvm::SmallVectorImpl<mlir::Value> &mapOperands);

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

@@ -1635,11 +1635,11 @@ def TargetOp : OpenMP_Op<"target", [IsolatedFromAbove, MapClauseOwningOpInterfac
     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 $is_device_ptr indicates list items are device pointers.
 
     The optional $has_device_addr indicates that list items already have device
-    addresses, so may be directly accessed from target device. May include array
-    sections.
+    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.