Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][OpenMP] Add semantic checks for is_device_ptr #71255

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

shraiysh
Copy link
Member

@shraiysh shraiysh commented Nov 3, 2023

This patch adds the following semantic check for is_device_ptr

A list item that appears in an is_device_ptr clause must be a
dummy argument that does not have the ALLOCATABLE, POINTER or
VALUE attribute.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 3, 2023

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Shraiysh (shraiysh)

Changes

This patch adds the following semantic check for is_device_ptr

A list item that appears in an is_device_ptr clause must be a
dummy argument that does not have the ALLOCATABLE, POINTER or
VALUE attribute.

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

3 Files Affected:

  • (modified) flang/include/flang/Semantics/tools.h (+3)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+16-12)
  • (modified) flang/test/Semantics/OpenMP/target01.f90 (+29-2)
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index e3deb2da1be04ab..633787f45e85255 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -143,6 +143,9 @@ inline bool IsPointer(const Symbol &symbol) {
 inline bool IsAllocatable(const Symbol &symbol) {
   return symbol.attrs().test(Attr::ALLOCATABLE);
 }
+inline bool IsValue(const Symbol &symbol) {
+  return symbol.attrs().test(Attr::VALUE);
+}
 // IsAllocatableOrObjectPointer() may be the better choice
 inline bool IsAllocatableOrPointer(const Symbol &symbol) {
   return IsPointer(symbol) || IsAllocatable(symbol);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 12f54fbd51e1c2f..a462a007fd8deb3 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -11,6 +11,7 @@
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/tools.h"
 #include <algorithm>
+#include <iostream>
 
 namespace Fortran::semantics {
 
@@ -2852,18 +2853,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::IsDevicePtr &x) {
     const auto &isDevicePtrClause{
         std::get<parser::OmpClause::IsDevicePtr>(itr->second->u)};
     const auto &isDevicePtrList{isDevicePtrClause.v};
-    std::list<parser::Name> isDevicePtrNameList;
-    for (const auto &ompObject : isDevicePtrList.v) {
-      if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
-        if (name->symbol) {
-          if (!(IsBuiltinCPtr(*(name->symbol)))) {
-            context_.Say(itr->second->source,
-                "Variable '%s' in IS_DEVICE_PTR clause must be of type C_PTR"_err_en_US,
-                name->ToString());
-          } else {
-            isDevicePtrNameList.push_back(*name);
-          }
-        }
+    SymbolSourceMap currSymbols;
+    GetSymbolsInObjectList(isDevicePtrList, currSymbols);
+    for (auto &[symbol, source] : currSymbols) {
+      if (!(IsBuiltinCPtr(*symbol))) {
+        context_.Say(itr->second->source,
+            "Variable '%s' in IS_DEVICE_PTR clause must be of type C_PTR"_err_en_US,
+            source.ToString());
+      } else if (!(IsDummy(*symbol))) {
+        context_.Say(itr->second->source,
+            "Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument"_err_en_US,
+            source.ToString());
+      } else if (IsAllocatableOrPointer(*symbol) || IsValue(*symbol)) {
+        context_.Say(itr->second->source,
+            "Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute."_err_en_US,
+            source.ToString());
       }
     }
   }
diff --git a/flang/test/Semantics/OpenMP/target01.f90 b/flang/test/Semantics/OpenMP/target01.f90
index d672b905a70ad2f..485fa1f2530c3b7 100644
--- a/flang/test/Semantics/OpenMP/target01.f90
+++ b/flang/test/Semantics/OpenMP/target01.f90
@@ -1,5 +1,6 @@
 ! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
- 
+
+subroutine foo(b)
 use iso_c_binding
 integer :: x,y
 type(C_PTR) :: b
@@ -28,4 +29,30 @@
   y = y - 1
 !$omp end target
 
-end
+end subroutine foo
+
+subroutine bar(b1, b2, b3)
+  use iso_c_binding
+  integer :: y
+  type(c_ptr) :: c
+  type(c_ptr), allocatable :: b1
+  type(c_ptr), pointer :: b2
+  type(c_ptr), value :: b3
+
+  !ERROR: Variable 'c' in IS_DEVICE_PTR clause must be a dummy argument
+  !$omp target is_device_ptr(c)
+    y = y + 1
+  !$omp end target
+  !ERROR: Variable 'b1' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
+  !$omp target is_device_ptr(b1)
+    y = y + 1
+  !$omp end target
+  !ERROR: Variable 'b2' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
+  !$omp target is_device_ptr(b2)
+    y = y + 1
+  !$omp end target
+  !ERROR: Variable 'b3' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
+  !$omp target is_device_ptr(b3)
+    y = y + 1
+  !$omp end target
+end subroutine bar

@@ -11,6 +11,7 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/tools.h"
#include <algorithm>
#include <iostream>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, can we avoid using iostream here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry for that, its an unrelated change - I was trying to dump the attribute list to std::cout, sorry. Will remove it.

This patch adds the following semantic check for is_device_ptr

```
A list item that appears in an is_device_ptr clause must be a
dummy argument that does not have the ALLOCATABLE, POINTER or
VALUE attribute.
```
Copy link
Contributor

@raghavendhra raghavendhra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@shraiysh shraiysh merged commit d6b69a1 into llvm:main Nov 6, 2023
3 checks passed
raghavendhra added a commit that referenced this pull request Dec 8, 2023
…cks in IS_DEVICE_PTR related to list-items being dummy arguments. (#74370)

Changed semantic check from giving error to giving a warning about
deprecation from OpenMP 5.2 and later about checks for dummy argument
list-items present on IS_DEVICE_PTR clause.

This P is blocker for
#71255
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants