-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[Flang][OpenMP] Add Parsing support for Indirect Clause #143505
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Jack Styles (Stylie777) ChangesAs part of OpenMP Version 5.1, support for the This adds Parsing support for the clause, along with semantics checks. Currently, lowering for the clause is not supported so a TODO message will be outputted to the user. It also performs version checking as See also: #110008 Full diff: https://github.com/llvm/llvm-project/pull/143505.diff 12 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index df9278697346f..64228c2295151 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -574,6 +574,7 @@ class ParseTreeDumper {
NODE_ENUM(OmpDependenceType, Value)
NODE(parser, OmpTaskDependenceType)
NODE_ENUM(OmpTaskDependenceType, Value)
+ NODE(parser, OmpIndirectClause)
NODE(parser, OmpIterationOffset)
NODE(parser, OmpIteration)
NODE(parser, OmpIterationVector)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index c99006f0c1c22..bce2606029c4d 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4300,6 +4300,11 @@ struct OmpHoldsClause {
WRAPPER_CLASS_BOILERPLATE(OmpHoldsClause, common::Indirection<Expr>);
};
+// Ref: [5.2: 209]
+struct OmpIndirectClause {
+ WRAPPER_CLASS_BOILERPLATE(OmpIndirectClause, std::optional<ScalarLogicalExpr>);
+};
+
// Ref: [5.2:72-73], in 4.5-5.1 it's scattered over individual directives
// that allow the IF clause.
//
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index f3088b18b77ff..c628de208d3c7 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -906,8 +906,8 @@ Inclusive make(const parser::OmpClause::Inclusive &inp,
Indirect make(const parser::OmpClause::Indirect &inp,
semantics::SemanticsContext &semaCtx) {
- // inp -> empty
- llvm_unreachable("Empty: indirect");
+ // inp.v.v -> std::optional<parser::ScalarLogicalExpr>
+ return Indirect{maybeApply(makeExprFn(semaCtx), inp.v.v)};
}
Init make(const parser::OmpClause::Init &inp,
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 08326fad8c143..e7bc1c66b458c 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -996,6 +996,8 @@ TYPE_PARSER( //
"IF" >> construct<OmpClause>(construct<OmpClause::If>(
parenthesized(Parser<OmpIfClause>{}))) ||
"INBRANCH" >> construct<OmpClause>(construct<OmpClause::Inbranch>()) ||
+ "INDIRECT" >> construct<OmpClause>(construct<OmpClause::Indirect>(
+ maybe(parenthesized(scalarLogicalExpr)))) ||
"INIT" >> construct<OmpClause>(construct<OmpClause::Init>(
parenthesized(Parser<OmpInitClause>{}))) ||
"INCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Inclusive>(
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 76dfd40c6a62c..ef3a7689a73ce 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1811,15 +1811,24 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) {
const parser::OmpClause *toClause = FindClause(llvm::omp::Clause::OMPC_to);
const parser::OmpClause *linkClause =
FindClause(llvm::omp::Clause::OMPC_link);
+ const parser::OmpClause *indirectClause = FindClause(llvm::omp::Clause::OMPC_indirect);
if (!enterClause && !toClause && !linkClause) {
context_.Say(x.source,
"If the DECLARE TARGET directive has a clause, it must contain at least one ENTER clause or LINK clause"_err_en_US);
}
+ if (indirectClause && !enterClause) {
+ context_.Say(x.source,
+ "The INDIRECT clause cannot be used without the ENTER clause with the DECLARE TARGET directive."_err_en_US);
+ }
unsigned version{context_.langOptions().OpenMPVersion};
if (toClause && version >= 52) {
context_.Warn(common::UsageWarning::OpenMPUsage, toClause->source,
"The usage of TO clause on DECLARE TARGET directive has been deprecated. Use ENTER clause instead."_warn_en_US);
}
+ if(indirectClause && version < 51) {
+ context_.Say(x.source,
+ "The usage of INDIRECT clause on DECLARE TARGET directive is only supported on OpenMP Version 5.1 or greater"_err_en_US);
+ }
}
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 1a8059d8548ed..a964124beb0f9 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -243,6 +243,7 @@ class OmpStructureChecker
void CheckSymbolNames(
const parser::CharBlock &source, const parser::OmpObjectList &objList);
void CheckIntentInPointer(SymbolSourceMap &, const llvm::omp::Clause);
+ void MakeScalarLogicalTrue(const parser::OmpClause *clause);
void CheckProcedurePointer(SymbolSourceMap &, const llvm::omp::Clause);
void CheckCrayPointee(const parser::OmpObjectList &objectList,
llvm::StringRef clause, bool suggestToUseCrayPointer = true);
diff --git a/flang/test/Lower/OpenMP/Todo/omp-clause-indirect.f90 b/flang/test/Lower/OpenMP/Todo/omp-clause-indirect.f90
new file mode 100644
index 0000000000000..d441cac47f5da
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/omp-clause-indirect.f90
@@ -0,0 +1,34 @@
+! This test checks the lowering of OpenMP Indirect Clause when used with the Declare Target directive
+
+! RUN: not flang -fc1 -emit-fir -fopenmp -fopenmp-version=52 %s 2>&1 | FileCheck %s
+
+module functions
+ implicit none
+
+ interface
+ function func() result(i)
+ character(1) :: i
+ end function
+ end interface
+
+contains
+ function func1() result(i)
+ !CHECK: not yet implemented: Unhandled clause INDIRECT in DECLARE TARGET construct
+ !$omp declare target enter(func1) indirect(.true.)
+ character(1) :: i
+ i = 'a'
+ return
+ end function
+end module
+
+program main
+ use functions
+ implicit none
+ procedure (func), pointer :: ptr1=>func1
+ character(1) :: val1
+
+ !$omp target map(from: val1)
+ val1 = ptr1()
+ !$omp end target
+
+end program
diff --git a/flang/test/Parser/OpenMP/declare-target-indirect-tree.f90 b/flang/test/Parser/OpenMP/declare-target-indirect-tree.f90
new file mode 100644
index 0000000000000..b76b7eb49e418
--- /dev/null
+++ b/flang/test/Parser/OpenMP/declare-target-indirect-tree.f90
@@ -0,0 +1,53 @@
+! REQUIRES: openmp_runtime
+
+! RUN: %flang_fc1 %openmp_flags -fopenmp-version=52 -fdebug-dump-parse-tree %s | FileCheck %s
+! RUN: %flang_fc1 %openmp_flags -fdebug-unparse -fopenmp-version=52 %s | FileCheck %s --check-prefix="UNPARSE"
+
+module functions
+ implicit none
+
+ interface
+ function func() result(i)
+ character(1) :: i
+ end function
+ end interface
+
+contains
+ function func1() result(i)
+ !$omp declare target enter(func1) indirect(.true.)
+ !CHECK: | | | | | OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> Enter -> OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'func1'
+ !CHECK-NEXT: | | | | | OmpClause -> Indirect -> OmpIndirectClause -> Scalar -> Logical -> Expr = '.true._4'
+ !CHECK-NEXT: | | | | | | LiteralConstant -> LogicalLiteralConstant
+ !CHECK-NEXT: | | | | | | | bool = 'true'
+ character(1) :: i
+ i = 'a'
+ return
+ end function
+
+ function func2() result(i)
+ !$omp declare target enter(func2) indirect
+ !CHECK: | | | | | OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> Enter -> OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'func2'
+ !CHECK-NEXT: | | | | | OmpClause -> Indirect -> OmpIndirectClause ->
+ character(1) :: i
+ i = 'b'
+ return
+ end function
+end module
+
+program main
+ use functions
+ implicit none
+ procedure (func), pointer :: ptr1=>func1, ptr2=>func2
+ character(1) :: val1, val2
+
+ !$omp target map(from: val1)
+ val1 = ptr1()
+ !$omp end target
+ !$omp target map(from: val2)
+ val2 = ptr2()
+ !$omp end target
+
+end program
+
+!UNPARSE: !$OMP DECLARE TARGET ENTER(func1) INDIRECT(.true._4)
+!UNPARSE: !$OMP DECLARE TARGET ENTER(func2) INDIRECT()
\ No newline at end of file
diff --git a/flang/test/Semantics/indirect01.f90 b/flang/test/Semantics/indirect01.f90
new file mode 100644
index 0000000000000..59850662275d9
--- /dev/null
+++ b/flang/test/Semantics/indirect01.f90
@@ -0,0 +1,34 @@
+! This test checks the lowering of OpenMP Indirect Clause when used with the Declare Target directive
+
+! RUN: not flang -fopenmp -fopenmp-version=52 %s 2>&1 | FileCheck %s
+
+module functions
+ implicit none
+
+ interface
+ function func() result(i)
+ character(1) :: i
+ end function
+ end interface
+
+contains
+ function func1() result(i)
+ !CHECK: The INDIRECT clause cannot be used without the ENTER clause with the DECLARE TARGET directive.
+ !$omp declare target indirect(.true.)
+ character(1) :: i
+ i = 'a'
+ return
+ end function
+end module
+
+program main
+ use functions
+ implicit none
+ procedure (func), pointer :: ptr1=>func1
+ character(1) :: val1
+
+ !$omp target map(from: val1)
+ val1 = ptr1()
+ !$omp end target
+
+end program
diff --git a/flang/test/Semantics/indirect02.f90 b/flang/test/Semantics/indirect02.f90
new file mode 100644
index 0000000000000..1b8b7dcadecf4
--- /dev/null
+++ b/flang/test/Semantics/indirect02.f90
@@ -0,0 +1,36 @@
+! This test checks the lowering of OpenMP Indirect Clause when used with the Declare Target directive
+
+! RUN: not flang -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s --check-prefix="CHECK-50"
+! RUN: not flang -fopenmp -fopenmp-version=52 %s 2>&1 | FileCheck %s --check-prefix="CHECK-52"
+
+module functions
+ implicit none
+
+ interface
+ function func() result(i)
+ character(1) :: i
+ end function
+ end interface
+
+contains
+ function func1() result(i)
+ !CHECK-50: The usage of INDIRECT clause on DECLARE TARGET directive is only supported on OpenMP Version 5.1 or greater
+ !CHECK-52: not yet implemented: Unhandled clause INDIRECT in DECLARE TARGET construct
+ !$omp declare target enter(func1) indirect(.true.)
+ character(1) :: i
+ i = 'a'
+ return
+ end function
+end module
+
+program main
+ use functions
+ implicit none
+ procedure (func), pointer :: ptr1=>func1
+ character(1) :: val1
+
+ !$omp target map(from: val1)
+ val1 = ptr1()
+ !$omp end target
+
+end program
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index e0714e812e5cd..de888ff86fe91 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -701,7 +701,7 @@ template <typename T, typename I, typename E> //
struct IndirectT {
using InvokedByFptr = E;
using WrapperTrait = std::true_type;
- InvokedByFptr v;
+ OPT(InvokedByFptr) v;
};
// V5.2: [14.1.2] `init` clause
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 0af4b436649a3..e091e7f7fe81f 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -244,6 +244,7 @@ def OMPC_Inclusive : Clause<"inclusive"> {
let flangClass = "OmpObjectList";
}
def OMPC_Indirect : Clause<"indirect"> {
+ let flangClass = "OmpIndirectClause";
}
def OMPC_Init : Clause<"init"> {
let clangClass = "OMPInitClause";
|
As part of OpenMP Version 5.1, support for the `indirect` clause was added for the `declare target` directive. This clause should follow an `enter` clause, and allows procedure calls to be done indirectly through OpenMP. This adds Parsing support for the clause, along with semantics checks. Currently, lowering for the clause is not supported so a TODO message will be outputted to the user. It also performs version checking as `indirect` is only support in OpenMP 5.1 or greater. See also: llvm#110008
bdfd0b9
to
5197006
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
unsigned version{context_.langOptions().OpenMPVersion}; | ||
if (toClause && version >= 52) { | ||
context_.Warn(common::UsageWarning::OpenMPUsage, toClause->source, | ||
"The usage of TO clause on DECLARE TARGET directive has been deprecated. Use ENTER clause instead."_warn_en_US); | ||
} | ||
if (indirectClause && version < 51) { | ||
context_.Say(x.source, | ||
"The usage of INDIRECT clause on DECLARE TARGET directive is only supported on OpenMP Version 5.1 or greater"_err_en_US); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use a slightly different form of "feature not supported in version xyz" messages, see lines 278-281 for example. The helper functions ThisVersion and TryVersion generate parts of the message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe CheckAllowedClause could be used for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this so that If an indirect clause is used, it will ensure it is allowed.
@@ -243,6 +243,7 @@ class OmpStructureChecker | |||
void CheckSymbolNames( | |||
const parser::CharBlock &source, const parser::OmpObjectList &objList); | |||
void CheckIntentInPointer(SymbolSourceMap &, const llvm::omp::Clause); | |||
void MakeScalarLogicalTrue(const parser::OmpClause *clause); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, this is indeed unused and should have been removed. I missed this.
The following has been completed: - Removed unneeded function declartion - Updated logic for version checking to use the `CheckAllowedClause` function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks. But please wait for approval from @kparzysz
As part of OpenMP Version 5.1, support for the
indirect
clause was added for thedeclare target
directive. This clause should follow anenter
clause, and allows procedure calls to be done indirectly through OpenMP.This adds Parsing support for the clause, along with semantics checks. Currently, lowering for the clause is not supported so a TODO message will be outputted to the user. It also performs version checking as
indirect
is only support in OpenMP 5.1 or greater.See also: #110008