[flang][OpenMP] Fix parsing of ASSUME directive #155257
Merged
Conversation
This file contains hidden or 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
The ASSUME directive is block-associated and whether the end-directive is optional or not depends on the form of the block. This is all taken care of automatically since the AST node for ASSUME inherits from OmpBlockConstruct.
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-parser Author: Krzysztof Parzyszek (kparzysz) ChangesThe ASSUME directive is block-associated and whether the end-directive is optional or not depends on the form of the block. This is all taken care of automatically since the AST node for ASSUME inherits from OmpBlockConstruct. Full diff: https://github.com/llvm/llvm-project/pull/155257.diff 8 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 7170dfb591fae..29ae376711f51 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -517,7 +517,6 @@ class ParseTreeDumper {
NODE(OmpAppendArgsClause, OmpAppendOp)
NODE(parser, OmpArgument)
NODE(parser, OmpArgumentList)
- NODE(parser, OmpAssumeDirective)
NODE(parser, OmpAtClause)
NODE_ENUM(OmpAtClause, ActionTime)
NODE(parser, OmpAtomicDefaultMemOrderClause)
@@ -571,7 +570,6 @@ class ParseTreeDumper {
NODE(parser, OmpDoacrossClause)
NODE(parser, OmpDynGroupprivateClause)
NODE(OmpDynGroupprivateClause, Modifier)
- NODE(parser, OmpEndAssumeDirective)
NODE(parser, OmpEndCriticalDirective)
NODE(parser, OmpEndDirective)
NODE(parser, OmpEndLoopDirective)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 6bd578b50dc3e..cbcf7f4328fd4 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -38,7 +38,6 @@ struct ConstructId {
static constexpr llvm::omp::Directive id{Id}; \
}
-MAKE_CONSTR_ID(OmpAssumeDirective, D::OMPD_assume);
MAKE_CONSTR_ID(OmpCriticalDirective, D::OMPD_critical);
MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant);
MAKE_CONSTR_ID(OmpErrorDirective, D::OMPD_error);
@@ -104,8 +103,7 @@ struct DirectiveNameScope {
} else if constexpr (TupleTrait<T>) {
if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
return std::get<OmpBeginDirective>(x.t).DirName();
- } else if constexpr (std::is_same_v<T, OmpAssumeDirective> ||
- std::is_same_v<T, OmpCriticalDirective> ||
+ } else if constexpr (std::is_same_v<T, OmpCriticalDirective> ||
std::is_same_v<T, OmpDeclareVariantDirective> ||
std::is_same_v<T, OmpErrorDirective> ||
std::is_same_v<T, OmpMetadirectiveDirective> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 1d1a4a163084b..e5f4c57c976f1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4835,28 +4835,14 @@ struct OpenMPDeclarativeAssumes {
CharBlock source;
};
-struct OmpAssumeDirective {
- TUPLE_CLASS_BOILERPLATE(OmpAssumeDirective);
- std::tuple<Verbatim, OmpClauseList> t;
- CharBlock source;
-};
-
-struct OmpEndAssumeDirective {
- WRAPPER_CLASS_BOILERPLATE(OmpEndAssumeDirective, Verbatim);
- CharBlock source;
-};
-
-// Ref: [5.2: 213-216]
+// Ref: [5.1:86-89], [5.2:215], [6.0:369]
//
-// assume-construct ->
-// ASSUME absent-clause | contains-clause | holds_clause | no-openmp-clause
-// no-openmp-routines-clause | no-parallelism-clause
-// block
+// assume-directive -> // since 5.1
+// ASSUME assumption-clause...
+// block
// [END ASSUME]
-struct OpenMPAssumeConstruct {
- TUPLE_CLASS_BOILERPLATE(OpenMPAssumeConstruct);
- std::tuple<OmpAssumeDirective, Block, std::optional<OmpEndAssumeDirective>> t;
- CharBlock source;
+struct OpenMPAssumeConstruct : public OmpBlockConstruct {
+ INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPAssumeConstruct, OmpBlockConstruct);
};
// 2.7.2 SECTIONS
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 51b49a591b02f..ac7cc2e6877e6 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1843,16 +1843,8 @@ TYPE_PARSER(
Parser<OmpMetadirectiveDirective>{})) /
endOmpLine))
-// Assume Construct
-TYPE_PARSER(sourced(construct<OmpAssumeDirective>(
- verbatim("ASSUME"_tok), Parser<OmpClauseList>{})))
-
-TYPE_PARSER(sourced(construct<OmpEndAssumeDirective>(
- startOmpLine >> verbatim("END ASSUME"_tok))))
-
-TYPE_PARSER(sourced(
- construct<OpenMPAssumeConstruct>(Parser<OmpAssumeDirective>{} / endOmpLine,
- block, maybe(Parser<OmpEndAssumeDirective>{} / endOmpLine))))
+TYPE_PARSER(construct<OpenMPAssumeConstruct>(
+ sourced(OmpBlockConstructParser{llvm::omp::Directive::OMPD_assume})))
// Block Construct
#define MakeBlockConstruct(dir) \
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 09dcfe60a46bc..bae9207eab97d 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2580,20 +2580,11 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
- void Unparse(const OpenMPAllocatorsConstruct &x) { //
+ void Unparse(const OpenMPAllocatorsConstruct &x) {
Unparse(static_cast<const OmpBlockConstruct &>(x));
}
- void Unparse(const OmpAssumeDirective &x) {
- BeginOpenMP();
- Word("!$OMP ASSUME");
- Walk(" ", std::get<OmpClauseList>(x.t).v);
- Put("\n");
- EndOpenMP();
- }
- void Unparse(const OmpEndAssumeDirective &x) {
- BeginOpenMP();
- Word("!$OMP END ASSUME\n");
- EndOpenMP();
+ void Unparse(const OpenMPAssumeConstruct &x) {
+ Unparse(static_cast<const OmpBlockConstruct &>(x));
}
void Unparse(const OmpCriticalDirective &x) {
BeginOpenMP();
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 6dad9a3d0711d..bd9b4c207e22d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -589,14 +589,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
checker_(GetDirName(x.t).source, Directive::OMPD_allocators);
return false;
}
- bool Pre(const parser::OmpAssumeDirective &x) {
- checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assume);
- return false;
- }
- bool Pre(const parser::OmpEndAssumeDirective &x) {
- checker_(x.v.source, Directive::OMPD_assume);
- return false;
- }
bool Pre(const parser::OmpMetadirectiveDirective &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_metadirective);
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index dbd4f512a0465..9c4d3a8738ee5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -531,6 +531,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
bool Pre(const parser::OpenMPDeclarativeAllocate &);
void Post(const parser::OpenMPDeclarativeAllocate &) { PopContext(); }
+ bool Pre(const parser::OpenMPAssumeConstruct &);
+ void Post(const parser::OpenMPAssumeConstruct &) { PopContext(); }
+
bool Pre(const parser::OpenMPAtomicConstruct &);
void Post(const parser::OpenMPAtomicConstruct &) { PopContext(); }
@@ -2220,6 +2223,11 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclarativeAllocate &x) {
return false;
}
+bool OmpAttributeVisitor::Pre(const parser::OpenMPAssumeConstruct &x) {
+ PushContext(x.source, llvm::omp::Directive::OMPD_assume);
+ return true;
+}
+
bool OmpAttributeVisitor::Pre(const parser::OpenMPAtomicConstruct &x) {
PushContext(x.source, llvm::omp::Directive::OMPD_atomic);
return true;
diff --git a/flang/test/Parser/OpenMP/assumption.f90 b/flang/test/Parser/OpenMP/assumption.f90
index f1cb0c87e1262..ffd071fd69659 100644
--- a/flang/test/Parser/OpenMP/assumption.f90
+++ b/flang/test/Parser/OpenMP/assumption.f90
@@ -1,59 +1,141 @@
-! RUN: %flang_fc1 -fopenmp-version=51 -fopenmp -fdebug-unparse-no-sema %s 2>&1 | FileCheck %s
-! RUN: %flang_fc1 -fopenmp-version=51 -fopenmp -fdebug-dump-parse-tree-no-sema %s 2>&1 | FileCheck %s --check-prefix="PARSE-TREE"
+!RUN: %flang_fc1 -fopenmp-version=51 -fopenmp -fdebug-unparse-no-sema %s | FileCheck --check-prefix="UNPARSE" %s
+!RUN: %flang_fc1 -fopenmp-version=51 -fopenmp -fdebug-dump-parse-tree-no-sema %s | FileCheck --check-prefix="PARSE-TREE" %s
+
subroutine sub1
integer :: r
-!CHECK: !$OMP ASSUME NO_OPENMP
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> NoOpenmp
!$omp assume no_openmp
-!CHECK: !$OMP ASSUME NO_PARALLELISM
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> NoParallelism
+ !$omp end assume
+
!$omp assume no_parallelism
-!CHECK: !$OMP ASSUME NO_OPENMP_ROUTINES
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> NoOpenmpRoutines
+ !$omp end assume
+
!$omp assume no_openmp_routines
-!CHECK: !$OMP ASSUME ABSENT(ALLOCATE), CONTAINS(WORKSHARE,TASK)
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> Absent -> OmpAbsentClause -> llvm::omp::Directive = allocate
-!PARSE-TREE: OmpClause -> Contains -> OmpContainsClause -> llvm::omp::Directive = workshare
-!PARSE-TREE: llvm::omp::Directive = task
- !$omp assume absent(allocate), contains(workshare, task)
-!CHECK: !$OMP ASSUME HOLDS(1==1)
+ !$omp end assume
+
+ !$omp assume absent(allocate), contains(workshare, task)
+ !$omp end assume
+
!$omp assume holds(1.eq.1)
+ !$omp end assume
print *, r
end subroutine sub1
+!UNPARSE: SUBROUTINE sub1
+!UNPARSE: INTEGER r
+!UNPARSE: !$OMP ASSUME NO_OPENMP
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: !$OMP ASSUME NO_PARALLELISM
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: !$OMP ASSUME NO_OPENMP_ROUTINES
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: !$OMP ASSUME ABSENT(ALLOCATE) CONTAINS(WORKSHARE,TASK)
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: !$OMP ASSUME HOLDS(1==1)
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: PRINT *, r
+!UNPARSE: END SUBROUTINE sub1
+
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> NoOpenmp
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> NoParallelism
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> NoOpenmpRoutines
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> Absent -> OmpAbsentClause -> llvm::omp::Directive = allocate
+!PARSE-TREE: | | OmpClause -> Contains -> OmpContainsClause -> llvm::omp::Directive = workshare
+!PARSE-TREE: | | llvm::omp::Directive = task
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> Holds -> OmpHoldsClause -> Expr -> EQ
+!PARSE-TREE: | | | Expr -> LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | | Expr -> LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+
+
subroutine sub2
integer :: r
integer :: v
-!CHECK !$OMP ASSUME NO_OPENMP
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
-!PARSE-TREE: OmpAssumeDirective
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> NoOpenmp
-!PARSE-TREE: Block
-!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt
-!PARSE-TREE: Expr -> Add
-!PARSE-TREE: OmpEndAssumeDirective
v = 87
!$omp assume no_openmp
r = r + 1
-!CHECK !$OMP END ASSUME
!$omp end assume
end subroutine sub2
-
+
+!UNPARSE: SUBROUTINE sub2
+!UNPARSE: INTEGER r
+!UNPARSE: INTEGER v
+!UNPARSE: v = 87
+!UNPARSE: !$OMP ASSUME NO_OPENMP
+!UNPARSE: r = r+1
+!UNPARSE: !$OMP END ASSUME
+!UNPARSE: END SUBROUTINE sub2
+
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt
+!PARSE-TREE: | Variable -> Designator -> DataRef -> Name = 'v'
+!PARSE-TREE: | Expr -> LiteralConstant -> IntLiteralConstant = '87'
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct
+!PARSE-TREE: | OmpBeginDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> NoOpenmp
+!PARSE-TREE: | | Flags = None
+!PARSE-TREE: | Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt
+!PARSE-TREE: | | | Variable -> Designator -> DataRef -> Name = 'r'
+!PARSE-TREE: | | | Expr -> Add
+!PARSE-TREE: | | | | Expr -> Designator -> DataRef -> Name = 'r'
+!PARSE-TREE: | | | | Expr -> LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | OmpEndDirective
+!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = assume
+!PARSE-TREE: | | OmpClauseList ->
+!PARSE-TREE: | | Flags = None
+
program p
-!CHECK !$OMP ASSUMES NO_OPENMP
-!PARSE-TREE: SpecificationPart
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes
-!PARSE-TREE: Verbatim
-!PARSE-TREE: OmpClauseList -> OmpClause -> NoOpenmp
!$omp assumes no_openmp
end program p
-
+
+!UNPARSE: PROGRAM p
+!UNPARSE: !$OMP ASSUMES NO_OPENMP
+!UNPARSE: END PROGRAM p
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes
+!PARSE-TREE: | Verbatim
+!PARSE-TREE: | OmpClauseList -> OmpClause -> NoOpenmp
|
tblah
reviewed
Aug 26, 2025
| !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAssumeConstruct | ||
| !PARSE-TREE: Verbatim | ||
| !PARSE-TREE: OmpClauseList -> OmpClause -> NoParallelism | ||
| !$omp end assume |
Contributor
There was a problem hiding this comment.
Is this now testing slightly different code? Perhaps these could be adapted so that each example takes a different allowed form (e.g. loosely structured, strictly structured, ...)?
Contributor
Author
There was a problem hiding this comment.
The original code in the testcase was not legal. It had a loosely structured block (i.e. empty list of statements) without the end-directive.
Contributor
Author
There was a problem hiding this comment.
Added strictly structured blocks without end-directive in a couple of places.
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.
The ASSUME directive is block-associated and whether the end-directive is optional or not depends on the form of the block. This is all taken care of automatically since the AST node for ASSUME inherits from OmpBlockConstruct.