[OpenACC] Implement 'routine' construct parsing

[erichkeane]

The 'routine' construct applies either to a function directly, or, when
provided a name, applies to the function named (and is visible in the
current scope). This patch implements the parsing for this. The
identifier provided (or Id Expression) is required to be a valid,
declared identifier, though the semantic analysis portion of the Routine
directive will need to enforce it being a function/overload set.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

The 'routine' construct applies either to a function directly, or, when
provided a name, applies to the function named (and is visible in the
current scope). This patch implements the parsing for this. The
identifier provided (or Id Expression) is required to be a valid,
declared identifier, though the semantic analysis portion of the Routine
directive will need to enforce it being a function/overload set.

---

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

6 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2)
• (modified) clang/include/clang/Basic/OpenACCKinds.h (+2-1)
• (modified) clang/include/clang/Parse/Parser.h (+5)
• (modified) clang/lib/Parse/ParseOpenACC.cpp (+68-10)
• (modified) clang/test/ParserOpenACC/parse-constructs.c (+42)
• (added) clang/test/ParserOpenACC/parse-constructs.cpp (+46)

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 54b5ba6e6414b2d..2fd7165a422859a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1364,6 +1364,8 @@ def warn_pragma_acc_unimplemented_clause_parsing
 def err_acc_invalid_directive
     : Error<"invalid OpenACC directive '%select{%1|%1 %2}0'">;
 def err_acc_missing_directive : Error<"expected OpenACC directive">;
+def err_acc_invalid_open_paren
+    : Error<"expected clause-list or newline in OpenACC directive">;
 
 // OpenMP support.
 def warn_pragma_omp_ignored : Warning<
diff --git a/clang/include/clang/Basic/OpenACCKinds.h b/clang/include/clang/Basic/OpenACCKinds.h
index cf4bad9ce0cb9ff..1a5bf7e0e831ce3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -55,7 +55,8 @@ enum class OpenACCDirectiveKind {
   Update,
   // FIXME: wait construct.
 
-  // FIXME: routine construct.
+  // Procedure Calls in Compute Regions.
+  Routine,
 
   // Invalid.
   Invalid,
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 465453826c0b982..ca29ce46873e8a4 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3532,9 +3532,14 @@ class Parser : public CodeCompletionHandler {
   /// Placeholder for now, should just ignore the directives after emitting a
   /// diagnostic. Eventually will be split into a few functions to parse
   /// different situations.
+public:
   DeclGroupPtrTy ParseOpenACCDirectiveDecl();
   StmtResult ParseOpenACCDirectiveStmt();
 
+private:
+  void ParseOpenACCDirective();
+  ExprResult ParseOpenACCRoutineName();
+
 private:
   //===--------------------------------------------------------------------===//
   // C++ 14: Templates [temp]
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 978a07ec82e4288..88c2fd68e075d65 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -46,6 +46,7 @@ OpenACCDirectiveKindEx getOpenACCDirectiveKind(StringRef Name) {
           .Case("host_data", OpenACCDirectiveKind::HostData)
           .Case("loop", OpenACCDirectiveKind::Loop)
           .Case("atomic", OpenACCDirectiveKind::Atomic)
+          .Case("routine", OpenACCDirectiveKind::Routine)
           .Case("declare", OpenACCDirectiveKind::Declare)
           .Case("init", OpenACCDirectiveKind::Init)
           .Case("shutdown", OpenACCDirectiveKind::Shutdown)
@@ -97,6 +98,8 @@ bool isOpenACCDirectiveKind(OpenACCDirectiveKind Kind, StringRef Tok) {
 
   case OpenACCDirectiveKind::Atomic:
     return Tok == "atomic";
+  case OpenACCDirectiveKind::Routine:
+    return Tok == "routine";
   case OpenACCDirectiveKind::Declare:
     return Tok == "declare";
   case OpenACCDirectiveKind::Init:
@@ -232,24 +235,79 @@ void ParseOpenACCClauseList(Parser &P) {
     P.Diag(P.getCurToken(), diag::warn_pragma_acc_unimplemented_clause_parsing);
 }
 
-void ParseOpenACCDirective(Parser &P) {
-  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(P);
+} // namespace
+
+// Routine has an optional paren-wrapped name of a function in the local scope.
+// We parse the name, emitting any diagnostics
+ExprResult Parser::ParseOpenACCRoutineName() {
+
+  ExprResult Res;
+  if (getLangOpts().CPlusPlus)
+    Res = ParseCXXIdExpression(/*isAddressOfOperand=*/false);
+  else {
+    // There isn't anything quite the same as ParseCXXIdExpression for C, so we
+    // need to get the identifier, then call into Sema ourselves.
+
+    if (expectIdentifier())
+      return ExprError();
+
+    Token FuncName = getCurToken();
+    UnqualifiedId Name;
+    CXXScopeSpec ScopeSpec;
+    SourceLocation TemplateKWLoc;
+    Name.setIdentifier(FuncName.getIdentifierInfo(), ConsumeToken());
+
+    // Ensure this is a valid identifier. We don't accept causing implicit
+    // function declarations per the spec, so always claim to not have trailing
+    // L Paren.
+    Res = Actions.ActOnIdExpression(getCurScope(), ScopeSpec, TemplateKWLoc,
+                                    Name, /*HasTrailingLParen=*/false,
+                                    /*isAddressOfOperand=*/false);
+  }
+
+  return getActions().CorrectDelayedTyposInExpr(Res);
+}
+
+void Parser::ParseOpenACCDirective() {
+  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(*this);
 
   // Once we've parsed the construct/directive name, some have additional
   // specifiers that need to be taken care of. Atomic has an 'atomic-clause'
   // that needs to be parsed.
   if (DirKind == OpenACCDirectiveKind::Atomic)
-    ParseOpenACCAtomicKind(P);
+    ParseOpenACCAtomicKind(*this);
+
+  // We've successfully parsed the construct/directive name, however a few of
+  // the constructs have optional parens that contain further details.
+  BalancedDelimiterTracker T(*this, tok::l_paren,
+                             tok::annot_pragma_openacc_end);
+
+  if (!T.consumeOpen()) {
+    switch (DirKind) {
+    default:
+      Diag(T.getOpenLocation(), diag::err_acc_invalid_open_paren);
+      T.skipToEnd();
+      break;
+    case OpenACCDirectiveKind::Routine: {
+      ExprResult RoutineName = ParseOpenACCRoutineName();
+      // If the routine name is invalid, just skip until the closing paren to
+      // recover more gracefully.
+      if (RoutineName.isInvalid())
+        T.skipToEnd();
+      else
+        T.consumeClose();
+      break;
+    }
+    }
+  }
 
   // Parses the list of clauses, if present.
-  ParseOpenACCClauseList(P);
+  ParseOpenACCClauseList(*this);
 
-  P.Diag(P.getCurToken(), diag::warn_pragma_acc_unimplemented);
-  P.SkipUntil(tok::annot_pragma_openacc_end);
+  Diag(getCurToken(), diag::warn_pragma_acc_unimplemented);
+  SkipUntil(tok::annot_pragma_openacc_end);
 }
 
-} // namespace
-
 // Parse OpenACC directive on a declaration.
 Parser::DeclGroupPtrTy Parser::ParseOpenACCDirectiveDecl() {
   assert(Tok.is(tok::annot_pragma_openacc) && "expected OpenACC Start Token");
@@ -257,7 +315,7 @@ Parser::DeclGroupPtrTy Parser::ParseOpenACCDirectiveDecl() {
   ParsingOpenACCDirectiveRAII DirScope(*this);
   ConsumeAnnotationToken();
 
-  ParseOpenACCDirective(*this);
+  ParseOpenACCDirective();
 
   return nullptr;
 }
@@ -269,7 +327,7 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
   ParsingOpenACCDirectiveRAII DirScope(*this);
   ConsumeAnnotationToken();
 
-  ParseOpenACCDirective(*this);
+  ParseOpenACCDirective();
 
   return StmtEmpty();
 }
diff --git a/clang/test/ParserOpenACC/parse-constructs.c b/clang/test/ParserOpenACC/parse-constructs.c
index 59d14cff9d416e9..f0f9d75ade1fb2e 100644
--- a/clang/test/ParserOpenACC/parse-constructs.c
+++ b/clang/test/ParserOpenACC/parse-constructs.c
@@ -16,9 +16,16 @@ void func() {
   // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
 #pragma acc parallel clause list
   for(;;){}
+  // expected-error@+3{{expected clause-list or newline in OpenACC directive}}
   // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
   // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
 #pragma acc parallel() clause list
+  for(;;){}
+  // expected-error@+4{{expected clause-list or newline in OpenACC directive}}
+  // expected-error@+3{{expected ')'}}
+  // expected-note@+2{{to match this '('}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc parallel( clause list
   for(;;){}
   // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
   // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
@@ -144,3 +151,38 @@ void func() {
 #pragma acc update clause list
   for(;;){}
 }
+
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine
+void routine_func();
+// expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine clause list
+void routine_func();
+
+// expected-error@+2{{use of undeclared identifier 'func_name'}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (func_name)
+// expected-error@+3{{use of undeclared identifier 'func_name'}}
+// expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (func_name) clause list
+
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (routine_func)
+// expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (routine_func) clause list
+
+// expected-error@+3{{expected ')'}}
+// expected-note@+2{{to match this '('}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (routine_func())
+
+// expected-error@+2{{expected identifier}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine()
+
+// expected-error@+2{{expected identifier}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(int)
diff --git a/clang/test/ParserOpenACC/parse-constructs.cpp b/clang/test/ParserOpenACC/parse-constructs.cpp
new file mode 100644
index 000000000000000..f26f9aee8546b47
--- /dev/null
+++ b/clang/test/ParserOpenACC/parse-constructs.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 %s -verify -fopenacc
+
+namespace NS {
+  void foo(); // expected-note{{declared here}}
+
+  template<typename T>
+  void templ(); // expected-note 2{{declared here}}
+}
+
+// expected-error@+2{{use of undeclared identifier 'foo'; did you mean 'NS::foo'?}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(foo)
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(NS::foo)
+
+// expected-error@+2{{use of undeclared identifier 'templ'; did you mean 'NS::templ'?}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(templ)
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(NS::templ)
+
+// expected-error@+2{{use of undeclared identifier 'templ'; did you mean 'NS::templ'?}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(templ<int>)
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(NS::templ<int>)
+
+// expected-error@+2{{use of undeclared identifier 'T'}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(templ<T>)
+// expected-error@+2{{use of undeclared identifier 'T'}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(NS::templ<T>)
+
+// expected-error@+3{{expected ')'}}
+// expected-note@+2{{to match this '('}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine (NS::foo())
+
+// expected-error@+2 {{expected unqualified-id}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine()
+
+// expected-error@+2 {{expected unqualified-id}}
+// expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc routine(int)

[alexey-bataev]

Enclose in braces for uniformity

[erichkeane]

Interestingly, when I was rewriting our brace policy a few years ago, nearly this exact case was one that people wanted to preserve, so the rule essentially became "once you start, use it for the rest, but not before you need to".

I wasn't a huge proponent of it, and don't mind here, so I've added them.

Nevermind, I looked and I mis-remembered, we DO suggest braces here! I remember a big argument about this one, and thought I 'lost'!