[flang][OpenMP] Add semantic check for declare target #71861
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shraiysh
requested review from
skatrak,
kiranchandramohan,
raghavendhra,
agozillon,
dpalermo,
NimishMishra,
TIFitis and
kiranktp
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-parser Author: Shraiysh (shraiysh) ChangesThis patch adds the following check from OpenMP 5.2. Also added a warning for the deprication of Full diff: https://github.com/llvm/llvm-project/pull/71861.diff 11 Files Affected:
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 0271f699a687ff0..bba1be27158ce7e 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -259,6 +259,8 @@ TYPE_PARSER(
parenthesized("STATIC" >> maybe("," >> scalarIntExpr)))) ||
"DYNAMIC_ALLOCATORS" >>
construct<OmpClause>(construct<OmpClause::DynamicAllocators>()) ||
+ "ENTER" >> construct<OmpClause>(construct<OmpClause::Enter>(
+ parenthesized(Parser<OmpObjectList>{}))) ||
"FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
parenthesized(scalarLogicalExpr))) ||
"FULL" >> construct<OmpClause>(construct<OmpClause::Full>()) ||
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index d7a0681d1c3d476..2cb938ab11d9f22 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1164,13 +1164,31 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Allocate &x) {
}
}
+void OmpStructureChecker::Enter(const parser::OmpDeclareTargetWithClause &x) {
+ SetClauseSets(llvm::omp::Directive::OMPD_declare_target);
+}
+
+void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) {
+ if (x.v.v.size() > 0) {
+ const parser::OmpClause *enterClause =
+ FindClause(llvm::omp::Clause::OMPC_enter);
+ const parser::OmpClause *toClause = FindClause(llvm::omp::Clause::OMPC_to);
+ const parser::OmpClause *linkClause =
+ FindClause(llvm::omp::Clause::OMPC_link);
+ if (!enterClause && !toClause && !linkClause) {
+ context_.Say(x.source,
+ "If the DECLARE TARGET directive has a clause, it must contain at lease one ENTER clause or LINK clause"_err_en_US);
+ }
+ if (toClause) {
+ context_.Say(toClause->source,
+ "The usage of TO clause on DECLARE TARGET directive has been deprecated. Use ENTER clause instead."_warn_en_US);
+ }
+ }
+}
+
void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
const auto &dir{std::get<parser::Verbatim>(x.t)};
PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);
- const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
- if (std::holds_alternative<parser::OmpDeclareTargetWithClause>(spec.u)) {
- SetClauseSets(llvm::omp::Directive::OMPD_declare_target);
- }
}
void OmpStructureChecker::Enter(const parser::OmpDeclareTargetWithList &x) {
@@ -1244,7 +1262,8 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
CheckThreadprivateOrDeclareTargetVar(*objectList);
} else if (const auto *clauseList{
parser::Unwrap<parser::OmpClauseList>(spec.u)}) {
- bool toClauseFound{false}, deviceTypeClauseFound{false};
+ bool toClauseFound{false}, deviceTypeClauseFound{false},
+ enterClauseFound{false};
for (const auto &clause : clauseList->v) {
common::visit(
common::visitors{
@@ -1259,6 +1278,12 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
CheckIsVarPartOfAnotherVar(dir.source, linkClause.v);
CheckThreadprivateOrDeclareTargetVar(linkClause.v);
},
+ [&](const parser::OmpClause::Enter &enterClause) {
+ enterClauseFound = true;
+ CheckSymbolNames(dir.source, enterClause.v);
+ CheckIsVarPartOfAnotherVar(dir.source, enterClause.v);
+ CheckThreadprivateOrDeclareTargetVar(enterClause.v);
+ },
[&](const parser::OmpClause::DeviceType &deviceTypeClause) {
deviceTypeClauseFound = true;
if (deviceTypeClause.v.v !=
@@ -1272,7 +1297,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
},
clause.u);
- if (toClauseFound && !deviceTypeClauseFound) {
+ if ((toClauseFound || enterClauseFound) && !deviceTypeClauseFound) {
deviceConstructFound_ = true;
}
}
@@ -2227,6 +2252,7 @@ CHECK_SIMPLE_CLAUSE(CancellationConstructType, OMPC_cancellation_construct_type)
CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
+CHECK_SIMPLE_CLAUSE(Enter, OMPC_enter)
CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index d35602cca75d549..90e5c9f19127750 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -80,6 +80,8 @@ class OmpStructureChecker
void Enter(const parser::OpenMPDeclareTargetConstruct &);
void Leave(const parser::OpenMPDeclareTargetConstruct &);
void Enter(const parser::OmpDeclareTargetWithList &);
+ void Enter(const parser::OmpDeclareTargetWithClause &);
+ void Leave(const parser::OmpDeclareTargetWithClause &);
void Enter(const parser::OpenMPExecutableAllocate &);
void Leave(const parser::OpenMPExecutableAllocate &);
void Enter(const parser::OpenMPAllocatorsConstruct &);
diff --git a/flang/test/Parser/OpenMP/declare_target-device_type.f90 b/flang/test/Parser/OpenMP/declare_target-device_type.f90
index 2f6b68ab30f6e91..19e5accc4fc034f 100644
--- a/flang/test/Parser/OpenMP/declare_target-device_type.f90
+++ b/flang/test/Parser/OpenMP/declare_target-device_type.f90
@@ -2,12 +2,13 @@
! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
subroutine openmp_declare_target
- !CHECK: !$omp declare target device_type(host)
- !$omp declare target device_type(host)
- !CHECK: !$omp declare target device_type(nohost)
- !$omp declare target device_type(nohost)
- !CHECK: !$omp declare target device_type(any)
- !$omp declare target device_type(any)
+ integer, save :: x
+ !CHECK: !$omp declare target device_type(host) enter(x)
+ !$omp declare target device_type(host) enter(x)
+ !CHECK: !$omp declare target device_type(nohost) enter(x)
+ !$omp declare target device_type(nohost) enter(x)
+ !CHECK: !$omp declare target device_type(any) enter(x)
+ !$omp declare target device_type(any) enter(x)
integer :: a(1024), i
!CHECK: do
do i = 1, 1024
diff --git a/flang/test/Semantics/OpenMP/declarative-directive.f90 b/flang/test/Semantics/OpenMP/declarative-directive.f90
index e48a682288583f7..326b4d446608dba 100644
--- a/flang/test/Semantics/OpenMP/declarative-directive.f90
+++ b/flang/test/Semantics/OpenMP/declarative-directive.f90
@@ -64,9 +64,10 @@ subroutine foo
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
- !$omp declare target to(Q, S) link(R)
+ !$omp declare target enter(Q, S) link(R)
+ !ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
!ERROR: MAP clause is not allowed on the DECLARE TARGET directive
- !$omp declare target map(from:Q)
+ !$omp declare target enter(Q) map(from:Q)
integer, parameter :: N=10000, M=1024
integer :: i
real :: Q(N, N), R(N,M), S(M,M)
diff --git a/flang/test/Semantics/OpenMP/declare-target01.f90 b/flang/test/Semantics/OpenMP/declare-target01.f90
index 862b4c5866f3fb6..6dec7896aec0b45 100644
--- a/flang/test/Semantics/OpenMP/declare-target01.f90
+++ b/flang/test/Semantics/OpenMP/declare-target01.f90
@@ -49,40 +49,43 @@ module declare_target01
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
!$omp declare target (y%KIND)
+ !WARNING: The usage of TO clause on DECLARE TARGET directive has been deprecated. Use ENTER clause instead.
!$omp declare target to (my_var)
- !$omp declare target to (my_var) device_type(host)
+ !$omp declare target enter (my_var)
+
+ !$omp declare target enter (my_var) device_type(host)
!ERROR: A variable that is part of another variable (as an array or structure element) cannot appear on the DECLARE TARGET directive
- !$omp declare target to (my_var%t_i)
+ !$omp declare target enter (my_var%t_i)
!ERROR: A variable that is part of another variable (as an array or structure element) cannot appear on the DECLARE TARGET directive
- !$omp declare target to (my_var%t_arr)
+ !$omp declare target enter (my_var%t_arr)
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
- !$omp declare target to (my_var%kind_param)
+ !$omp declare target enter (my_var%kind_param)
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
- !$omp declare target to (my_var%len_param)
+ !$omp declare target enter (my_var%len_param)
- !$omp declare target to (arr)
+ !$omp declare target enter (arr)
- !$omp declare target to (arr) device_type(nohost)
+ !$omp declare target enter (arr) device_type(nohost)
!ERROR: A variable that is part of another variable (as an array or structure element) cannot appear on the DECLARE TARGET directive
- !$omp declare target to (arr(1))
+ !$omp declare target enter (arr(1))
!ERROR: A variable that is part of another variable (as an array or structure element) cannot appear on the DECLARE TARGET directive
- !$omp declare target to (arr(1:2))
+ !$omp declare target enter (arr(1:2))
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
- !$omp declare target to (x%KIND)
+ !$omp declare target enter (x%KIND)
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
- !$omp declare target to (w%LEN)
+ !$omp declare target enter (w%LEN)
!ERROR: A type parameter inquiry cannot appear on the DECLARE TARGET directive
- !$omp declare target to (y%KIND)
+ !$omp declare target enter (y%KIND)
!$omp declare target link (my_var2)
diff --git a/flang/test/Semantics/OpenMP/declare-target02.f90 b/flang/test/Semantics/OpenMP/declare-target02.f90
index 57a2776ff934054..77c8bb62fb7b23a 100644
--- a/flang/test/Semantics/OpenMP/declare-target02.f90
+++ b/flang/test/Semantics/OpenMP/declare-target02.f90
@@ -16,12 +16,12 @@ program declare_target02
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
!$omp declare target (a1)
- !$omp declare target to (arr1_to)
+ !$omp declare target enter (arr1_to)
- !$omp declare target to (blk1_to)
+ !$omp declare target enter (blk1_to)
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
- !$omp declare target to (a1_to)
+ !$omp declare target enter (a1_to)
!$omp declare target link (arr1_link)
@@ -35,7 +35,7 @@ program declare_target02
!$omp declare target (eq_a)
!ERROR: A variable in a DECLARE TARGET directive cannot appear in an EQUIVALENCE statement
- !$omp declare target to (eq_a)
+ !$omp declare target enter (eq_a)
!ERROR: A variable in a DECLARE TARGET directive cannot appear in an EQUIVALENCE statement
!$omp declare target link (eq_b)
@@ -44,7 +44,7 @@ program declare_target02
!$omp declare target (eq_c)
!ERROR: A variable in a DECLARE TARGET directive cannot appear in an EQUIVALENCE statement
- !$omp declare target to (eq_c)
+ !$omp declare target enter (eq_c)
!ERROR: A variable in a DECLARE TARGET directive cannot appear in an EQUIVALENCE statement
!$omp declare target link (eq_d)
@@ -70,15 +70,15 @@ subroutine func()
!$omp declare target (a3)
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
- !$omp declare target to (arr2_to)
+ !$omp declare target enter (arr2_to)
- !$omp declare target to (arr3_to)
+ !$omp declare target enter (arr3_to)
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
- !$omp declare target to (a2_to)
+ !$omp declare target enter (a2_to)
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
- !$omp declare target to (a3_to)
+ !$omp declare target enter (a3_to)
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
!$omp declare target link (arr2_link)
@@ -104,12 +104,12 @@ module mod4
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
!$omp declare target (a4)
- !$omp declare target to (arr4_to)
+ !$omp declare target enter (arr4_to)
- !$omp declare target to (blk4_to)
+ !$omp declare target enter (blk4_to)
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
- !$omp declare target to (a4_to)
+ !$omp declare target enter (a4_to)
!$omp declare target link (arr4_link)
@@ -133,13 +133,13 @@ subroutine func5()
!$omp declare target (a5)
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
- !$omp declare target to (arr5_to)
+ !$omp declare target enter (arr5_to)
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
- !$omp declare target to (blk5_to)
+ !$omp declare target enter (blk5_to)
!ERROR: A variable in a DECLARE TARGET directive cannot be an element of a common block
- !$omp declare target to (a5_to)
+ !$omp declare target enter (a5_to)
!ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
!$omp declare target link (arr5_link)
diff --git a/flang/test/Semantics/OpenMP/declare-target06.f90 b/flang/test/Semantics/OpenMP/declare-target06.f90
index a2d8263a60294ba..ea29c4b05810ab3 100644
--- a/flang/test/Semantics/OpenMP/declare-target06.f90
+++ b/flang/test/Semantics/OpenMP/declare-target06.f90
@@ -16,7 +16,7 @@ module test_0
!ERROR: The given DECLARE TARGET directive clause has an invalid argument
!ERROR: No explicit type declared for 'no_implicit_materialization_3'
-!$omp declare target to(no_implicit_materialization_3)
+!$omp declare target enter(no_implicit_materialization_3)
INTEGER :: data_int = 10
!$omp declare target(data_int)
diff --git a/flang/test/Semantics/OpenMP/requires04.f90 b/flang/test/Semantics/OpenMP/requires04.f90
index ae1c2c08aa1c7ec..b8d4002cbe7d9e7 100644
--- a/flang/test/Semantics/OpenMP/requires04.f90
+++ b/flang/test/Semantics/OpenMP/requires04.f90
@@ -5,7 +5,8 @@
! device constructs, such as declare target with device_type=nohost|any.
subroutine f
- !$omp declare target device_type(nohost)
+ integer, save :: x
+ !$omp declare target enter(x) device_type(nohost)
end subroutine f
subroutine g
diff --git a/flang/test/Semantics/OpenMP/requires05.f90 b/flang/test/Semantics/OpenMP/requires05.f90
index 9281f8bab4a1694..149d646d2e3af97 100644
--- a/flang/test/Semantics/OpenMP/requires05.f90
+++ b/flang/test/Semantics/OpenMP/requires05.f90
@@ -5,7 +5,7 @@
! device constructs, such as declare target with 'to' clause and no device_type.
subroutine f
- !$omp declare target to(f)
+ !$omp declare target enter(f)
end subroutine f
subroutine g
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index f8b3b0c7524979b..8bfacd85864c062 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -289,6 +289,9 @@ def OMPC_To : Clause<"to"> {
let clangClass = "OMPToClause";
let flangClass = "OmpObjectList";
}
+def OMPC_Enter : Clause<"enter"> {
+ let flangClass = "OmpObjectList";
+}
def OMPC_From : Clause<"from"> {
let clangClass = "OMPFromClause";
let flangClass = "OmpObjectList";
@@ -1124,7 +1127,8 @@ def OMP_DeclareTarget : Directive<"declare target"> {
let allowedClauses = [
VersionedClause<OMPC_To>,
VersionedClause<OMPC_Link>,
- VersionedClause<OMPC_Indirect>
+ VersionedClause<OMPC_Indirect>,
+ VersionedClause<OMPC_Enter, 52>
];
let allowedOnceClauses = [
VersionedClause<OMPC_DeviceType, 50>
|
|
LGTM, but likely a good idea to wait on one more reviewer. However, I would perhaps change the name and commit message of the PR to indicate that this patch also adds the Enter clause to declare target! I'll also point out that it likely won't work in the lowering the same way as To currently does (perhaps there's another PR for this that I've managed to miss, sorry if that's the case), as there's no handling of it, so OpenMP.cpp needs to be taught how to handle it and lower it to MLIR. Which might be a problem if we're indicating to use Enter rather than To, which has some support currently I believe (not as thoroughly tested as link though). You could probably just teach it that enter and to are the same, and treat both cases as a To, as they're synonyms. However, you can also feel free to add Enter (and probably indirect, even if there's no handling for it yet) to the MLIR omp dialects enum and then alter the MLIR -> LLVM-IR lowering to incorporate the idea of enter (would likely just be change enter -> to or just add an enter OR to for everywhere to shows up currently). Not the biggest deal but might be a bit of an issue if we're actively warning users to use the other that has less support. |
agozillon
approved these changes
Nov 10, 2023
shraiysh
force-pushed
the
declare_target_semantics1
branch
from
bb2cf5f
to
4b893d2
Compare
|
I have added tests for lowering - this ensures behavior similar to the |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This patch adds the following check from OpenMP 5.2. ``` If the directive has a clause, it must contain at least one enter clause or at least one link clause. ``` Also added a warning for the deprication of `TO` clause on `DECLARE TARGET` construct. ``` The clause-name to may be used as a synonym for the clause-name enter. This use has been deprecated. ```
shraiysh
force-pushed
the
declare_target_semantics1
branch
from
4b893d2
to
52fe8f4
Compare
|
This commit seems to be causing the test
If you cannot fix this quickly, can you please revert while you investigate so that the buildbots can get caught back up? |
shraiysh
added a commit
that referenced
this pull request
Nov 17, 2023
|
Reverted it. Thank you for pointing this out. Sorry for the trouble 🙇 |
|
I am not too familiar with the clang side of things, but the problem is here - |
sr-tream
pushed a commit
to sr-tream/llvm-project
that referenced
this pull request
Nov 20, 2023
This patch adds the following check from OpenMP 5.2. ``` If the directive has a clause, it must contain at least one enter clause or at least one link clause. ``` Also added a warning for the deprication of `TO` clause on `DECLARE TARGET` construct. ``` The clause-name to may be used as a synonym for the clause-name enter. This use has been deprecated. ``` Based on the tests for to clause, the tests for enter clause are added. This patch does not add tests where both to and enter clause are used together.
sr-tream
pushed a commit
to sr-tream/llvm-project
that referenced
this pull request
Nov 20, 2023
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
This patch adds the following check from OpenMP 5.2. ``` If the directive has a clause, it must contain at least one enter clause or at least one link clause. ``` Also added a warning for the deprication of `TO` clause on `DECLARE TARGET` construct. ``` The clause-name to may be used as a synonym for the clause-name enter. This use has been deprecated. ``` Based on the tests for to clause, the tests for enter clause are added. This patch does not add tests where both to and enter clause are used together.
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds the following check from OpenMP 5.2.
Also added a warning for the deprication of
TOclause onDECLARE TARGETconstruct.