Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpenACC] Implement initial parsing for parallel construct #72661

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

erichkeane
Copy link
Collaborator

@erichkeane erichkeane commented Nov 17, 2023

As the first real parsing effort for the OpenACC implementation effort, this implements the parsing for first construct/directive name. This does not do any semantic analysis, nor any of the clauses. Those will come in a future patch.

For the time being, we warn when we hit a point that we don't implement the parsing for either of these situations.

As the first real parsing effort for the OpenACC implementation effort,
this implements the parsing for construct/directive names. This does not
do any semantic analysis, nor any parsing for the parens for afew of the
constructs, nor any of the clauses.  Those will come in a future patch.

For the time being, we warn when we hit a point that we don't implement
the parsing for either of these situations.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 17, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 17, 2023

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

As the first real parsing effort for the OpenACC implementation effort, this implements the parsing for construct/directive names. This does not do any semantic analysis, nor any parsing for the parens for afew of the constructs, nor any of the clauses. Those will come in a future patch.

For the time being, we warn when we hit a point that we don't implement the parsing for either of these situations.


Patch is 26.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72661.diff

11 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+21-4)
  • (added) clang/include/clang/Basic/OpenACCKinds.h (+72)
  • (modified) clang/include/clang/Parse/Parser.h (+5-1)
  • (modified) clang/include/clang/Parse/RAIIObjectsForParser.h (+19)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+1-1)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+1-1)
  • (modified) clang/lib/Parse/ParseOpenACC.cpp (+227-5)
  • (modified) clang/lib/Parse/Parser.cpp (+4-3)
  • (added) clang/test/ParserOpenACC/parse-constructs.c (+148)
  • (modified) clang/test/ParserOpenACC/unimplemented.c (+3-3)
  • (modified) clang/test/ParserOpenACC/unimplemented.cpp (+3-3)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c3d06053caa5eea..54ae536e9cf7935 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1343,13 +1343,30 @@ def err_openclcxx_virtual_function : Error<
   "virtual functions are not supported in C++ for OpenCL">;
 
 // OpenACC Support.
-def warn_pragma_acc_ignored : Warning<
-  "unexpected '#pragma acc ...' in program">, InGroup<SourceUsesOpenACC>, DefaultIgnore;
-def err_acc_unexpected_directive : Error<
-  "unexpected OpenACC directive %select{|'#pragma acc %1'}0">;
+def warn_pragma_acc_ignored
+    : Warning<"unexpected '#pragma acc ...' in program">,
+      InGroup<SourceUsesOpenACC>,
+      DefaultIgnore;
+def err_acc_unexpected_directive
+    : Error<"unexpected OpenACC directive %select{|'#pragma acc %1'}0">;
 def warn_pragma_acc_unimplemented
     : Warning<"OpenACC directives not yet implemented, pragma ignored">,
       InGroup<SourceUsesOpenACC>;
+def warn_pragma_acc_unimplemented_construct_parens
+    : Warning<
+          "OpenACC %select{'cache' 'var-list'|'wait-argument'|'routine-name'}0 "
+          "parsing not yet implemented">,
+      InGroup<SourceUsesOpenACC>;
+def warn_pragma_acc_unimplemented_clause_parsing
+    : Warning<"OpenACC clause parsing not yet implemented">,
+      InGroup<SourceUsesOpenACC>;
+def err_acc_invalid_directive
+    : Error<"invalid OpenACC directive '%select{%1|%1 %2}0'">;
+def err_acc_invalid_atomic_clause
+    : Error<"invalid OpenACC 'atomic-clause' '%0'; expected 'read', 'write', "
+            "'update', or 'capture'">;
+def err_acc_invalid_open_paren
+    : Error<"expected clause-list or newline in pragma 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
new file mode 100644
index 000000000000000..8a901d96bec5207
--- /dev/null
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -0,0 +1,72 @@
+//===--- OpenACCKinds.h - OpenACC Enums -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Defines some OpenACC-specific enums and functions.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_OPENACCKINDS_H
+#define LLVM_CLANG_BASIC_OPENACCKINDS_H
+
+namespace clang {
+// Represents the Construct/Directive kind of a pragma directive. Note the
+// OpenACC standard is inconsistent between calling these Construct vs
+// Directive, but we're calling it a Directive to be consistent with OpenMP.
+enum class OpenACCDirectiveKind {
+  // Compute Constructs.
+  Parallel,
+  Serial,
+  Kernels,
+
+  // Data Environment. "enter data" and "exit data" are also referred to in the
+  // Executable Directives section, but just as a back reference to the Data
+  // Environment.
+  Data,
+  EnterData,
+  ExitData,
+  HostData,
+
+  // Misc.
+  Loop,
+  Cache,
+
+  // Combined Constructs.
+  ParallelLoop,
+  SerialLoop,
+  KernelsLoop,
+
+  // Atomic Construct.  The OpenACC standard considers these as a single
+  // construct, however the atomic-clause (read, write, update, capture) are
+  // important for legalization of the application of this to statements/blocks.
+  AtomicRead,
+  AtomicWrite,
+  AtomicUpdate,
+  AtomicCapture,
+
+  // Declare Directive.
+  Declare,
+
+  // Executable Directives. "wait" is first referred to here, but ends up being
+  // in its own section after "routine".
+  Init,
+  Shutdown,
+  Set,
+  Update,
+  Wait,
+
+  // Procedure Calls in Compute Regions.
+  Routine,
+
+  // Invalid.
+  Invalid,
+};
+}
+
+
+#endif // LLVM_CLANG_BASIC_OPENACCKINDS_H
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 4631e9a4679c435..d20a26dbf2562a7 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -53,6 +53,7 @@ namespace clang {
 class Parser : public CodeCompletionHandler {
   friend class ColonProtectionRAIIObject;
   friend class ParsingOpenMPDirectiveRAII;
+  friend class ParsingOpenACCDirectiveRAII;
   friend class InMessageExpressionRAIIObject;
   friend class OffsetOfStateRAIIObject;
   friend class PoisonSEHIdentifiersRAIIObject;
@@ -230,6 +231,9 @@ class Parser : public CodeCompletionHandler {
   /// Parsing OpenMP directive mode.
   bool OpenMPDirectiveParsing = false;
 
+  /// Parsing OpenACC directive mode.
+  bool OpenACCDirectiveParsing = false;
+
   /// When true, we are directly inside an Objective-C message
   /// send expression.
   ///
@@ -3531,7 +3535,7 @@ 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.
-  DeclGroupPtrTy ParseOpenACCDirective();
+  DeclGroupPtrTy ParseOpenACCDirectiveDecl();
   StmtResult ParseOpenACCDirectiveStmt();
 
 private:
diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h
index cb525c9d0edd6bc..e1626a7870bb7ae 100644
--- a/clang/include/clang/Parse/RAIIObjectsForParser.h
+++ b/clang/include/clang/Parse/RAIIObjectsForParser.h
@@ -309,6 +309,25 @@ namespace clang {
     ~ParsingOpenMPDirectiveRAII() { restore(); }
   };
 
+  /// Activates OpenACC parsing mode to preseve OpenACC specific annotation
+  /// tokens.
+  class ParsingOpenACCDirectiveRAII {
+    Parser &P;
+    bool OldVal;
+
+  public:
+    ParsingOpenACCDirectiveRAII(Parser &P, bool Value = true)
+        : P(P), OldVal(P.OpenACCDirectiveParsing) {
+      P.OpenACCDirectiveParsing = Value;
+    }
+
+    /// This can be used to restore the state early, before the dtor
+    /// is run.
+    void restore() { P.OpenMPDirectiveParsing = OldVal; }
+
+    ~ParsingOpenACCDirectiveRAII() { restore(); }
+  };
+
   /// RAII object that makes '>' behave either as an operator
   /// or as the closing angle bracket for a template argument list.
   class GreaterThanIsOperatorScope {
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index d86f477b4c9ff7a..8cb5b09fd3b0fa6 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4735,7 +4735,7 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
     }
 
     if (Tok.is(tok::annot_pragma_openacc)) {
-      ParseOpenACCDirective();
+      ParseOpenACCDirectiveDecl();
       continue;
     }
 
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index e12215d74bcc8ed..d9125955fda2783 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -3430,7 +3430,7 @@ Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclarationWithPragmas(
     return ParseOpenMPDeclarativeDirectiveWithExtDecl(
         AS, AccessAttrs, /*Delayed=*/true, TagType, TagDecl);
   case tok::annot_pragma_openacc:
-    return ParseOpenACCDirective();
+    return ParseOpenACCDirectiveDecl();
 
   default:
     if (tok::isPragmaAnnotation(Tok.getKind())) {
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 2fba6cd2805cf97..ed4ad440b9972b6 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -10,18 +10,240 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Basic/OpenACCKinds.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/Parser.h"
+#include "clang/Parse/RAIIObjectsForParser.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
 
 using namespace clang;
+using namespace llvm;
+
+namespace {
+// An enum that contains the extended 'partial' parsed variants. This type
+// should never escape the initial parse functionality, but is useful for
+// simplifying the implementation.
+enum class OpenACCDirectiveKindEx {
+  Invalid = static_cast<int>(OpenACCDirectiveKind::Invalid),
+  // 'enter data' and 'exit data'
+  Enter,
+  Exit,
+  // 'atomic read', 'atomic write', 'atomic update', and 'atomic capture'.
+  Atomic,
+};
+
+// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token), and doesn't fully handle 'enter data', 'exit
+// data', nor any of the 'atomic' variants, just the first token of each.  So
+// this should only be used by `ParseOpenACCDirectiveKind`.
+OpenACCDirectiveKindEx GetOpenACCDirectiveKind(StringRef Name) {
+  OpenACCDirectiveKind DirKind =
+      llvm::StringSwitch<OpenACCDirectiveKind>(Name)
+          .Case("parallel", OpenACCDirectiveKind::Parallel)
+          .Case("serial", OpenACCDirectiveKind::Serial)
+          .Case("kernels", OpenACCDirectiveKind::Kernels)
+          .Case("data", OpenACCDirectiveKind::Data)
+          .Case("host_data", OpenACCDirectiveKind::HostData)
+          .Case("loop", OpenACCDirectiveKind::Loop)
+          .Case("cache", OpenACCDirectiveKind::Cache)
+          .Case("declare", OpenACCDirectiveKind::Declare)
+          .Case("init", OpenACCDirectiveKind::Init)
+          .Case("shutdown", OpenACCDirectiveKind::Shutdown)
+          .Case("set", OpenACCDirectiveKind::Shutdown)
+          .Case("update", OpenACCDirectiveKind::Update)
+          .Case("wait", OpenACCDirectiveKind::Wait)
+          .Case("routine", OpenACCDirectiveKind::Routine)
+          .Default(OpenACCDirectiveKind::Invalid);
+
+  if (DirKind != OpenACCDirectiveKind::Invalid)
+    return static_cast<OpenACCDirectiveKindEx>(DirKind);
+
+  return llvm::StringSwitch<OpenACCDirectiveKindEx>(Name)
+      .Case("enter", OpenACCDirectiveKindEx::Enter)
+      .Case("exit", OpenACCDirectiveKindEx::Exit)
+      .Case("atomic", OpenACCDirectiveKindEx::Atomic)
+      .Default(OpenACCDirectiveKindEx::Invalid);
+}
+
+// "enter data" and "exit data" are permitted as their own constructs. Handle
+// these, knowing the previous token is either 'enter' or 'exit'. The current
+// token should be the one after the "enter" or "exit".
+OpenACCDirectiveKind
+ParseOpenACCEnterExitDataDirective(Parser &P, Token FirstTok,
+                                   StringRef FirstTokSpelling,
+                                   OpenACCDirectiveKindEx ExtDirKind) {
+  Token SecondTok = P.getCurToken();
+  std::string SecondTokSpelling = P.getPreprocessor().getSpelling(SecondTok);
+
+  if (SecondTokSpelling != "data") {
+    P.Diag(FirstTok, diag::err_acc_invalid_directive)
+        << 1 << FirstTokSpelling << SecondTokSpelling;
+    return OpenACCDirectiveKind::Invalid;
+  }
+
+  P.ConsumeToken();
+  return ExtDirKind == OpenACCDirectiveKindEx::Enter
+             ? OpenACCDirectiveKind::EnterData
+             : OpenACCDirectiveKind::ExitData;
+}
+
+OpenACCDirectiveKind ParseOpenACCAtomicDirective(Parser &P) {
+  Token AtomicClauseToken = P.getCurToken();
+  std::string AtomicClauseSpelling =
+      P.getPreprocessor().getSpelling(AtomicClauseToken);
+
+  OpenACCDirectiveKind DirKind =
+      llvm::StringSwitch<OpenACCDirectiveKind>(AtomicClauseSpelling)
+          .Case("read", OpenACCDirectiveKind::AtomicRead)
+          .Case("write", OpenACCDirectiveKind::AtomicWrite)
+          .Case("update", OpenACCDirectiveKind::AtomicUpdate)
+          .Case("capture", OpenACCDirectiveKind::AtomicCapture)
+          .Default(OpenACCDirectiveKind::Invalid);
+
+  if (DirKind == OpenACCDirectiveKind::Invalid)
+    P.Diag(AtomicClauseToken, diag::err_acc_invalid_atomic_clause)
+        << AtomicClauseSpelling;
+
+  P.ConsumeToken();
+  return DirKind;
+}
+
+// Parse and consume the tokens for OpenACC Directive/Construct kinds.
+OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
+  Token FirstTok = P.getCurToken();
+  P.ConsumeToken();
+  std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
+
+  OpenACCDirectiveKindEx ExDirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+
+  Token SecondTok = P.getCurToken();
+  // Go through the Extended kinds to see if we can convert this to the
+  // non-Extended kinds, and handle invalid.
+  switch (ExDirKind) {
+  case OpenACCDirectiveKindEx::Invalid:
+    P.Diag(FirstTok, diag::err_acc_invalid_directive) << 0 << FirstTokSpelling;
+    return OpenACCDirectiveKind::Invalid;
+  case OpenACCDirectiveKindEx::Enter:
+  case OpenACCDirectiveKindEx::Exit:
+    return ParseOpenACCEnterExitDataDirective(P, FirstTok, FirstTokSpelling,
+                                              ExDirKind);
+  case OpenACCDirectiveKindEx::Atomic:
+    return ParseOpenACCAtomicDirective(P);
+  }
+
+  // Combined Constructs allows parallel loop, serial loop, or kernels loop. Any
+  // other attempt at a combined constructwill be diagnosed as an invalid
+  // clause.
+
+  switch (static_cast<OpenACCDirectiveKind>(ExDirKind)) {
+  default:
+    break;
+  case OpenACCDirectiveKind::Parallel:
+    if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+      P.ConsumeToken();
+      return OpenACCDirectiveKind::ParallelLoop;
+    }
+    break;
+  case OpenACCDirectiveKind::Serial:
+    if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+      P.ConsumeToken();
+      return OpenACCDirectiveKind::SerialLoop;
+    }
+    break;
+  case OpenACCDirectiveKind::Kernels:
+    if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+      P.ConsumeToken();
+      return OpenACCDirectiveKind::KernelsLoop;
+    }
+    break;
+  }
+
+  return static_cast<OpenACCDirectiveKind>(ExDirKind);
+}
+
+void ParseOpenACCClauseList(Parser &P) {
+  // FIXME: In the future, we'll start parsing the clauses here, but for now we
+  // haven't implemented that, so just emit the unimplemented diagnostic and
+  // fail reasonably.
+  if (P.getCurToken().isNot(tok::annot_pragma_openacc_end))
+    P.Diag(P.getCurToken(), diag::warn_pragma_acc_unimplemented_clause_parsing);
+}
+
+void ParseOpenACCDirective(Parser &P) {
+  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(P);
+
+  if (DirKind == OpenACCDirectiveKind::Invalid) {
+    P.SkipUntil(tok::annot_pragma_openacc_end);
+    return;
+  }
+
+  // We've successfully parsed the construct/directive name, a few require
+  // special parsing which we will attempt, then revert to clause parsing.
+  BalancedDelimiterTracker T(P, tok::l_paren, tok::annot_pragma_openacc_end);
+
+  // Before we can parse clauses, there are a few directives that have special
+  // cases that need parsing before clauses:
+  //
+  // 1- 'cache' doesn't take clauses, just a 'var list'.
+  // 2- 'wait' has an optional wait-argument.
+  // 3- 'routine' takes an optional 'name' before the clause list.
+  //
+  // As these are not implemented, diagnose, and continue.
+  if (!T.consumeOpen()) {
+    switch (DirKind) {
+    default:
+      P.Diag(T.getOpenLocation(), diag::err_acc_invalid_open_paren);
+      break;
+    case OpenACCDirectiveKind::Cache:
+      P.Diag(T.getOpenLocation(),
+             diag::warn_pragma_acc_unimplemented_construct_parens)
+          << 0;
+      break;
+    case OpenACCDirectiveKind::Wait:
+      P.Diag(T.getOpenLocation(),
+             diag::warn_pragma_acc_unimplemented_construct_parens)
+          << 1;
+      break;
+    case OpenACCDirectiveKind::Routine:
+      P.Diag(T.getOpenLocation(),
+             diag::warn_pragma_acc_unimplemented_construct_parens)
+          << 2;
+      break;
+    }
+    T.skipToEnd();
+  }
+
+  // Parses the list of clauses, if present.
+  ParseOpenACCClauseList(P);
+
+  P.Diag(P.getCurToken(), diag::warn_pragma_acc_unimplemented);
+  P.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");
+
+  ParsingOpenACCDirectiveRAII DirScope(*this);
+  ConsumeAnnotationToken();
+
+  ParseOpenACCDirective(*this);
 
-Parser::DeclGroupPtrTy Parser::ParseOpenACCDirective() {
-  Diag(Tok, diag::warn_pragma_acc_unimplemented);
-  SkipUntil(tok::annot_pragma_openacc_end);
   return nullptr;
 }
+
+// Parse OpenACC Directive on a Statement.
 StmtResult Parser::ParseOpenACCDirectiveStmt() {
-  Diag(Tok, diag::warn_pragma_acc_unimplemented);
-  SkipUntil(tok::annot_pragma_openacc_end);
+  assert(Tok.is(tok::annot_pragma_openacc) && "expected OpenACC Start Token");
+
+  ParsingOpenACCDirectiveRAII DirScope(*this);
+  ConsumeAnnotationToken();
+
+  ParseOpenACCDirective(*this);
+
   return StmtEmpty();
 }
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 41b74624bf1b765..1baeb2aeb021faa 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -320,8 +320,9 @@ bool Parser::SkipUntil(ArrayRef<tok::TokenKind> Toks, SkipUntilFlags Flags) {
       break;
     case tok::annot_pragma_openacc:
     case tok::annot_pragma_openacc_end:
-      // FIXME: Like OpenMP above, we should not be doing this if we're parsing
-      // an OpenACC Directive.
+      // Stop before an OpenACC pragma boundary.
+      if (OpenACCDirectiveParsing)
+        return false;
       ConsumeAnnotationToken();
       break;
     case tok::annot_module_begin:
@@ -858,7 +859,7 @@ Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
     return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs);
   }
   case tok::annot_pragma_openacc:
-    return ParseOpenACCDirective();
+    return ParseOpenACCDirectiveDecl();
   case tok::annot_pragma_ms_pointers_to_members:
     HandlePragmaMSPointersToMembers();
     return nullptr;
diff --git a/clang/test/ParserOpenACC/parse-constructs.c b/clang/test/ParserOpenACC/parse-constructs.c
new file mode 100644
index 000000000000000..8e7f84d3f0d424e
--- /dev/null
+++ b/clang/test/ParserOpenACC/parse-constructs.c
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 %s -verify -fopenacc
+
+
+
+void func() {
+
+  //expected-error@+1{{invalid OpenACC directive 'invalid'}}
+#pragma acc invalid
+  for(;;){}
+
+  // 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@+3{{expected clause-list or newline in pragma 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-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc serial clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc kernels clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc data clause list
+  for(;;){}
+  // expected-error@+1{{invalid OpenACC directive 'enter invalid'}}
+#pragma acc enter invalid clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc enter data clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc exit data clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc host_data clause list
+  for(;;){}
+  // expected-warning@+2{{OpenACC clause parsing not yet implemented}}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma acc loop clause list
+  for(;;){}
+  // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+#pragma ac...
[truncated]

Copy link

github-actions bot commented Nov 17, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

// Directive, but we're calling it a Directive to be consistent with OpenMP.
enum class OpenACCDirectiveKind {
// Compute Constructs.
Parallel,
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest starting with a single construct support, say parallel. Other constructs must be added later, when each particular feature is going to be implemented.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Our implementation strategy is to start with all of the 'parsing' first, starting at constructs, I don't see why this is an issue? I understand the grammar enough that this seems valid.

At this point, we HAVE successfully implemented parsing for all of the 'non-optional-parens' versions.

Doing it this way permits us to do the implementation of clauses next: clauses in OpenACC are shared between all of the constructs, so implementing 1 construct at a time would prevent us from being able to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To clarify: Are you asking me to split this patch up into 23 different ones here? The first adding the infrastructure + parallel, followed by most of the 22 that are just adding an entry here and to a string-switch?

Copy link
Member

Choose a reason for hiding this comment

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

No, not neccesary. The first patch, that lands main infrastructure, better to have a small parsing functionality, the future patch(es) may include parsing of other stuff, if it small enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright then, I'll do that.

@@ -3531,7 +3535,7 @@ 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.
DeclGroupPtrTy ParseOpenACCDirective();
DeclGroupPtrTy ParseOpenACCDirectiveDecl();
Copy link
Member

Choose a reason for hiding this comment

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

Make it in a separate NFC patch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

// An enum that contains the extended 'partial' parsed variants. This type
// should never escape the initial parse functionality, but is useful for
// simplifying the implementation.
enum class OpenACCDirectiveKindEx {
Copy link
Member

Choose a reason for hiding this comment

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

Same, start with a first simple construct support.

// clause.

switch (static_cast<OpenACCDirectiveKind>(ExDirKind)) {
default:
Copy link
Member

Choose a reason for hiding this comment

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

Not recommended to use default, list of possible values explicitly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Eh? We do this sort of thing all the time, the whole point is that we're intentionally not handling EVERYTHING else, since these are the only ones needing special treatment.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That doesn't apply here, these aren't fully-covered switches. They are intentionally partially covered switches with a default for 'rest'.

Copy link
Member

Choose a reason for hiding this comment

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

It just this rule recommends to have full covered switch for enums to be able to catch corner cases where some value is not caught correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thats not my understanding. The point is that if you have:

enum MyE {
A, B, C, D, E };

void foo(MyE e) {
  switch (e) {
     case A:
     case B:
     case C:
     case D:
     case E:
         break;
     default: // don't do this, it is fully covered, and now covers up the warning if something is added to MyE
  }
}

default:
break;
case OpenACCDirectiveKind::Parallel:
if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
Copy link
Member

Choose a reason for hiding this comment

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

Same, start with a simple parallel at first

@erichkeane erichkeane added the clang:openacc Clang OpenACC Implementation label Nov 17, 2023
@erichkeane erichkeane changed the title [OpenACC] Implement initial parsing for Construct/Directive Names [OpenACC] Implement initial parsing for parallel construct Nov 17, 2023
@erichkeane
Copy link
Collaborator Author

@alexey-bataev : Patch reduced to just parallel as requested.

@erichkeane erichkeane merged commit 64b6ef0 into llvm:main Nov 17, 2023
3 checks passed
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
)

As the first real parsing effort for the OpenACC implementation effort,
this implements the parsing for first construct/directive name. This
does not do any semantic analysis, nor any of the clauses. Those will
come in a future patch.

For the time being, we warn when we hit a point that we don't implement
the parsing for either of these situations.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
)

As the first real parsing effort for the OpenACC implementation effort,
this implements the parsing for first construct/directive name. This
does not do any semantic analysis, nor any of the clauses. Those will
come in a future patch.

For the time being, we warn when we hit a point that we don't implement
the parsing for either of these situations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openacc Clang OpenACC Implementation clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants