[OpenMP] [Semantics] [Flang] Adding more semantic checks for USE_DEVICE_PTR and USE_DEVICE_ADDR clauses.

[raghavendhra]

Summary

The following restrictions for USE_DEVICE_PTR and USE_DEVICE_ADDR clauses on OMP TARGET DATA directive are implemented in this patch.

• A list item may not be specified more than once in use_device_ptr clauses that appear on the directive.
• A list item may not be specified more than once in use_device_addr clauses that appear on the directive.
• A list item may not be specified in both a use_device_addr clause and a use_device_ptr clause on the directive.
• A list item that appears in a use_device_ptr or use_device_addr clause must not be a structure element.
• A list item that appears in a use_device_ptr must be of type C_PTR.

[raghavendhra]

Rebasing and fixed formatting.

[raghavendhra]

Rebasing

[raghavendhra]

Rebase

[raghavendhra]

Fixed formatting and rebasing.

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

235

Wouldn't it be nicer to have a function per directive to check for their specific semantic instead of this if (directive) pattern. checkMultipleOcurrence can be extracted as a function if you want to reuse it.

[kiranchandramohan]

Looks mostly OK. A few minor comments.

flang/lib/Semantics/check-omp-structure.cpp

235

+1

251

This was supported prior to OpenMP 5.1. Keep in mind that there might be some old code that might need this. No change requested here.

2651–2667

You could move this check into a new function and share it with UseDevicePtr, CheckDependList function.

[raghavendhra]

Addressed review comments by moving CheckMultipleOccurences() from lambda to function. Rebased the patch.

[kiranchandramohan]

LG.

Nit: Spell out types if the type is not immediately clear from the local context.

[raghavendhra]

Final rebase before commit.

[vdonaldson]

flang/lib/Semantics/check-omp-structure.cpp

2630

I'm seeing an error: unused variable ‘dataRef’ [-Werror=unused-variable] build error on this line.

[raghavendhra]

Sorry! Let me try to fix that ASAP!

[raghavendhra]

Addressed this unused variable build error by submitting a new patch https://reviews.llvm.org/D157126

Sorry for inconvenience caused. Thanks!

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.inc"
   // Get the OpenMP Clause Kind for the corresponding Parser class
   template <typename A>
   llvm::omp::Clause GetClauseKindForParserClass(const A &) {
 #define GEN_FLANG_CLAUSE_PARSER_KIND_MAP
 #include "llvm/Frontend/OpenMP/OMP.inc"
   }
 
 private:
+  void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars,
+      const std::list<parser::Name> &nameList, const parser::CharBlock &item,
+      const std::string &clauseName);
   void CheckMultListItems();
+  void CheckStructureElement(const parser::OmpObjectList &ompObjectList,
+      const llvm::omp::Clause clause);
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
   bool IsCloselyNestedRegion(const OmpDirectiveSet &set);
   void HasInvalidTeamsNesting(
       const llvm::omp::Directive &dir, const parser::CharBlock &source);
   void HasInvalidDistributeNesting(const parser::OpenMPLoopConstruct &x);
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,

flang/lib/Semantics/check-omp-structure.cpp

while (index != -1) {
         return false;
       }
       index--;
     }
   }
   return false;
 }
 
-void OmpStructureChecker::CheckMultListItems() {
-  semantics::UnorderedSymbolSet listVars;
-  auto checkMultipleOcurrence = [&](const std::list<parser::Name> &nameList,
-                                    const parser::CharBlock &item,
-                                    const std::string &clauseName) {
-    for (auto const &var : nameList) {
-      if (llvm::is_contained(listVars, *(var.symbol))) {
-        context_.Say(item,
-            "List item '%s' present at multiple %s clauses"_err_en_US,
-            var.ToString(), clauseName);
-      }
-      listVars.insert(*(var.symbol));
-    }
-  };
+void OmpStructureChecker::CheckMultipleOccurrence(
+    semantics::UnorderedSymbolSet &listVars,
+    const std::list<parser::Name> &nameList, const parser::CharBlock &item,
+}
+
+void OmpStructureChecker::CheckMultListItems() {
+  semantics::UnorderedSymbolSet listVars;
 
   // Aligned clause
   auto alignedClauses{FindClauses(llvm::omp::Clause::OMPC_aligned)};
   for (auto itr = alignedClauses.first; itr != alignedClauses.second; ++itr) {
     const auto &alignedClause{
         std::get<parser::OmpClause::Aligned>(itr->second->u)};
     const auto &alignedList{std::get<0>(alignedClause.v.t)};
     std::list<parser::Name> alignedNameList;
for (const auto &ompObject : alignedList.v) {
             alignedNameList.push_back(*name);
           }
         } else {
           // The symbol is null, return early
           return;
         }
       }
     }
-    checkMultipleOcurrence(alignedNameList, itr->second->source, "ALIGNED");
+    CheckMultipleOccurrence(
+        listVars, alignedNameList, itr->second->source, "ALIGNED");
   }
 
   // Nontemporal clause
   auto nonTemporalClauses{FindClauses(llvm::omp::Clause::OMPC_nontemporal)};
   for (auto itr = nonTemporalClauses.first; itr != nonTemporalClauses.second;
        ++itr) {
     const auto &nontempClause{
         std::get<parser::OmpClause::Nontemporal>(itr->second->u)};
     const auto &nontempNameList{nontempClause.v};
-    checkMultipleOcurrence(nontempNameList, itr->second->source, "NONTEMPORAL");
+    CheckMultipleOccurrence(
+        listVars, nontempNameList, itr->second->source, "NONTEMPORAL");
   }
 }
 
 bool OmpStructureChecker::HasInvalidWorksharingNesting(
     const parser::CharBlock &source, const OmpDirectiveSet &set) {
   // set contains all the invalid closely nested directives
   // for the given directive (`source` here)
   if (IsCloselyNestedRegion(set)) {
     context_.Say(source,
         "A worksharing region may not be closely nested inside a "
         "worksharing, explicit task, taskloop, critical, ordered, atomic, or "
         "master region"_err_en_US);
     return true;
   }
   return false;
 }
 
 void OmpStructureChecker::HasInvalidDistributeNesting(
     const parser::OpenMPLoopConstruct &x) {
   bool violation{false};
 
   OmpDirectiveSet distributeSet{llvm::omp::Directive::OMPD_distribute,
       llvm::omp::Directive::OMPD_distribute_parallel_do,
       llvm::omp::Directive::OMPD_distribute_parallel_do_simd,
       llvm::omp::Directive::OMPD_distribute_parallel_for,
       llvm::omp::Directive::OMPD_distribute_parallel_for_simd,

 CHECK_SIMPLE_CLAUSE(Sizes, OMPC_sizes)
 CHECK_SIMPLE_CLAUSE(TaskReduction, OMPC_task_reduction)
 CHECK_SIMPLE_CLAUSE(To, OMPC_to)
 CHECK_SIMPLE_CLAUSE(UnifiedAddress, OMPC_unified_address)
 CHECK_SIMPLE_CLAUSE(UnifiedSharedMemory, OMPC_unified_shared_memory)
 CHECK_SIMPLE_CLAUSE(Uniform, OMPC_uniform)
 CHECK_SIMPLE_CLAUSE(Unknown, OMPC_unknown)
 CHECK_SIMPLE_CLAUSE(Untied, OMPC_untied)
-CHECK_SIMPLE_CLAUSE(UseDevicePtr, OMPC_use_device_ptr)
 CHECK_SIMPLE_CLAUSE(UsesAllocators, OMPC_uses_allocators)
 CHECK_SIMPLE_CLAUSE(Update, OMPC_update)
-CHECK_SIMPLE_CLAUSE(UseDeviceAddr, OMPC_use_device_addr)
 CHECK_SIMPLE_CLAUSE(Write, OMPC_write)
 CHECK_SIMPLE_CLAUSE(Init, OMPC_init)
 CHECK_SIMPLE_CLAUSE(Use, OMPC_use)
 CHECK_SIMPLE_CLAUSE(Novariants, OMPC_novariants)
 CHECK_SIMPLE_CLAUSE(Nocontext, OMPC_nocontext)
 CHECK_SIMPLE_CLAUSE(At, OMPC_at)
 CHECK_SIMPLE_CLAUSE(Severity, OMPC_severity)
 CHECK_SIMPLE_CLAUSE(Message, OMPC_message)
void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_copyin);
 
   SymbolSourceMap currSymbols;
   GetSymbolsInObjectList(x.v, currSymbols);
   CheckCopyingPolymorphicAllocatable(
       currSymbols, llvm::omp::Clause::OMPC_copyin);
 }
 
+void OmpStructureChecker::CheckStructureElement(
+    const parser::OmpObjectList &ompObjectList,
+    const llvm::omp::Clause clause) {
+  for (const auto &ompObject : ompObjectList.v) {
+    common::visit(
+        common::visitors{
+            [&](const parser::Designator &designator) {
+              if (const auto *dataRef{
+                      std::get_if<parser::DataRef>(&designator.u)}) {
+                if (parser::Unwrap<parser::StructureComponent>(ompObject)) {
+                  context_.Say(GetContext().clauseSource,
+                      "A variable that is part of another variable "
+                      "(structure element) cannot appear on the %s "
+                      "%s clause"_err_en_US,
+                      ContextDirectiveAsFortran(),
+                      parser::ToUpperCaseLetters(getClauseName(clause).str()));
+                }
+              }
+            },
+            [&](const parser::Name &name) {},
+        },
+        ompObject.u);
+  }
+  return;
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::UseDevicePtr &x) {
+  CheckStructureElement(x.v, llvm::omp::Clause::OMPC_use_device_ptr);
+  CheckAllowed(llvm::omp::Clause::OMPC_use_device_ptr);
+  SymbolSourceMap currSymbols;
+  GetSymbolsInObjectList(x.v, currSymbols);
+  semantics::UnorderedSymbolSet listVars;
+  auto useDevicePtrClauses{FindClauses(llvm::omp::Clause::OMPC_use_device_ptr)};
+  for (auto itr = useDevicePtrClauses.first; itr != useDevicePtrClauses.second;
+       ++itr) {
+    const auto &useDevicePtrClause{
+        std::get<parser::OmpClause::UseDevicePtr>(itr->second->u)};
+    const auto &useDevicePtrList{useDevicePtrClause.v};
+    std::list<parser::Name> useDevicePtrNameList;
+    for (const auto &ompObject : useDevicePtrList.v) {
+      if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+        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,
+                name->ToString());
+          } else {
+            useDevicePtrNameList.push_back(*name);
+          }
+        }
+      }
+    }
+    CheckMultipleOccurrence(
+        listVars, useDevicePtrNameList, itr->second->source, "USE_DEVICE_PTR");
+  }
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::UseDeviceAddr &x) {
+  CheckStructureElement(x.v, llvm::omp::Clause::OMPC_use_device_addr);
+  CheckAllowed(llvm::omp::Clause::OMPC_use_device_addr);
+  SymbolSourceMap currSymbols;
+  GetSymbolsInObjectList(x.v, currSymbols);
+  semantics::UnorderedSymbolSet listVars;
+  auto useDeviceAddrClauses{
+      FindClauses(llvm::omp::Clause::OMPC_use_device_addr)};
+  for (auto itr = useDeviceAddrClauses.first;
+       itr != useDeviceAddrClauses.second; ++itr) {
+    const auto &useDeviceAddrClause{
+        std::get<parser::OmpClause::UseDeviceAddr>(itr->second->u)};
+    const auto &useDeviceAddrList{useDeviceAddrClause.v};
+    std::list<parser::Name> useDeviceAddrNameList;
+    for (const auto &ompObject : useDeviceAddrList.v) {
+      if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+        if (name->symbol) {
+          useDeviceAddrNameList.push_back(*name);
+        }
+      }
+    }
+    CheckMultipleOccurrence(listVars, useDeviceAddrNameList,
+        itr->second->source, "USE_DEVICE_ADDR");
+  }
+}
+
 llvm::StringRef OmpStructureChecker::getClauseName(llvm::omp::Clause clause) {
   return llvm::omp::getOpenMPClauseName(clause);
 }
 
 llvm::StringRef OmpStructureChecker::getDirectiveName(
     llvm::omp::Directive directive) {
   return llvm::omp::getOpenMPDirectiveName(directive);
 }

flang/lib/Semantics/resolve-directives.cpp

common::visit(
                   context_.Say(designator.source, // 2.15.3
                       "List items must be declared in the same scoping unit "
                       "in which the %s directive appears"_err_en_US,
                       parser::ToUpperCaseLetters(
                           llvm::omp::getOpenMPDirectiveName(
                               GetContext().directive)
                               .str()));
                 }
+                if ((GetContext().directive ==
+                        llvm::omp::Directive::OMPD_target_data) &&
+                    (((ompFlag == Symbol::Flag::OmpUseDevicePtr) &&
+                         symbol->test(Symbol::Flag::OmpUseDeviceAddr)) ||
+                        ((ompFlag == Symbol::Flag::OmpUseDeviceAddr) &&
+                            symbol->test(Symbol::Flag::OmpUseDevicePtr)))) {
+                  context_.Say(designator.source,
+                      "Variable '%s' may not "
+                      "appear on both USE_DEVICE_PTR and USE_DEVICE_ADDR "
+                      "clauses on a TARGET DATA construct"_err_en_US,
+                      symbol->name());
+                }
               }
             } else {
               // Array sections to be changed to substrings as needed
               if (AnalyzeExpr(context_, designator)) {
                 if (std::holds_alternative<parser::Substring>(designator.u)) {
                   context_.Say(designator.source,
                       "Substrings are not allowed on OpenMP "
                       "directives or clauses"_err_en_US);

flang/test/Semantics/OpenMP/use_device_addr1.f90

+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 5.0
+! 2.10.1 use_device_ptr clause
+! List item in USE_DEVICE_ADDR clause must not be structure element.
+! Same list item can not be present multiple times or in multipe
+! USE_DEVICE_ADDR clauses.
+
+subroutine omp_target_data
+   integer :: a(1024)
+   integer, target :: b(1024)
+   type my_type
+    integer :: my_b(1024)
+   end type my_type
+
+   type(my_type) :: my_var
+   a = 1
+
+   !ERROR: A variable that is part of another variable (structure element) cannot appear on the TARGET DATA USE_DEVICE_ADDR clause
+   !$omp target data map(tofrom: a) use_device_addr(my_var%my_b)
+      my_var%my_b = a
+   !$omp end target data
+
+   !ERROR: List item 'b' present at multiple USE_DEVICE_ADDR clauses
+   !$omp target data map(tofrom: a) use_device_addr(b,b)
+      b = a
+   !$omp end target data
+
+   !ERROR: List item 'b' present at multiple USE_DEVICE_ADDR clauses
+   !$omp target data map(tofrom: a) use_device_addr(b) use_device_addr(b)
+      b = a
+   !$omp end target data
+
+end subroutine omp_target_data

flang/test/Semantics/OpenMP/use_device_ptr1.f90

+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 5.0
+! 2.10.1 use_device_ptr clause
+! List item in USE_DEVICE_PTR clause must not be structure element.
+! List item in USE_DEVICE_PTR clause must be of type C_PTR.
+! List items that appear in a use_device_ptr clause can not appear in
+! use_device_addr clause.
+! Same list item can not be present multiple times or in multipe
+! USE_DEVICE_PTR clauses.
+
+subroutine omp_target_data
+   use iso_c_binding
+   integer :: a(1024)
+   type(C_PTR) :: b
+   integer, pointer :: arrayB
+   type my_type
+    type(C_PTR) :: my_cptr
+   end type my_type
+
+   type(my_type) :: my_var
+   a = 1
+
+   !ERROR: A variable that is part of another variable (structure element) cannot appear on the TARGET DATA USE_DEVICE_PTR clause
+   !$omp target data map(tofrom: a, arrayB) use_device_ptr(my_var%my_cptr)
+      allocate(arrayB)
+      call c_f_pointer(my_var%my_cptr, arrayB)
+      a = arrayB
+   !$omp end target data
+
+   !ERROR: 'a' in USE_DEVICE_PTR clause must be of type C_PTR
+   !$omp target data map(tofrom: a) use_device_ptr(a)
+      a = 2
+   !$omp end target data
+
+   !ERROR: List item 'b' present at multiple USE_DEVICE_PTR clauses
+   !$omp target data map(tofrom: a, arrayB) use_device_ptr(b) use_device_ptr(b)
+      allocate(arrayB)
+      call c_f_pointer(b, arrayB)
+      a = arrayB
+   !$omp end target data
+
+   !ERROR: List item 'b' present at multiple USE_DEVICE_PTR clauses
+   !$omp target data map(tofrom: a, arrayB) use_device_ptr(b,b)
+      allocate(arrayB)
+      call c_f_pointer(b, arrayB)
+      a = arrayB
+   !$omp end target data
+
+   !ERROR: Variable 'b' may not appear on both USE_DEVICE_PTR and USE_DEVICE_ADDR clauses on a TARGET DATA construct
+   !$omp target data map(tofrom: a, arrayB) use_device_addr(b) use_device_ptr(b)
+      allocate(arrayB)
+      call c_f_pointer(b, arrayB)
+      a = arrayB
+   !$omp end target data
+
+   !ERROR: Variable 'b' may not appear on both USE_DEVICE_PTR and USE_DEVICE_ADDR clauses on a TARGET DATA construct
+   !$omp target data map(tofrom: a, arrayB) use_device_ptr(b) use_device_addr(b)
+      allocate(arrayB)
+      call c_f_pointer(b, arrayB)
+      a = arrayB
+   !$omp end target data
+
+end subroutine omp_target_data
+