Skip to content

Conversation

@kparzysz
Copy link
Contributor

No description provided.

…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.
@kparzysz kparzysz requested review from Stylie777 and tblah September 24, 2025 19:43
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics flang:parser labels Sep 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2025

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

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

6 Files Affected:

  • (modified) flang/include/flang/Parser/openmp-utils.h (-2)
  • (modified) flang/include/flang/Parser/parse-tree.h (+2-2)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+4-2)
  • (modified) flang/lib/Parser/unparse.cpp (+2-2)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (-4)
  • (modified) flang/test/Parser/OpenMP/assumption.f90 (+5-3)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index bf54f970a7d3a..77c31b939e522 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -39,7 +39,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(OpenMPExecutableAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPRequiresConstruct, D::OMPD_requires);
@@ -94,7 +93,6 @@ struct DirectiveNameScope {
       if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
         return std::get<OmpBeginDirective>(x.t).DirName();
       } 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, 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 486be8b60ff8c..bd55166eb9f80 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4877,8 +4877,8 @@ struct OpenMPUtilityConstruct {
 //   ASSUMES absent-clause | contains-clause | holds-clause | no-openmp-clause |
 //          no-openmp-routines-clause | no-parallelism-clause
 struct OpenMPDeclarativeAssumes {
-  TUPLE_CLASS_BOILERPLATE(OpenMPDeclarativeAssumes);
-  std::tuple<Verbatim, OmpClauseList> t;
+  WRAPPER_CLASS_BOILERPLATE(
+      OpenMPDeclarativeAssumes, OmpDirectiveSpecification);
   CharBlock source;
 };
 
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index bd080386c0aea..12e89e8c35456 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1852,8 +1852,10 @@ TYPE_PARSER(
     lookAhead(endOmpLine / !statement(allocateStmt)))
 
 // Assumes Construct
-TYPE_PARSER(sourced(construct<OpenMPDeclarativeAssumes>(
-    verbatim("ASSUMES"_tok), Parser<OmpClauseList>{})))
+TYPE_PARSER(construct<OpenMPDeclarativeAssumes>(
+    predicated(OmpDirectiveNameParser{},
+        IsDirective(llvm::omp::Directive::OMPD_assumes)) >=
+    Parser<OmpDirectiveSpecification>{}))
 
 // Declarative constructs
 TYPE_PARSER(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index a2b0b9ef3196c..9812a656092ac 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2558,8 +2558,8 @@ class UnparseVisitor {
 
   void Unparse(const OpenMPDeclarativeAssumes &x) {
     BeginOpenMP();
-    Word("!$OMP ASSUMES ");
-    Walk(std::get<OmpClauseList>(x.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 05ff541657b1a..6538e0b794791 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -620,10 +620,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
     checker_(GetDirName(x.t).source, Directive::OMPD_allocators);
     return false;
   }
-  bool Pre(const parser::OpenMPDeclarativeAssumes &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
-    return false;
-  }
   bool Pre(const parser::OpenMPGroupprivate &x) {
     checker_(x.v.DirName().source, Directive::OMPD_groupprivate);
     return false;
diff --git a/flang/test/Parser/OpenMP/assumption.f90 b/flang/test/Parser/OpenMP/assumption.f90
index 0f333f99f9085..86cbad9e42f78 100644
--- a/flang/test/Parser/OpenMP/assumption.f90
+++ b/flang/test/Parser/OpenMP/assumption.f90
@@ -141,9 +141,11 @@ program p
 end program p
 
 !UNPARSE: PROGRAM p
-!UNPARSE: !$OMP ASSUMES  NO_OPENMP
+!UNPARSE: !$OMP ASSUMES NO_OPENMP
 !UNPARSE: END PROGRAM p
 
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes
-!PARSE-TREE: | Verbatim
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = assumes
 !PARSE-TREE: | OmpClauseList -> OmpClause -> NoOpenmp
+!PARSE-TREE: | Flags = None
+!PARSE-TREE: ImplicitPart ->

@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2025

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

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

6 Files Affected:

  • (modified) flang/include/flang/Parser/openmp-utils.h (-2)
  • (modified) flang/include/flang/Parser/parse-tree.h (+2-2)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+4-2)
  • (modified) flang/lib/Parser/unparse.cpp (+2-2)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (-4)
  • (modified) flang/test/Parser/OpenMP/assumption.f90 (+5-3)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index bf54f970a7d3a..77c31b939e522 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -39,7 +39,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(OpenMPExecutableAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPRequiresConstruct, D::OMPD_requires);
@@ -94,7 +93,6 @@ struct DirectiveNameScope {
       if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
         return std::get<OmpBeginDirective>(x.t).DirName();
       } 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, 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 486be8b60ff8c..bd55166eb9f80 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4877,8 +4877,8 @@ struct OpenMPUtilityConstruct {
 //   ASSUMES absent-clause | contains-clause | holds-clause | no-openmp-clause |
 //          no-openmp-routines-clause | no-parallelism-clause
 struct OpenMPDeclarativeAssumes {
-  TUPLE_CLASS_BOILERPLATE(OpenMPDeclarativeAssumes);
-  std::tuple<Verbatim, OmpClauseList> t;
+  WRAPPER_CLASS_BOILERPLATE(
+      OpenMPDeclarativeAssumes, OmpDirectiveSpecification);
   CharBlock source;
 };
 
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index bd080386c0aea..12e89e8c35456 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1852,8 +1852,10 @@ TYPE_PARSER(
     lookAhead(endOmpLine / !statement(allocateStmt)))
 
 // Assumes Construct
-TYPE_PARSER(sourced(construct<OpenMPDeclarativeAssumes>(
-    verbatim("ASSUMES"_tok), Parser<OmpClauseList>{})))
+TYPE_PARSER(construct<OpenMPDeclarativeAssumes>(
+    predicated(OmpDirectiveNameParser{},
+        IsDirective(llvm::omp::Directive::OMPD_assumes)) >=
+    Parser<OmpDirectiveSpecification>{}))
 
 // Declarative constructs
 TYPE_PARSER(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index a2b0b9ef3196c..9812a656092ac 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2558,8 +2558,8 @@ class UnparseVisitor {
 
   void Unparse(const OpenMPDeclarativeAssumes &x) {
     BeginOpenMP();
-    Word("!$OMP ASSUMES ");
-    Walk(std::get<OmpClauseList>(x.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 05ff541657b1a..6538e0b794791 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -620,10 +620,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
     checker_(GetDirName(x.t).source, Directive::OMPD_allocators);
     return false;
   }
-  bool Pre(const parser::OpenMPDeclarativeAssumes &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
-    return false;
-  }
   bool Pre(const parser::OpenMPGroupprivate &x) {
     checker_(x.v.DirName().source, Directive::OMPD_groupprivate);
     return false;
diff --git a/flang/test/Parser/OpenMP/assumption.f90 b/flang/test/Parser/OpenMP/assumption.f90
index 0f333f99f9085..86cbad9e42f78 100644
--- a/flang/test/Parser/OpenMP/assumption.f90
+++ b/flang/test/Parser/OpenMP/assumption.f90
@@ -141,9 +141,11 @@ program p
 end program p
 
 !UNPARSE: PROGRAM p
-!UNPARSE: !$OMP ASSUMES  NO_OPENMP
+!UNPARSE: !$OMP ASSUMES NO_OPENMP
 !UNPARSE: END PROGRAM p
 
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes
-!PARSE-TREE: | Verbatim
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclarativeAssumes -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = assumes
 !PARSE-TREE: | OmpClauseList -> OmpClause -> NoOpenmp
+!PARSE-TREE: | Flags = None
+!PARSE-TREE: ImplicitPart ->

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Base automatically changed from users/kparzysz/r10-ods-declare-target to main September 26, 2025 20:47
@kparzysz kparzysz merged commit 17bfec5 into main Sep 27, 2025
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/r11-ods-assumes branch September 27, 2025 12:33
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp flang:parser flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants