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 check for target update #71305

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

shraiysh
Copy link
Member

@shraiysh shraiysh commented Nov 5, 2023

This patch adds the following semantic check for target update construct.

A list item can only appear in a to or from clause, but not in both.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 5, 2023

@llvm/pr-subscribers-flang-openmp

Author: Shraiysh (shraiysh)

Changes

This patch adds the following semantic check for target update construct.

A list item can only appear in a to or from clause, but not in both.

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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+14)
  • (modified) flang/test/Semantics/OpenMP/target-update01.f90 (+3)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index a328424e0c098d0..9f8af2217e47be2 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1401,6 +1401,20 @@ void OmpStructureChecker::CheckTargetUpdate() {
     context_.Say(GetContext().directiveSource,
         "At least one motion-clause (TO/FROM) must be specified on TARGET UPDATE construct."_err_en_US);
   }
+  if (toClause && fromClause) {
+    SymbolSourceMap toSymbols, fromSymbols;
+    GetSymbolsInObjectList(
+        std::get<parser::OmpClause::To>(toClause->u).v, toSymbols);
+    GetSymbolsInObjectList(
+        std::get<parser::OmpClause::From>(fromClause->u).v, fromSymbols);
+    for (auto &[symbol, source] : toSymbols) {
+      if (fromSymbols.find(symbol) != fromSymbols.end()) {
+        context_.Say(source,
+            "Symbol '%s': A list item can only appear in a to or from clause, but not in both."_err_en_US,
+            symbol->name());
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(
diff --git a/flang/test/Semantics/OpenMP/target-update01.f90 b/flang/test/Semantics/OpenMP/target-update01.f90
index a0728e52ba3d230..f77d3f248df6a0a 100644
--- a/flang/test/Semantics/OpenMP/target-update01.f90
+++ b/flang/test/Semantics/OpenMP/target-update01.f90
@@ -13,4 +13,7 @@ subroutine foo(x)
   !ERROR: At most one NOWAIT clause can appear on the TARGET UPDATE directive
   !$omp target update to(x) nowait nowait
 
+  !ERROR: Symbol 'x': A list item can only appear in a to or from clause, but not in both.
+  !$omp target update to(x) from(x)
+
 end subroutine

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 5, 2023

@llvm/pr-subscribers-flang-semantics

Author: Shraiysh (shraiysh)

Changes

This patch adds the following semantic check for target update construct.

A list item can only appear in a to or from clause, but not in both.

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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+14)
  • (modified) flang/test/Semantics/OpenMP/target-update01.f90 (+3)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index a328424e0c098d0..9f8af2217e47be2 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1401,6 +1401,20 @@ void OmpStructureChecker::CheckTargetUpdate() {
     context_.Say(GetContext().directiveSource,
         "At least one motion-clause (TO/FROM) must be specified on TARGET UPDATE construct."_err_en_US);
   }
+  if (toClause && fromClause) {
+    SymbolSourceMap toSymbols, fromSymbols;
+    GetSymbolsInObjectList(
+        std::get<parser::OmpClause::To>(toClause->u).v, toSymbols);
+    GetSymbolsInObjectList(
+        std::get<parser::OmpClause::From>(fromClause->u).v, fromSymbols);
+    for (auto &[symbol, source] : toSymbols) {
+      if (fromSymbols.find(symbol) != fromSymbols.end()) {
+        context_.Say(source,
+            "Symbol '%s': A list item can only appear in a to or from clause, but not in both."_err_en_US,
+            symbol->name());
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(
diff --git a/flang/test/Semantics/OpenMP/target-update01.f90 b/flang/test/Semantics/OpenMP/target-update01.f90
index a0728e52ba3d230..f77d3f248df6a0a 100644
--- a/flang/test/Semantics/OpenMP/target-update01.f90
+++ b/flang/test/Semantics/OpenMP/target-update01.f90
@@ -13,4 +13,7 @@ subroutine foo(x)
   !ERROR: At most one NOWAIT clause can appear on the TARGET UPDATE directive
   !$omp target update to(x) nowait nowait
 
+  !ERROR: Symbol 'x': A list item can only appear in a to or from clause, but not in both.
+  !$omp target update to(x) from(x)
+
 end subroutine

for (auto &[symbol, source] : toSymbols) {
if (fromSymbols.find(symbol) != fromSymbols.end()) {
context_.Say(source,
"Symbol '%s': A list item can only appear in a to or from clause, but not in both."_err_en_US,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We have the clause name in caps usually.
Symbol is an internal concept. Might be better to phrase in terms of what the standard says.
Would it be possible to highlight both occurrences?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

This patch adds the following semantic check for target update
construct.

```
A list item can only appear in a to or from clause, but not in both.
```
Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

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

LG. Thanks.

@shraiysh shraiysh merged commit ee10005 into llvm:main Nov 6, 2023
3 checks passed
@shraiysh shraiysh deleted the target_update2 branch November 6, 2023 23:16
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.

3 participants