-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_SIMD #160390
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
Conversation
…sing The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it. However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages). To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier.
|
@llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesFull diff: https://github.com/llvm/llvm-project/pull/160390.diff 9 Files Affected:
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 64be9714f6cc2..e17728b5413e2 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -41,7 +41,6 @@ struct ConstructId {
MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
-MAKE_CONSTR_ID(OpenMPDeclareSimdConstruct, D::OMPD_declare_simd);
MAKE_CONSTR_ID(OpenMPDeclareTargetConstruct, D::OMPD_declare_target);
MAKE_CONSTR_ID(OpenMPExecutableAllocate, D::OMPD_allocate);
MAKE_CONSTR_ID(OpenMPRequiresConstruct, D::OMPD_requires);
@@ -94,7 +93,6 @@ struct DirectiveNameScope {
} else if constexpr (std::is_same_v<T, OpenMPDeclarativeAllocate> ||
std::is_same_v<T, OpenMPDeclarativeAssumes> ||
std::is_same_v<T, OpenMPDeclareReductionConstruct> ||
- std::is_same_v<T, OpenMPDeclareSimdConstruct> ||
std::is_same_v<T, OpenMPDeclareTargetConstruct> ||
std::is_same_v<T, OpenMPExecutableAllocate> ||
std::is_same_v<T, OpenMPRequiresConstruct>) {
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index de65088c01eae..be30a95763208 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4987,9 +4987,9 @@ struct OpenMPDeclareReductionConstruct {
// 2.8.2 declare-simd -> DECLARE SIMD [(proc-name)] [declare-simd-clause[ [,]
// declare-simd-clause]...]
struct OpenMPDeclareSimdConstruct {
- TUPLE_CLASS_BOILERPLATE(OpenMPDeclareSimdConstruct);
+ WRAPPER_CLASS_BOILERPLATE(
+ OpenMPDeclareSimdConstruct, OmpDirectiveSpecification);
CharBlock source;
- std::tuple<Verbatim, std::optional<Name>, OmpClauseList> t;
};
// ref: [6.0:301-303]
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 6ec6eb4038933..0085576292ff5 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1836,8 +1836,9 @@ TYPE_PARSER(
// 2.8.2 Declare Simd construct
TYPE_PARSER(sourced(construct<OpenMPDeclareSimdConstruct>(
- verbatim("DECLARE SIMD"_tok) || verbatim("DECLARE_SIMD"_tok),
- maybe(parenthesized(name)), Parser<OmpClauseList>{})))
+ predicated(Parser<OmpDirectiveName>{},
+ IsDirective(llvm::omp::Directive::OMPD_declare_simd)) >=
+ Parser<OmpDirectiveSpecification>{})))
TYPE_PARSER(sourced( //
construct<OpenMPGroupprivate>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index fc81cfb7a3818..c9774dd137d2b 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2573,11 +2573,10 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
- void Unparse(const OpenMPDeclareSimdConstruct &y) {
+ void Unparse(const OpenMPDeclareSimdConstruct &x) {
BeginOpenMP();
- Word("!$OMP DECLARE SIMD ");
- Walk("(", std::get<std::optional<Name>>(y.t), ")");
- Walk(std::get<OmpClauseList>(y.t));
+ Word("!$OMP ");
+ Walk(x.v);
Put("\n");
EndOpenMP();
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9b1932255bd05..55aa5a0a9f54d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -624,11 +624,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
return false;
}
- bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
- checker_(
- std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_simd);
- return false;
- }
bool Pre(const parser::OpenMPDeclareTargetConstruct &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_target);
@@ -1356,8 +1351,33 @@ void OmpStructureChecker::Leave(const parser::OpenMPThreadprivate &x) {
}
void OmpStructureChecker::Enter(const parser::OpenMPDeclareSimdConstruct &x) {
- const auto &dir{std::get<parser::Verbatim>(x.t)};
- PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_declare_simd);
+ const parser::OmpDirectiveName &dirName{x.v.DirName()};
+ PushContextAndClauseSets(dirName.source, dirName.v);
+
+ const parser::OmpArgumentList &args{x.v.Arguments()};
+ if (args.v.empty()) {
+ return;
+ } else if (args.v.size() > 1) {
+ context_.Say(args.source,
+ "DECLARE_SIMD directive should have at most one argument"_err_en_US);
+ return;
+ }
+
+ const parser::OmpArgument &arg{args.v.front()};
+ if (auto *sym{GetArgumentSymbol(arg)}) {
+ if (!IsProcedure(*sym) && !IsFunction(*sym)) {
+ context_.Say(arg.source,
+ "The name '%s' should refer to a procedure"_err_en_US, sym->name());
+ }
+ if (sym->test(Symbol::Flag::Implicit)) {
+ context_.Say(arg.source,
+ "The name '%s' has been implicitly declared"_err_en_US,
+ sym->name());
+ }
+ } else {
+ context_.Say(arg.source,
+ "The argument to the DECLARE_SIMD directive should be a procedure name"_err_en_US);
+ }
}
void OmpStructureChecker::Leave(const parser::OpenMPDeclareSimdConstruct &) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index f1f78620532f5..218e3e7266ca9 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -473,9 +473,10 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
PushContext(x.source, llvm::omp::Directive::OMPD_declare_simd);
- const auto &name{std::get<std::optional<parser::Name>>(x.t)};
- if (name) {
- ResolveOmpName(*name, Symbol::Flag::OmpDeclareSimd);
+ for (const parser::OmpArgument &arg : x.v.Arguments().v) {
+ if (auto *object{omp::GetArgumentObject(arg)}) {
+ ResolveOmpObject(*object, Symbol::Flag::OmpDeclareSimd);
+ }
}
return true;
}
diff --git a/flang/test/Parser/OpenMP/linear-clause.f90 b/flang/test/Parser/OpenMP/linear-clause.f90
index 5ea31ce58fc5a..b53dfe5f941a3 100644
--- a/flang/test/Parser/OpenMP/linear-clause.f90
+++ b/flang/test/Parser/OpenMP/linear-clause.f90
@@ -84,18 +84,16 @@ subroutine f03(x)
!UNPARSE: SUBROUTINE f03 (x)
!UNPARSE: INTEGER x
-!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL)
+!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL)
!UNPARSE: END SUBROUTINE
-!PARSE-TREE: SpecificationPart
-![...]
-!PARSE-TREE: | DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct
-!PARSE-TREE: | | Verbatim
-!PARSE-TREE: | | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause
-!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
-!PARSE-TREE: | | | Modifier -> OmpLinearModifier -> Value = Uval
-!PARSE-TREE: | | | bool = 'true'
-!PARSE-TREE: ExecutionPart -> Block
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare simd
+!PARSE-TREE: | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause
+!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | Modifier -> OmpLinearModifier -> Value = Uval
+!PARSE-TREE: | | bool = 'true'
+!PARSE-TREE: | Flags = None
subroutine f04(x)
integer :: x
@@ -104,17 +102,15 @@ subroutine f04(x)
!UNPARSE: SUBROUTINE f04 (x)
!UNPARSE: INTEGER x
-!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL, STEP(3_4))
+!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL, STEP(3_4))
!UNPARSE: END SUBROUTINE
-!PARSE-TREE: SpecificationPart
-![...]
-!PARSE-TREE: | DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct
-!PARSE-TREE: | | Verbatim
-!PARSE-TREE: | | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause
-!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
-!PARSE-TREE: | | | Modifier -> OmpLinearModifier -> Value = Uval
-!PARSE-TREE: | | | Modifier -> OmpStepComplexModifier -> Scalar -> Integer -> Expr = '3_4'
-!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '3'
-!PARSE-TREE: | | | bool = 'true'
-!PARSE-TREE: ExecutionPart -> Block
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare simd
+!PARSE-TREE: | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause
+!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | Modifier -> OmpLinearModifier -> Value = Uval
+!PARSE-TREE: | | Modifier -> OmpStepComplexModifier -> Scalar -> Integer -> Expr = '3_4'
+!PARSE-TREE: | | | LiteralConstant -> IntLiteralConstant = '3'
+!PARSE-TREE: | | bool = 'true'
+!PARSE-TREE: | Flags = None
diff --git a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
index f55ff958b0952..b72c5a2c1c086 100644
--- a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
+++ b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
@@ -111,12 +111,13 @@ subroutine f03
end
!UNPARSE: SUBROUTINE f03
-!UNPARSE: !$OMP DECLARE SIMD
+!UNPARSE: !$OMP DECLARE_SIMD
!UNPARSE: END SUBROUTINE
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct
-!PARSE-TREE: | Verbatim
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare simd
!PARSE-TREE: | OmpClauseList ->
+!PARSE-TREE: | Flags = None
subroutine f04
!$omp declare_target
diff --git a/flang/test/Semantics/OpenMP/declare-simd.f90 b/flang/test/Semantics/OpenMP/declare-simd.f90
new file mode 100644
index 0000000000000..825eb58f2adb5
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-simd.f90
@@ -0,0 +1,23 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=60
+
+module m
+
+!ERROR: The name 'x' should refer to a procedure
+!ERROR: The name 'x' has been implicitly declared
+!$omp declare_simd(x)
+
+!ERROR: DECLARE_SIMD directive should have at most one argument
+!$omp declare_simd(f00, f01)
+
+!ERROR: The argument to the DECLARE_SIMD directive should be a procedure name
+!$omp declare_simd(v : integer)
+
+contains
+
+subroutine f00
+end
+
+subroutine f01
+end
+
+end module
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
tblah
left a comment
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 with one nit
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/40447 Here is the relevant piece of the build log for the reference |
No description provided.