[llvm-branch-commits] [llvm] 71699a9 - [flang][openacc] Enforce restriction on routine directive and clauses

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 17 08:38:32 PST 2020


Author: Valentin Clement
Date: 2020-12-17T11:33:34-05:00
New Revision: 71699a998d4f648396a1a12820c0f04cc61f8e19

URL: https://github.com/llvm/llvm-project/commit/71699a998d4f648396a1a12820c0f04cc61f8e19
DIFF: https://github.com/llvm/llvm-project/commit/71699a998d4f648396a1a12820c0f04cc61f8e19.diff

LOG: [flang][openacc] Enforce restriction on routine directive and clauses

This patch add some checks for the restriction on the routine directive
and fix several issue at the same time.

Validity tests have been added in a separate file than acc-clause-validity.f90 since this one
became quite large. I plan to split the larger file once on-going review are done.

Reviewed By: sameeranjoshi

Differential Revision: https://reviews.llvm.org/D92672

Added: 
    flang/test/Semantics/acc-routine-validity.f90

Modified: 
    flang/include/flang/Parser/dump-parse-tree.h
    flang/include/flang/Parser/parse-tree.h
    flang/lib/Parser/openacc-parsers.cpp
    flang/lib/Parser/unparse.cpp
    flang/lib/Semantics/check-acc-structure.cpp
    flang/lib/Semantics/resolve-directives.cpp
    flang/test/Semantics/acc-clause-validity.f90
    llvm/include/llvm/Frontend/OpenACC/ACC.td

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 8a7d1d1302b2..92f7113b316a 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -66,6 +66,7 @@ class ParseTreeDumper {
   NODE(parser, AccClause)
 #define GEN_FLANG_DUMP_PARSE_TREE_CLAUSES
 #include "llvm/Frontend/OpenACC/ACC.cpp.inc"
+  NODE(parser, AccBindClause)
   NODE(parser, AccDefaultClause)
   NODE_ENUM(parser::AccDefaultClause, Arg)
   NODE(parser, AccClauseList)

diff  --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index a2beac4737f6..6bf4d8568bde 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3852,6 +3852,12 @@ struct AccDeclarativeDirective {
 };
 
 // OpenACC Clauses
+struct AccBindClause {
+  UNION_CLASS_BOILERPLATE(AccBindClause);
+  std::variant<Name, ScalarDefaultCharExpr> u;
+  CharBlock source;
+};
+
 struct AccDefaultClause {
   ENUM_CLASS(Arg, None, Present)
   WRAPPER_CLASS_BOILERPLATE(AccDefaultClause, Arg);
@@ -4048,7 +4054,8 @@ struct OpenACCCombinedConstruct {
 struct OpenACCDeclarativeConstruct {
   UNION_CLASS_BOILERPLATE(OpenACCDeclarativeConstruct);
   CharBlock source;
-  std::variant<OpenACCStandaloneDeclarativeConstruct> u;
+  std::variant<OpenACCStandaloneDeclarativeConstruct, OpenACCRoutineConstruct>
+      u;
 };
 
 // OpenACC directives enclosing do loop
@@ -4068,8 +4075,8 @@ struct OpenACCStandaloneConstruct {
 struct OpenACCConstruct {
   UNION_CLASS_BOILERPLATE(OpenACCConstruct);
   std::variant<OpenACCBlockConstruct, OpenACCCombinedConstruct,
-      OpenACCLoopConstruct, OpenACCStandaloneConstruct, OpenACCRoutineConstruct,
-      OpenACCCacheConstruct, OpenACCWaitConstruct, OpenACCAtomicConstruct>
+      OpenACCLoopConstruct, OpenACCStandaloneConstruct, OpenACCCacheConstruct,
+      OpenACCWaitConstruct, OpenACCAtomicConstruct>
       u;
 };
 

diff  --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index b4d2b285cd6c..2447ed70b1a1 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -28,8 +28,8 @@ TYPE_PARSER("AUTO" >> construct<AccClause>(construct<AccClause::Auto>()) ||
                    maybe(parenthesized(scalarIntExpr)))) ||
     "ATTACH" >> construct<AccClause>(construct<AccClause::Attach>(
                     parenthesized(Parser<AccObjectList>{}))) ||
-    "BIND" >>
-        construct<AccClause>(construct<AccClause::Bind>(parenthesized(name))) ||
+    "BIND" >> construct<AccClause>(
+                  construct<AccClause::Bind>(Parser<AccBindClause>{})) ||
     "CAPTURE" >> construct<AccClause>(construct<AccClause::Capture>()) ||
     "COLLAPSE" >> construct<AccClause>(construct<AccClause::Collapse>(
                       parenthesized(scalarIntConstantExpr))) ||
@@ -166,6 +166,10 @@ TYPE_PARSER(sourced(construct<AccReductionOperator>(
         ".EQV." >> pure(AccReductionOperator::Operator::Eqv),
         ".NEQV." >> pure(AccReductionOperator::Operator::Neqv)))))
 
+// 2.15.1 Bind clause
+TYPE_PARSER(sourced(construct<AccBindClause>(parenthesized(name))) ||
+    sourced(construct<AccBindClause>(parenthesized(scalarDefaultCharExpr))))
+
 // 2.5.14 Default clause
 TYPE_PARSER(construct<AccDefaultClause>(
     parenthesized(first("NONE" >> pure(AccDefaultClause::Arg::None),
@@ -287,8 +291,10 @@ TYPE_PARSER(construct<OpenACCStandaloneDeclarativeConstruct>(
     sourced(Parser<AccDeclarativeDirective>{}), Parser<AccClauseList>{}))
 
 TYPE_PARSER(
-    startAccLine >> sourced(construct<OpenACCDeclarativeConstruct>(
-                        Parser<OpenACCStandaloneDeclarativeConstruct>{})))
+    startAccLine >> first(sourced(construct<OpenACCDeclarativeConstruct>(
+                              Parser<OpenACCStandaloneDeclarativeConstruct>{})),
+                        sourced(construct<OpenACCDeclarativeConstruct>(
+                            Parser<OpenACCRoutineConstruct>{}))))
 
 // OpenACC constructs
 TYPE_CONTEXT_PARSER("OpenACC construct"_en_US,
@@ -297,7 +303,6 @@ TYPE_CONTEXT_PARSER("OpenACC construct"_en_US,
             construct<OpenACCConstruct>(Parser<OpenACCCombinedConstruct>{}),
             construct<OpenACCConstruct>(Parser<OpenACCLoopConstruct>{}),
             construct<OpenACCConstruct>(Parser<OpenACCStandaloneConstruct>{}),
-            construct<OpenACCConstruct>(Parser<OpenACCRoutineConstruct>{}),
             construct<OpenACCConstruct>(Parser<OpenACCCacheConstruct>{}),
             construct<OpenACCConstruct>(Parser<OpenACCWaitConstruct>{}),
             construct<OpenACCConstruct>(Parser<OpenACCAtomicConstruct>{})))

diff  --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index bd1c1a2c71eb..0f0c6f285f61 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1839,6 +1839,15 @@ class UnparseVisitor {
   void Unparse(const AccDataModifier::Modifier &x) {
     Word(AccDataModifier::EnumToString(x));
   }
+  void Unparse(const AccBindClause &x) {
+    std::visit(common::visitors{
+                   [&](const Name &y) { Put('('), Walk(y), Put(')'); },
+                   [&](const ScalarDefaultCharExpr &y) {
+                     Put('('), Walk(y), Put(')');
+                   },
+               },
+        x.u);
+  }
   void Unparse(const AccDefaultClause &x) {
     switch (x.v) {
     case AccDefaultClause::Arg::None:

diff  --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index c120f9a591e6..45499a69e604 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -235,6 +235,18 @@ void AccStructureChecker::Leave(const parser::OpenACCStandaloneConstruct &x) {
 
 void AccStructureChecker::Enter(const parser::OpenACCRoutineConstruct &x) {
   PushContextAndClauseSets(x.source, llvm::acc::Directive::ACCD_routine);
+  const auto &optName{std::get<std::optional<parser::Name>>(x.t)};
+  if (!optName) {
+    const auto &verbatim{std::get<parser::Verbatim>(x.t)};
+    const auto &scope{context_.FindScope(verbatim.source)};
+    const Scope &containingScope{GetProgramUnitContaining(scope)};
+    if (containingScope.kind() == Scope::Kind::Module) {
+      context_.Say(GetContext().directiveSource,
+          "ROUTINE directive without name must appear within the specification "
+          "part of a subroutine or function definition, or within an interface "
+          "body for a subroutine or function in an interface block"_err_en_US);
+    }
+  }
 }
 void AccStructureChecker::Leave(const parser::OpenACCRoutineConstruct &) {
   // Restriction - line 2790

diff  --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 16fdb09a5dbc..7ca42bac569a 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -114,16 +114,17 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
   template <typename A> bool Pre(const A &) { return true; }
   template <typename A> void Post(const A &) {}
 
-  bool Pre(const parser::SpecificationPart &x) {
-    Walk(std::get<std::list<parser::OpenACCDeclarativeConstruct>>(x.t));
-    return false;
-  }
-
   bool Pre(const parser::OpenACCBlockConstruct &);
   void Post(const parser::OpenACCBlockConstruct &) { PopContext(); }
   bool Pre(const parser::OpenACCCombinedConstruct &);
   void Post(const parser::OpenACCCombinedConstruct &) { PopContext(); }
 
+  bool Pre(const parser::OpenACCDeclarativeConstruct &);
+  void Post(const parser::OpenACCDeclarativeConstruct &) { PopContext(); }
+
+  bool Pre(const parser::OpenACCRoutineConstruct &);
+  bool Pre(const parser::AccBindClause &);
+
   void Post(const parser::AccBeginBlockDirective &) {
     GetContext().withinConstruct = true;
   }
@@ -207,6 +208,7 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
   void ResolveAccObject(const parser::AccObject &, Symbol::Flag);
   Symbol *ResolveAcc(const parser::Name &, Symbol::Flag, Scope &);
   Symbol *ResolveAcc(Symbol &, Symbol::Flag, Scope &);
+  Symbol *ResolveName(const parser::Name &);
   Symbol *ResolveAccCommonBlockName(const parser::Name *);
   Symbol *DeclareOrMarkOtherAccessEntity(const parser::Name &, Symbol::Flag);
   Symbol *DeclareOrMarkOtherAccessEntity(Symbol &, Symbol::Flag);
@@ -440,6 +442,21 @@ bool AccAttributeVisitor::Pre(const parser::OpenACCBlockConstruct &x) {
   return true;
 }
 
+bool AccAttributeVisitor::Pre(const parser::OpenACCDeclarativeConstruct &x) {
+  if (const auto *declConstruct{
+          std::get_if<parser::OpenACCStandaloneDeclarativeConstruct>(&x.u)}) {
+    const auto &declDir{
+        std::get<parser::AccDeclarativeDirective>(declConstruct->t)};
+    PushContext(declDir.source, llvm::acc::Directive::ACCD_declare);
+  } else if (const auto *routineConstruct{
+                 std::get_if<parser::OpenACCRoutineConstruct>(&x.u)}) {
+    const auto &verbatim{std::get<parser::Verbatim>(routineConstruct->t)};
+    PushContext(verbatim.source, llvm::acc::Directive::ACCD_routine);
+  }
+  ClearDataSharingAttributeObjects();
+  return true;
+}
+
 bool AccAttributeVisitor::Pre(const parser::OpenACCLoopConstruct &x) {
   const auto &beginDir{std::get<parser::AccBeginLoopDirective>(x.t)};
   const auto &loopDir{std::get<parser::AccLoopDirective>(beginDir.t)};
@@ -471,6 +488,35 @@ bool AccAttributeVisitor::Pre(const parser::OpenACCStandaloneConstruct &x) {
   return true;
 }
 
+Symbol *AccAttributeVisitor::ResolveName(const parser::Name &name) {
+  Symbol *prev{currScope().FindSymbol(name.source)};
+  if (prev != name.symbol) {
+    name.symbol = prev;
+  }
+  return prev;
+}
+
+bool AccAttributeVisitor::Pre(const parser::OpenACCRoutineConstruct &x) {
+  const auto &optName{std::get<std::optional<parser::Name>>(x.t)};
+  if (optName) {
+    if (!ResolveName(*optName))
+      context_.Say((*optName).source,
+          "No function or subroutine declared for '%s'"_err_en_US,
+          (*optName).source);
+  }
+  return true;
+}
+
+bool AccAttributeVisitor::Pre(const parser::AccBindClause &x) {
+  if (const auto *name{std::get_if<parser::Name>(&x.u)}) {
+    if (!ResolveName(*name))
+      context_.Say(name->source,
+          "No function or subroutine declared for '%s'"_err_en_US,
+          name->source);
+  }
+  return true;
+}
+
 bool AccAttributeVisitor::Pre(const parser::OpenACCCombinedConstruct &x) {
   const auto &beginBlockDir{std::get<parser::AccBeginCombinedDirective>(x.t)};
   const auto &combinedDir{

diff  --git a/flang/test/Semantics/acc-clause-validity.f90 b/flang/test/Semantics/acc-clause-validity.f90
index a5c6193d32f5..d410c5b4bf85 100644
--- a/flang/test/Semantics/acc-clause-validity.f90
+++ b/flang/test/Semantics/acc-clause-validity.f90
@@ -1240,18 +1240,4 @@ program openacc_clause_validity
   !ERROR: Only array element or subarray are allowed in CACHE directive
   !$acc cache(/i/)
 
- contains
-
-   subroutine sub1(a)
-     real :: a(:)
-     !ERROR: At least one of GANG, SEQ, VECTOR, WORKER clause must appear on the ROUTINE directive
-     !$acc routine
-   end subroutine sub1
-
-   subroutine sub2(a)
-     real :: a(:)
-     !ERROR: Clause NOHOST is not allowed after clause DEVICE_TYPE on the ROUTINE directive
-     !$acc routine seq device_type(*) nohost
-   end subroutine sub2
-
 end program openacc_clause_validity

diff  --git a/flang/test/Semantics/acc-routine-validity.f90 b/flang/test/Semantics/acc-routine-validity.f90
new file mode 100644
index 000000000000..987545c408c2
--- /dev/null
+++ b/flang/test/Semantics/acc-routine-validity.f90
@@ -0,0 +1,79 @@
+! RUN: %S/test_errors.sh %s %t %f18 -fopenacc
+
+! Check OpenACC clause validity for the following construct and directive:
+!   2.15.1 routine
+
+module openacc_routine_validity
+  implicit none
+
+  !$acc routine(sub3) seq
+
+  !$acc routine(fct2) vector
+
+  !ERROR: At least one of GANG, SEQ, VECTOR, WORKER clause must appear on the ROUTINE directive
+  !$acc routine(sub3)
+
+  !ERROR: ROUTINE directive without name must appear within the specification part of a subroutine or function definition, or within an interface body for a subroutine or function in an interface block
+  !$acc routine seq
+
+  !ERROR: No function or subroutine declared for 'dummy'
+  !$acc routine(dummy) seq
+
+contains
+
+  subroutine sub1(a)
+    real :: a(:)
+    !ERROR: At least one of GANG, SEQ, VECTOR, WORKER clause must appear on the ROUTINE directive
+    !$acc routine
+  end subroutine sub1
+
+  subroutine sub2(a)
+    real :: a(:)
+    !ERROR: Clause NOHOST is not allowed after clause DEVICE_TYPE on the ROUTINE directive
+    !$acc routine seq device_type(*) nohost
+  end subroutine sub2
+
+  subroutine sub3(a)
+    real :: a(:)
+  end subroutine sub3
+
+  subroutine sub4(a)
+    real :: a(:)
+    !$acc routine seq
+  end subroutine sub4
+
+  subroutine sub5(a)
+    real :: a(:)
+    !$acc routine(sub5) seq
+  end subroutine sub5
+
+  function fct1(a)
+    integer :: fct1
+    real :: a(:)
+    !$acc routine vector nohost
+  end function fct1
+
+  function fct2(a)
+    integer :: fct2
+    real :: a(:)
+  end function fct2
+
+  function fct3(a)
+    integer :: fct3
+    real :: a(:)
+    !$acc routine seq bind(fct2)
+  end function fct3
+
+  function fct4(a)
+    integer :: fct4
+    real :: a(:)
+    !$acc routine seq bind("_fct4")
+  end function fct4
+
+  subroutine sub6(a)
+    real :: a(:)
+    !ERROR: No function or subroutine declared for 'dummy_sub'
+    !$acc routine seq bind(dummy_sub)
+  end subroutine sub6
+
+end module openacc_routine_validity

diff  --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index a1ba90d13f4f..d5998845a839 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -48,7 +48,7 @@ def ACCC_Attach : Clause<"attach"> {
 
 // 2.15.1
 def ACCC_Bind : Clause<"bind"> {
-  let flangClassValue = "Name";
+  let flangClassValue = "AccBindClause";
 }
 
 // 2.12


        


More information about the llvm-branch-commits mailing list