[flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_VARIANT (#160371)

Author: kparzysz

flang/include/flang/Parser/openmp-utils.h

@@ -38,7 +38,6 @@ struct ConstructId {
     static constexpr llvm::omp::Directive id{Id}; \
   }
 
-MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant);
 MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
 MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
@@ -92,8 +91,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, OmpDeclareVariantDirective> ||
-          std::is_same_v<T, OpenMPDeclarativeAllocate> ||
+      } 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> ||

flang/include/flang/Parser/parse-tree.h

@@ -4933,10 +4933,14 @@ struct OpenMPSectionsConstruct {
       t;
 };
 
+// Ref: [4.5:58-60], [5.0:58-60], [5.1:63-68], [5.2:197-198], [6.0:334-336]
+//
+// declare-variant-directive ->
+//    DECLARE_VARIANT([base-name:]variant-name)     // since 4.5
 struct OmpDeclareVariantDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpDeclareVariantDirective);
+  WRAPPER_CLASS_BOILERPLATE(
+      OmpDeclareVariantDirective, OmpDirectiveSpecification);
   CharBlock source;
-  std::tuple<Verbatim, std::optional<Name>, Name, OmpClauseList> t;
 };
 
 // 2.10.6 declare-target -> DECLARE TARGET (extended-list) |

flang/lib/Parser/openmp-parsers.cpp

@@ -1763,8 +1763,9 @@ TYPE_PARSER(construct<OmpInitializerClause>(
 
 // OpenMP 5.2: 7.5.4 Declare Variant directive
 TYPE_PARSER(sourced(construct<OmpDeclareVariantDirective>(
-    verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok),
-    "(" >> maybe(name / ":"), name / ")", Parser<OmpClauseList>{})))
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
+    Parser<OmpDirectiveSpecification>{})))
 
 // 2.16 Declare Reduction Construct
 TYPE_PARSER(sourced(construct<OpenMPDeclareReductionConstruct>(

flang/lib/Parser/unparse.cpp

@@ -2539,12 +2539,8 @@ class UnparseVisitor {
   }
   void Unparse(const OmpDeclareVariantDirective &x) {
     BeginOpenMP();
-    Word("!$OMP DECLARE VARIANT ");
-    Put("(");
-    Walk(std::get<std::optional<Name>>(x.t), ":");
-    Walk(std::get<Name>(x.t));
-    Put(")");
-    Walk(std::get<OmpClauseList>(x.t));
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
     EndOpenMP();
   }

flang/lib/Semantics/check-omp-structure.cpp

@@ -634,11 +634,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
         std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_target);
     return false;
   }
-  bool Pre(const parser::OmpDeclareVariantDirective &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source,
-        Directive::OMPD_declare_variant);
-    return false;
-  }
   bool Pre(const parser::OpenMPGroupprivate &x) {
     checker_(x.v.DirName().source, Directive::OMPD_groupprivate);
     return false;
@@ -1370,9 +1365,50 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareSimdConstruct &) {
 }
 
 void OmpStructureChecker::Enter(const parser::OmpDeclareVariantDirective &x) {
-  const auto &dir{std::get<parser::Verbatim>(x.t)};
-  PushContextAndClauseSets(
-      dir.source, llvm::omp::Directive::OMPD_declare_variant);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+
+  const parser::OmpArgumentList &args{x.v.Arguments()};
+  if (args.v.size() != 1) {
+    context_.Say(args.source,
+        "DECLARE_VARIANT directive should have a single argument"_err_en_US);
+    return;
+  }
+
+  auto InvalidArgument{[&](parser::CharBlock source) {
+    context_.Say(source,
+        "The argument to the DECLARE_VARIANT directive should be [base-name:]variant-name"_err_en_US);
+  }};
+
+  auto CheckSymbol{[&](const Symbol *sym, parser::CharBlock source) {
+    if (sym) {
+      if (!IsProcedure(*sym) && !IsFunction(*sym)) {
+        auto &msg{context_.Say(source,
+            "The name '%s' should refer to a procedure"_err_en_US,
+            sym->name())};
+        if (sym->test(Symbol::Flag::Implicit)) {
+          msg.Attach(source, "The name '%s' has been implicitly declared"_en_US,
+              sym->name());
+        }
+      }
+    } else {
+      InvalidArgument(source);
+    }
+  }};
+
+  const parser::OmpArgument &arg{args.v.front()};
+  common::visit( //
+      common::visitors{
+          [&](const parser::OmpBaseVariantNames &y) {
+            CheckSymbol(GetObjectSymbol(std::get<0>(y.t)), arg.source);
+            CheckSymbol(GetObjectSymbol(std::get<1>(y.t)), arg.source);
+          },
+          [&](const parser::OmpLocator &y) {
+            CheckSymbol(GetArgumentSymbol(arg), arg.source);
+          },
+          [&](auto &&y) { InvalidArgument(arg.source); },
+      },
+      arg.u);
 }
 
 void OmpStructureChecker::Leave(const parser::OmpDeclareVariantDirective &) {

flang/lib/Semantics/resolve-names.cpp

@@ -1540,20 +1540,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OmpDeclareVariantDirective &x) {
     AddOmpSourceRange(x.source);
-    auto FindSymbolOrError = [&](const parser::Name &procName) {
-      auto *symbol{FindSymbol(NonDerivedTypeScope(), procName)};
-      if (!symbol) {
-        context().Say(procName.source,
-            "Implicit subroutine declaration '%s' in !$OMP DECLARE VARIANT"_err_en_US,
-            procName.source);
-      }
-    };
-    auto &baseProcName = std::get<std::optional<parser::Name>>(x.t);
-    if (baseProcName) {
-      FindSymbolOrError(*baseProcName);
-    }
-    auto &varProcName = std::get<parser::Name>(x.t);
-    FindSymbolOrError(varProcName);
     return true;
   }
 
@@ -1687,16 +1673,19 @@ class OmpVisitor : public virtual DeclarationVisitor {
       PopScope();
     }
   }
+
+  // These objects are handled explicitly, and the AST traversal should not
+  // reach a point where it calls the Pre functions for them.
   bool Pre(const parser::OmpMapperSpecifier &x) {
-    // OmpMapperSpecifier is handled explicitly, and the AST traversal
-    // should not reach a point where it calls this function.
     llvm_unreachable("This function should not be reached by AST traversal");
   }
   bool Pre(const parser::OmpReductionSpecifier &x) {
-    // OmpReductionSpecifier is handled explicitly, and the AST traversal
-    // should not reach a point where it calls this function.
     llvm_unreachable("This function should not be reached by AST traversal");
   }
+  bool Pre(const parser::OmpBaseVariantNames &x) {
+    llvm_unreachable("This function should not be reached by AST traversal");
+  }
+
   bool Pre(const parser::OmpDirectiveSpecification &x);
   void Post(const parser::OmpDirectiveSpecification &) {
     messageHandler().set_currStmtSource(std::nullopt);

flang/test/Parser/OpenMP/declare-variant.f90

@@ -2,15 +2,19 @@
 ! RUN: %flang_fc1 -fdebug-dump-parse-tree-no-sema -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
 
 subroutine sub0
-!CHECK: !$OMP DECLARE VARIANT (sub:vsub) MATCH(CONSTRUCT={PARALLEL})
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | Name = 'sub'
-!PARSE-TREE: | Name = 'vsub'
+!CHECK: !$OMP DECLARE VARIANT(sub:vsub) MATCH(CONSTRUCT={PARALLEL})
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpBaseVariantNames
+!PARSE-TREE: | | OmpObject -> Designator -> DataRef -> Name = 'sub'
+!PARSE-TREE: | | OmpObject -> Designator -> DataRef -> Name = 'vsub'
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | OmpTraitSetSelectorName -> Value = Construct
 !PARSE-TREE: | | OmpTraitSelector
 !PARSE-TREE: | | | OmpTraitSelectorName -> llvm::omp::Directive = parallel
+!PARSE-TREE: | Flags = None
+
   !$omp declare variant (sub:vsub) match (construct={parallel})
 contains
   subroutine vsub
@@ -30,14 +34,17 @@ subroutine vsub (v1)
     integer, value :: v1
   end
   subroutine sub (v1)
-!CHECK: !$OMP DECLARE VARIANT (vsub) MATCH(CONSTRUCT={DISPATCH}
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | Name = 'vsub'
+!CHECK: !$OMP DECLARE VARIANT(vsub) MATCH(CONSTRUCT={DISPATCH})
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'vsub'
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | OmpTraitSetSelectorName -> Value = Construct
 !PARSE-TREE: | | OmpTraitSelector
 !PARSE-TREE: | | | OmpTraitSelectorName -> llvm::omp::Directive = dispatch
+!PARSE-TREE: | Flags = None
+
     !$omp declare variant(vsub), match(construct={dispatch})
     integer, value :: v1
   end
@@ -56,17 +63,20 @@ subroutine vsub (v1, a1, a2)
     integer(omp_interop_kind), value :: a2
   end
   subroutine sub (v1)
-!CHECK: !$OMP DECLARE VARIANT (vsub) MATCH(CONSTRUCT={DISPATCH}) APPEND_ARGS(INTEROP(T&
-!CHECK: !$OMP&ARGET),INTEROP(TARGET))
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | Name = 'vsub'
+!CHECK: !$OMP DECLARE VARIANT(vsub) MATCH(CONSTRUCT={DISPATCH}) APPEND_ARGS(INTEROP(TA&
+!CHECK: !$OMP&RGET),INTEROP(TARGET))
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'vsub'
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | OmpTraitSetSelectorName -> Value = Construct
 !PARSE-TREE: | | OmpTraitSelector
 !PARSE-TREE: | | | OmpTraitSelectorName -> llvm::omp::Directive = dispatch
 !PARSE-TREE: | OmpClause -> AppendArgs -> OmpAppendArgsClause -> OmpAppendOp -> OmpInteropType -> Value = Target
 !PARSE-TREE: | OmpAppendOp -> OmpInteropType -> Value = Target
+!PARSE-TREE: | Flags = None
+
     !$omp declare variant(vsub), match(construct={dispatch}), append_args (interop(target), interop(target))
     integer, value :: v1
   end
@@ -81,11 +91,12 @@ subroutine sb3 (x1, x2)
 contains
   subroutine sub (v1, v2)
     type(c_ptr), value :: v1, v2
-!CHECK: !$OMP DECLARE VARIANT (vsub) MATCH(CONSTRUCT={DISPATCH}) ADJUST_ARGS(NOTHING:v&
-!CHECK: !$OMP&1) ADJUST_ARGS(NEED_DEVICE_PTR:v2)
-!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | Name = 'vsub'
+!CHECK: !$OMP DECLARE VARIANT(vsub) MATCH(CONSTRUCT={DISPATCH}) ADJUST_ARGS(NOTHING:v1&
+!CHECK: !$OMP&) ADJUST_ARGS(NEED_DEVICE_PTR:v2)
+
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'vsub'
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | OmpTraitSetSelectorName -> Value = Construct
 !PARSE-TREE: | | OmpTraitSelector
@@ -96,6 +107,8 @@ subroutine sub (v1, v2)
 !PARSE-TREE: | OmpClause -> AdjustArgs -> OmpAdjustArgsClause
 !PARSE-TREE: | | OmpAdjustOp -> Value = Need_Device_Ptr
 !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'v2'
+!PARSE-TREE: | Flags = None
+
     !$omp declare variant(vsub) match ( construct = { dispatch } ) adjust_args(nothing : v1 ) adjust_args(need_device_ptr : v2)
   end
   subroutine vsub(v1, v2)
@@ -119,13 +132,15 @@ subroutine f2 (x, y)
     !$omp declare variant (f1) match (construct={simd(uniform(y))})
   end
 end subroutine
-!CHECK: !$OMP DECLARE VARIANT (f1) MATCH(CONSTRUCT={SIMD(UNIFORM(y))})
-!PARSE-TREE: | | | | DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE-NEXT: | | | | | Verbatim
-!PARSE-TREE-NEXT: | | | | | Name = 'f1'
-!PARSE-TREE-NEXT: | | | | | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
-!PARSE-TREE-NEXT: | | | | | | OmpTraitSetSelectorName -> Value = Construct
-!PARSE-TREE-NEXT: | | | | | | OmpTraitSelector
-!PARSE-TREE-NEXT: | | | | | | | OmpTraitSelectorName -> Value = Simd
-!PARSE-TREE-NEXT: | | | | | | | Properties
-!PARSE-TREE-NEXT: | | | | | | | | OmpTraitProperty -> OmpClause -> Uniform -> Name = 'y'
+!CHECK: !$OMP DECLARE VARIANT(f1) MATCH(CONSTRUCT={SIMD(UNIFORM(y))})
+
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'f1'
+!PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
+!PARSE-TREE: | | OmpTraitSetSelectorName -> Value = Construct
+!PARSE-TREE: | | OmpTraitSelector
+!PARSE-TREE: | | | OmpTraitSelectorName -> Value = Simd
+!PARSE-TREE: | | | Properties
+!PARSE-TREE: | | | | OmpTraitProperty -> OmpClause -> Uniform -> Name = 'y'
+!PARSE-TREE: | Flags = None

flang/test/Parser/OpenMP/openmp6-directive-spellings.f90

@@ -145,12 +145,12 @@ subroutine g05
 !UNPARSE:   SUBROUTINE g05
 !UNPARSE:   END SUBROUTINE
 !UNPARSE:  END INTERFACE
-!UNPARSE: !$OMP DECLARE VARIANT (g05) MATCH(USER={CONDITION(.true._4)})
+!UNPARSE: !$OMP DECLARE_VARIANT(g05) MATCH(USER={CONDITION(.true._4)})
 !UNPARSE: END SUBROUTINE
 
-!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | Name = 'g05'
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare variant
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'g05'
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector
 !PARSE-TREE: | | OmpTraitSetSelectorName -> Value = User
 !PARSE-TREE: | | OmpTraitSelector
@@ -159,6 +159,7 @@ subroutine g05
 !PARSE-TREE: | | | | OmpTraitProperty -> Scalar -> Expr = '.true._4'
 !PARSE-TREE: | | | | | LiteralConstant -> LogicalLiteralConstant
 !PARSE-TREE: | | | | | | bool = 'true'
+!PARSE-TREE: | Flags = None
 
 subroutine f06
   implicit none

flang/test/Semantics/OpenMP/declare-variant.f90

@@ -1,9 +1,9 @@
 ! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=51
 
 subroutine sub0
-!ERROR: Implicit subroutine declaration 'vsub1' in !$OMP DECLARE VARIANT
+!ERROR: The name 'vsub1' should refer to a procedure
   !$omp declare variant (sub:vsub1) match (construct={parallel})
-!ERROR: Implicit subroutine declaration 'sub1' in !$OMP DECLARE VARIANT
+!ERROR: The name 'sub1' should refer to a procedure
   !$omp declare variant (sub1:vsub) match (construct={parallel})
 contains
   subroutine vsub