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 Default clause for Compute Constructs #88135

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

erichkeane
Copy link
Collaborator

As a followup to my previous commits, this is an implementation of a single clause, in this case the 'default' clause. This implements all semantic analysis for it on compute clauses, and continues to leave it rejected for all others (some as 'doesnt appertain', others as 'not implemented' as appropriate).

This also implements and tests the TreeTransform as requested in the previous patch.

[OpenACC] Implement Default clause for Compute Constructs

As a followup to my previous commits, this is an implementation of a
single clause, in this case the 'default' clause.  This implements all
semantic analysis for it on compute clauses, and continues to leave it
rejected for all others (some as 'doesnt appertain', others as 'not
implemented' as appropriate).

This also implements and tests the TreeTransform as requested in the
previous patch.
@erichkeane erichkeane added the clang:openacc Clang OpenACC Implementation label Apr 9, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Apr 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-clang-modules

Author: Erich Keane (erichkeane)

Changes

As a followup to my previous commits, this is an implementation of a single clause, in this case the 'default' clause. This implements all semantic analysis for it on compute clauses, and continues to leave it rejected for all others (some as 'doesnt appertain', others as 'not implemented' as appropriate).

This also implements and tests the TreeTransform as requested in the previous patch.


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

17 Files Affected:

  • (modified) clang/include/clang/AST/OpenACCClause.h (+38)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
  • (modified) clang/include/clang/Basic/OpenACCKinds.h (+23)
  • (modified) clang/include/clang/Sema/SemaOpenACC.h (+16-1)
  • (modified) clang/lib/AST/OpenACCClause.cpp (+19)
  • (modified) clang/lib/AST/StmtProfile.cpp (+5)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+11)
  • (modified) clang/lib/Parse/ParseOpenACC.cpp (+6-2)
  • (modified) clang/lib/Sema/SemaOpenACC.cpp (+58-7)
  • (modified) clang/lib/Sema/TreeTransform.h (+39-3)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+6-1)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+6-1)
  • (modified) clang/test/ParserOpenACC/parse-clauses.c (+6-14)
  • (modified) clang/test/SemaOpenACC/compute-construct-ast.cpp (+10-4)
  • (added) clang/test/SemaOpenACC/compute-construct-clause-ast.cpp (+74)
  • (added) clang/test/SemaOpenACC/compute-construct-default-clause.c (+55)
  • (added) clang/test/SemaOpenACC/compute-construct-default-clause.cpp (+39)
diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h
index 06a0098bbda4cd..c979550ed00fc3 100644
--- a/clang/include/clang/AST/OpenACCClause.h
+++ b/clang/include/clang/AST/OpenACCClause.h
@@ -51,6 +51,36 @@ class OpenACCClauseWithParams : public OpenACCClause {
   SourceLocation getLParenLoc() const { return LParenLoc; }
 };
 
+// A 'default' clause, has the optional 'none' or 'present' argument.
+class OpenACCDefaultClause : public OpenACCClauseWithParams {
+  friend class ASTReaderStmt;
+  friend class ASTWriterStmt;
+
+  OpenACCDefaultClauseKind DefaultClauseKind;
+
+protected:
+  OpenACCDefaultClause(OpenACCDefaultClauseKind K, SourceLocation BeginLoc,
+                       SourceLocation LParenLoc, SourceLocation EndLoc)
+      : OpenACCClauseWithParams(OpenACCClauseKind::Default, BeginLoc, LParenLoc,
+                                EndLoc),
+        DefaultClauseKind(K) {
+    assert((DefaultClauseKind == OpenACCDefaultClauseKind::None ||
+            DefaultClauseKind == OpenACCDefaultClauseKind::Present) &&
+           "Invalid Clause Kind");
+  }
+
+public:
+  OpenACCDefaultClauseKind getDefaultClauseKind() const {
+    return DefaultClauseKind;
+  }
+
+  static OpenACCDefaultClause *Create(const ASTContext &C,
+                                      OpenACCDefaultClauseKind K,
+                                      SourceLocation BeginLoc,
+                                      SourceLocation LParenLoc,
+                                      SourceLocation EndLoc);
+};
+
 template <class Impl> class OpenACCClauseVisitor {
   Impl &getDerived() { return static_cast<Impl &>(*this); }
 
@@ -66,6 +96,8 @@ template <class Impl> class OpenACCClauseVisitor {
 
     switch (C->getClauseKind()) {
     case OpenACCClauseKind::Default:
+      VisitOpenACCDefaultClause(*static_cast<const OpenACCDefaultClause *>(C));
+      return;
     case OpenACCClauseKind::Finalize:
     case OpenACCClauseKind::IfPresent:
     case OpenACCClauseKind::Seq:
@@ -112,6 +144,10 @@ template <class Impl> class OpenACCClauseVisitor {
     }
     llvm_unreachable("Invalid Clause kind");
   }
+
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause) {
+    return getDerived().VisitOpenACCDefaultClause(Clause);
+  }
 };
 
 class OpenACCClausePrinter final
@@ -128,6 +164,8 @@ class OpenACCClausePrinter final
     }
   }
   OpenACCClausePrinter(raw_ostream &OS) : OS(OS) {}
+
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause);
 };
 
 } // namespace clang
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4fbbc42273ba93..ad9abd121bab6f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12254,6 +12254,10 @@ def err_acc_construct_appertainment
             "be used in a statement context">;
 def err_acc_clause_appertainment
     : Error<"OpenACC '%1' clause is not valid on '%0' directive">;
+def err_acc_duplicate_clause_diallowed
+    : Error<"OpenACC '%1' clause cannot appear more than once on a '%0' "
+            "directive">;
+def note_acc_previous_clause_here : Note<"previous clause is here">;
 def err_acc_branch_in_out_compute_construct
     : Error<"invalid %select{branch|return|throw}0 %select{out of|into}1 "
             "OpenACC Compute Construct">;
diff --git a/clang/include/clang/Basic/OpenACCKinds.h b/clang/include/clang/Basic/OpenACCKinds.h
index 4456f4afd142df..eba2a642f4dc0b 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -419,6 +419,29 @@ enum class OpenACCDefaultClauseKind {
   Invalid,
 };
 
+template <typename StreamTy>
+inline StreamTy &PrintOpenACCDefaultClauseKind(StreamTy &Out,
+                                               OpenACCDefaultClauseKind K) {
+  switch (K) {
+  case OpenACCDefaultClauseKind::None:
+    return Out << "none";
+  case OpenACCDefaultClauseKind::Present:
+    return Out << "present";
+  case OpenACCDefaultClauseKind::Invalid:
+    return Out << "<invalid>";
+  }
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &Out,
+                                             OpenACCDefaultClauseKind K) {
+  return PrintOpenACCDefaultClauseKind(Out, K);
+}
+
+inline llvm::raw_ostream &operator<<(llvm::raw_ostream &Out,
+                                     OpenACCDefaultClauseKind K) {
+  return PrintOpenACCDefaultClauseKind(Out, K);
+}
+
 enum class OpenACCReductionOperator {
   /// '+'.
   Addition,
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index 45929e4a9db3f1..3b27e45e240772 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/SemaBase.h"
+#include <variant>
 
 namespace clang {
 class OpenACCClause;
@@ -35,7 +36,11 @@ class SemaOpenACC : public SemaBase {
     SourceRange ClauseRange;
     SourceLocation LParenLoc;
 
-    // TODO OpenACC: Add variant here to store details of individual clauses.
+    struct DefaultDetails {
+      OpenACCDefaultClauseKind DefaultClauseKind;
+    };
+
+    std::variant<DefaultDetails> Details;
 
   public:
     OpenACCParsedClause(OpenACCDirectiveKind DirKind,
@@ -52,8 +57,18 @@ class SemaOpenACC : public SemaBase {
 
     SourceLocation getEndLoc() const { return ClauseRange.getEnd(); }
 
+    OpenACCDefaultClauseKind getDefaultClauseKind() const {
+      assert(ClauseKind == OpenACCClauseKind::Default);
+      return std::get<DefaultDetails>(Details).DefaultClauseKind;
+    }
+
     void setLParenLoc(SourceLocation EndLoc) { LParenLoc = EndLoc; }
     void setEndLoc(SourceLocation EndLoc) { ClauseRange.setEnd(EndLoc); }
+
+    void setDefaultDetails(OpenACCDefaultClauseKind DefKind) {
+      assert(ClauseKind == OpenACCClauseKind::Default);
+      Details = DefaultDetails{DefKind};
+    }
   };
 
   SemaOpenACC(Sema &S);
diff --git a/clang/lib/AST/OpenACCClause.cpp b/clang/lib/AST/OpenACCClause.cpp
index e1db872f25c322..24d667d8ba5665 100644
--- a/clang/lib/AST/OpenACCClause.cpp
+++ b/clang/lib/AST/OpenACCClause.cpp
@@ -15,3 +15,22 @@
 #include "clang/AST/ASTContext.h"
 
 using namespace clang;
+
+OpenACCDefaultClause *OpenACCDefaultClause::Create(const ASTContext &C,
+                                                   OpenACCDefaultClauseKind K,
+                                                   SourceLocation BeginLoc,
+                                                   SourceLocation LParenLoc,
+                                                   SourceLocation EndLoc) {
+  void *Mem =
+      C.Allocate(sizeof(OpenACCDefaultClause), alignof(OpenACCDefaultClause));
+
+  return new (Mem) OpenACCDefaultClause(K, BeginLoc, LParenLoc, EndLoc);
+}
+
+//===----------------------------------------------------------------------===//
+//  OpenACC clauses printing methods
+//===----------------------------------------------------------------------===//
+void OpenACCClausePrinter::VisitOpenACCDefaultClause(
+    const OpenACCDefaultClause &C) {
+  OS << "default(" << C.getDefaultClauseKind();
+}
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index bec8bc71f5554c..be3dd4b673cf98 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2456,7 +2456,12 @@ class OpenACCClauseProfiler
       Visit(Clause);
     }
   }
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause);
 };
+
+/// Nothing to do here, there are no sub-statements.
+void OpenACCClauseProfiler::VisitOpenACCDefaultClause(
+    const OpenACCDefaultClause &Clause) {}
 } // namespace
 
 void StmtProfiler::VisitOpenACCComputeConstruct(
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 431f5d8bdb2b5f..085a7f51ce99ad 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -390,6 +390,17 @@ void TextNodeDumper::Visit(const OpenACCClause *C) {
   {
     ColorScope Color(OS, ShowColors, AttrColor);
     OS << C->getClauseKind();
+
+    // Handle clauses with parens for types that have no children, likely
+    // because there is no sub expression.
+    switch (C->getClauseKind()) {
+    case OpenACCClauseKind::Default:
+      OS << '(' << cast<OpenACCDefaultClause>(C)->getDefaultClauseKind() << ')';
+      break;
+    default:
+      // Nothing to do here.
+      break;
+    }
   }
   dumpPointer(C);
   dumpSourceRange(SourceRange(C->getBeginLoc(), C->getEndLoc()));
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index f434e1542c801f..59a4a5f5346762 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -831,9 +831,13 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams(
 
       ConsumeToken();
 
-      if (getOpenACCDefaultClauseKind(DefKindTok) ==
-          OpenACCDefaultClauseKind::Invalid)
+      OpenACCDefaultClauseKind DefKind =
+          getOpenACCDefaultClauseKind(DefKindTok);
+
+      if (DefKind == OpenACCDefaultClauseKind::Invalid)
         Diag(DefKindTok, diag::err_acc_invalid_default_clause_kind);
+      else
+        ParsedClause.setDefaultDetails(DefKind);
 
       break;
     }
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 2ba1e49b5739db..2c806f5f72b7b3 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -39,11 +39,27 @@ bool diagnoseConstructAppertainment(SemaOpenACC &S, OpenACCDirectiveKind K,
 
 bool doesClauseApplyToDirective(OpenACCDirectiveKind DirectiveKind,
                                 OpenACCClauseKind ClauseKind) {
-  // FIXME: For each clause as we implement them, we can add the
-  // 'legalization' list here.
-
-  // Do nothing so we can go to the 'unimplemented' diagnostic instead.
-  return true;
+  switch (ClauseKind) {
+    // FIXME: For each clause as we implement them, we can add the
+    // 'legalization' list here.
+  case OpenACCClauseKind::Default:
+    switch (DirectiveKind) {
+    case OpenACCDirectiveKind::Parallel:
+    case OpenACCDirectiveKind::Serial:
+    case OpenACCDirectiveKind::Kernels:
+    case OpenACCDirectiveKind::ParallelLoop:
+    case OpenACCDirectiveKind::SerialLoop:
+    case OpenACCDirectiveKind::KernelsLoop:
+    case OpenACCDirectiveKind::Data:
+      return true;
+    default:
+      return false;
+    }
+  default:
+    // Do nothing so we can go to the 'unimplemented' diagnostic instead.
+    return true;
+  }
+  llvm_unreachable("Invalid clause kind");
 }
 } // namespace
 
@@ -63,8 +79,43 @@ SemaOpenACC::ActOnClause(ArrayRef<const OpenACCClause *> ExistingClauses,
     return nullptr;
   }
 
-  // TODO OpenACC: Switch over the clauses we implement here and 'create'
-  // them.
+  switch (Clause.getClauseKind()) {
+  case OpenACCClauseKind::Default: {
+    // Restrictions only properly implemented on 'compute' constructs, and
+    // 'compute' constructs are the only construct that can do anything with
+    // this yet, so skip/treat as unimplemented in this case.
+    if (Clause.getDirectiveKind() != OpenACCDirectiveKind::Parallel &&
+        Clause.getDirectiveKind() != OpenACCDirectiveKind::Serial &&
+        Clause.getDirectiveKind() != OpenACCDirectiveKind::Kernels)
+      break;
+
+    // Don't add an invalid clause to the AST.
+    if (Clause.getDefaultClauseKind() == OpenACCDefaultClauseKind::Invalid)
+      return nullptr;
+
+    // OpenACC 3.3, Section 2.5.4:
+    // At most one 'default' clause may appear, and it must have a value of
+    // either 'none' or 'present'.
+    // Second half of the sentence is diagnosed during parsing.
+    auto Itr = llvm::find_if(ExistingClauses, [](const OpenACCClause *C) {
+      return C->getClauseKind() == OpenACCClauseKind::Default;
+    });
+
+    if (Itr != ExistingClauses.end()) {
+      SemaRef.Diag(Clause.getBeginLoc(),
+                   diag::err_acc_duplicate_clause_diallowed)
+          << Clause.getDirectiveKind() << Clause.getClauseKind();
+      SemaRef.Diag((*Itr)->getBeginLoc(), diag::note_acc_previous_clause_here);
+      return nullptr;
+    }
+
+    return OpenACCDefaultClause::Create(
+        getASTContext(), Clause.getDefaultClauseKind(), Clause.getBeginLoc(),
+        Clause.getLParenLoc(), Clause.getEndLoc());
+  }
+  default:
+    break;
+  }
 
   Diag(Clause.getBeginLoc(), diag::warn_acc_clause_unimplemented)
       << Clause.getClauseKind();
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 13d7b00430d523..25e011a8ba6d84 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4034,6 +4034,11 @@ class TreeTransform {
   llvm::SmallVector<OpenACCClause *>
   TransformOpenACCClauseList(OpenACCDirectiveKind DirKind,
                              ArrayRef<const OpenACCClause *> OldClauses);
+
+  OpenACCClause *
+  TransformOpenACCClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                         OpenACCDirectiveKind DirKind,
+                         const OpenACCClause *OldClause);
 };
 
 template <typename Derived>
@@ -11074,13 +11079,44 @@ OMPClause *TreeTransform<Derived>::TransformOMPXBareClause(OMPXBareClause *C) {
 //===----------------------------------------------------------------------===//
 // OpenACC transformation
 //===----------------------------------------------------------------------===//
+template <typename Derived>
+OpenACCClause *TreeTransform<Derived>::TransformOpenACCClause(
+    ArrayRef<const OpenACCClause *> ExistingClauses,
+    OpenACCDirectiveKind DirKind, const OpenACCClause *OldClause) {
+
+  SemaOpenACC::OpenACCParsedClause ParsedClause(
+      DirKind, OldClause->getClauseKind(), OldClause->getBeginLoc());
+  ParsedClause.setEndLoc(OldClause->getEndLoc());
+
+  if (const auto *WithParms = dyn_cast<OpenACCClauseWithParams>(OldClause))
+    ParsedClause.setLParenLoc(WithParms->getLParenLoc());
+
+  switch (OldClause->getClauseKind()) {
+  case OpenACCClauseKind::Default:
+    // There is nothing to do here as nothing dependent can appear in this
+    // clause. So just set the values so Sema can set the right value.
+    ParsedClause.setDefaultDetails(
+        cast<OpenACCDefaultClause>(OldClause)->getDefaultClauseKind());
+    break;
+  default:
+    assert(false && "Unhandled OpenACC clause in TreeTransform");
+    return nullptr;
+  }
+
+  return getSema().OpenACC().ActOnClause(ExistingClauses, ParsedClause);
+}
+
 template <typename Derived>
 llvm::SmallVector<OpenACCClause *>
 TreeTransform<Derived>::TransformOpenACCClauseList(
     OpenACCDirectiveKind DirKind, ArrayRef<const OpenACCClause *> OldClauses) {
-  // TODO OpenACC: Ensure we loop through the list and transform the individual
-  // clauses.
-  return {};
+  llvm::SmallVector<OpenACCClause *> TransformedClauses;
+  for (const auto *Clause : OldClauses) {
+    if (OpenACCClause *TransformedClause = getDerived().TransformOpenACCClause(
+            TransformedClauses, DirKind, Clause))
+      TransformedClauses.push_back(TransformedClause);
+  }
+  return TransformedClauses;
 }
 
 template <typename Derived>
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index fa5bb9f2d5435a..679302e7a838f1 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -11763,7 +11763,12 @@ OpenACCClause *ASTRecordReader::readOpenACCClause() {
   [[maybe_unused]] SourceLocation EndLoc = readSourceLocation();
 
   switch (ClauseKind) {
-  case OpenACCClauseKind::Default:
+  case OpenACCClauseKind::Default: {
+    SourceLocation LParenLoc = readSourceLocation();
+    OpenACCDefaultClauseKind DCK = readEnum<OpenACCDefaultClauseKind>();
+    return OpenACCDefaultClause::Create(getContext(), DCK, BeginLoc, LParenLoc,
+                                        EndLoc);
+  }
   case OpenACCClauseKind::Finalize:
   case OpenACCClauseKind::IfPresent:
   case OpenACCClauseKind::Seq:
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index baf03f69d73065..4cd74b1ba9d72d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -7406,7 +7406,12 @@ void ASTRecordWriter::writeOpenACCClause(const OpenACCClause *C) {
   writeSourceLocation(C->getEndLoc());
 
   switch (C->getClauseKind()) {
-  case OpenACCClauseKind::Default:
+  case OpenACCClauseKind::Default: {
+    const auto *DC = cast<OpenACCDefaultClause>(C);
+    writeSourceLocation(DC->getLParenLoc());
+    writeEnum(DC->getDefaultClauseKind());
+    return;
+  }
   case OpenACCClauseKind::Finalize:
   case OpenACCClauseKind::IfPresent:
   case OpenACCClauseKind::Seq:
diff --git a/clang/test/ParserOpenACC/parse-clauses.c b/clang/test/ParserOpenACC/parse-clauses.c
index b58b332ad3245c..b363a0cb1362b0 100644
--- a/clang/test/ParserOpenACC/parse-clauses.c
+++ b/clang/test/ParserOpenACC/parse-clauses.c
@@ -173,45 +173,37 @@ void DefaultClause() {
 #pragma acc serial default), seq
   for(;;){}
 
-  // expected-error@+2{{expected identifier}}
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+1{{expected identifier}}
 #pragma acc serial default()
   for(;;){}
 
-  // expected-error@+3{{expected identifier}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{expected identifier}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default() seq
   for(;;){}
 
-  // expected-error@+3{{expected identifier}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{expected identifier}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(), seq
   for(;;){}
 
-  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+1{{invalid value for 'default' clause; expected 'present' or 'none'}}
 #pragma acc serial default(invalid)
   for(;;){}
 
-  // expected-error@+3{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(auto) seq
   for(;;){}
 
-  // expected-error@+3{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(invalid), seq
   for(;;){}
 
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
 #pragma acc serial default(none)
   for(;;){}
 
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(present), seq
   for(;;){}
diff --git a/clang/test/SemaOpenACC/compute-construct-ast.cpp b/clang/test/SemaOpenACC/compute-construct-ast.cpp
index 55c080838a1886..e632522f877b53 100644
--- a/clang/test/SemaOpenACC/compute-construct-ast.cpp
+++ b/clang/test/SemaOpenACC/compute-construct-ast.cpp
@@ -12,14 +12,16 @@ void NormalFunc() {
   // CHECK-LABEL: NormalFunc
   // CHECK-NEXT: CompoundStmt
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+  // CHECK-NEXT: default(none)
   // CHECK-NEXT: CompoundStmt
-#pragma acc parallel
+#pragma acc parallel default(none)
   {
 #pragma acc parallel
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+  // CHECK-NEXT: default(present)
   // CHECK-NEXT: CompoundStmt
-#pragma acc parallel
+#pragma acc parallel default(present)
     {}
   }
   // FIXME: Add a test once we have clauses for this.
@@ -50,12 +52,12 @@ void ...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

As a followup to my previous commits, this is an implementation of a single clause, in this case the 'default' clause. This implements all semantic analysis for it on compute clauses, and continues to leave it rejected for all others (some as 'doesnt appertain', others as 'not implemented' as appropriate).

This also implements and tests the TreeTransform as requested in the previous patch.


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

17 Files Affected:

  • (modified) clang/include/clang/AST/OpenACCClause.h (+38)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
  • (modified) clang/include/clang/Basic/OpenACCKinds.h (+23)
  • (modified) clang/include/clang/Sema/SemaOpenACC.h (+16-1)
  • (modified) clang/lib/AST/OpenACCClause.cpp (+19)
  • (modified) clang/lib/AST/StmtProfile.cpp (+5)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+11)
  • (modified) clang/lib/Parse/ParseOpenACC.cpp (+6-2)
  • (modified) clang/lib/Sema/SemaOpenACC.cpp (+58-7)
  • (modified) clang/lib/Sema/TreeTransform.h (+39-3)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+6-1)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+6-1)
  • (modified) clang/test/ParserOpenACC/parse-clauses.c (+6-14)
  • (modified) clang/test/SemaOpenACC/compute-construct-ast.cpp (+10-4)
  • (added) clang/test/SemaOpenACC/compute-construct-clause-ast.cpp (+74)
  • (added) clang/test/SemaOpenACC/compute-construct-default-clause.c (+55)
  • (added) clang/test/SemaOpenACC/compute-construct-default-clause.cpp (+39)
diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h
index 06a0098bbda4cd..c979550ed00fc3 100644
--- a/clang/include/clang/AST/OpenACCClause.h
+++ b/clang/include/clang/AST/OpenACCClause.h
@@ -51,6 +51,36 @@ class OpenACCClauseWithParams : public OpenACCClause {
   SourceLocation getLParenLoc() const { return LParenLoc; }
 };
 
+// A 'default' clause, has the optional 'none' or 'present' argument.
+class OpenACCDefaultClause : public OpenACCClauseWithParams {
+  friend class ASTReaderStmt;
+  friend class ASTWriterStmt;
+
+  OpenACCDefaultClauseKind DefaultClauseKind;
+
+protected:
+  OpenACCDefaultClause(OpenACCDefaultClauseKind K, SourceLocation BeginLoc,
+                       SourceLocation LParenLoc, SourceLocation EndLoc)
+      : OpenACCClauseWithParams(OpenACCClauseKind::Default, BeginLoc, LParenLoc,
+                                EndLoc),
+        DefaultClauseKind(K) {
+    assert((DefaultClauseKind == OpenACCDefaultClauseKind::None ||
+            DefaultClauseKind == OpenACCDefaultClauseKind::Present) &&
+           "Invalid Clause Kind");
+  }
+
+public:
+  OpenACCDefaultClauseKind getDefaultClauseKind() const {
+    return DefaultClauseKind;
+  }
+
+  static OpenACCDefaultClause *Create(const ASTContext &C,
+                                      OpenACCDefaultClauseKind K,
+                                      SourceLocation BeginLoc,
+                                      SourceLocation LParenLoc,
+                                      SourceLocation EndLoc);
+};
+
 template <class Impl> class OpenACCClauseVisitor {
   Impl &getDerived() { return static_cast<Impl &>(*this); }
 
@@ -66,6 +96,8 @@ template <class Impl> class OpenACCClauseVisitor {
 
     switch (C->getClauseKind()) {
     case OpenACCClauseKind::Default:
+      VisitOpenACCDefaultClause(*static_cast<const OpenACCDefaultClause *>(C));
+      return;
     case OpenACCClauseKind::Finalize:
     case OpenACCClauseKind::IfPresent:
     case OpenACCClauseKind::Seq:
@@ -112,6 +144,10 @@ template <class Impl> class OpenACCClauseVisitor {
     }
     llvm_unreachable("Invalid Clause kind");
   }
+
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause) {
+    return getDerived().VisitOpenACCDefaultClause(Clause);
+  }
 };
 
 class OpenACCClausePrinter final
@@ -128,6 +164,8 @@ class OpenACCClausePrinter final
     }
   }
   OpenACCClausePrinter(raw_ostream &OS) : OS(OS) {}
+
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause);
 };
 
 } // namespace clang
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4fbbc42273ba93..ad9abd121bab6f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12254,6 +12254,10 @@ def err_acc_construct_appertainment
             "be used in a statement context">;
 def err_acc_clause_appertainment
     : Error<"OpenACC '%1' clause is not valid on '%0' directive">;
+def err_acc_duplicate_clause_diallowed
+    : Error<"OpenACC '%1' clause cannot appear more than once on a '%0' "
+            "directive">;
+def note_acc_previous_clause_here : Note<"previous clause is here">;
 def err_acc_branch_in_out_compute_construct
     : Error<"invalid %select{branch|return|throw}0 %select{out of|into}1 "
             "OpenACC Compute Construct">;
diff --git a/clang/include/clang/Basic/OpenACCKinds.h b/clang/include/clang/Basic/OpenACCKinds.h
index 4456f4afd142df..eba2a642f4dc0b 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -419,6 +419,29 @@ enum class OpenACCDefaultClauseKind {
   Invalid,
 };
 
+template <typename StreamTy>
+inline StreamTy &PrintOpenACCDefaultClauseKind(StreamTy &Out,
+                                               OpenACCDefaultClauseKind K) {
+  switch (K) {
+  case OpenACCDefaultClauseKind::None:
+    return Out << "none";
+  case OpenACCDefaultClauseKind::Present:
+    return Out << "present";
+  case OpenACCDefaultClauseKind::Invalid:
+    return Out << "<invalid>";
+  }
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &Out,
+                                             OpenACCDefaultClauseKind K) {
+  return PrintOpenACCDefaultClauseKind(Out, K);
+}
+
+inline llvm::raw_ostream &operator<<(llvm::raw_ostream &Out,
+                                     OpenACCDefaultClauseKind K) {
+  return PrintOpenACCDefaultClauseKind(Out, K);
+}
+
 enum class OpenACCReductionOperator {
   /// '+'.
   Addition,
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index 45929e4a9db3f1..3b27e45e240772 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/SemaBase.h"
+#include <variant>
 
 namespace clang {
 class OpenACCClause;
@@ -35,7 +36,11 @@ class SemaOpenACC : public SemaBase {
     SourceRange ClauseRange;
     SourceLocation LParenLoc;
 
-    // TODO OpenACC: Add variant here to store details of individual clauses.
+    struct DefaultDetails {
+      OpenACCDefaultClauseKind DefaultClauseKind;
+    };
+
+    std::variant<DefaultDetails> Details;
 
   public:
     OpenACCParsedClause(OpenACCDirectiveKind DirKind,
@@ -52,8 +57,18 @@ class SemaOpenACC : public SemaBase {
 
     SourceLocation getEndLoc() const { return ClauseRange.getEnd(); }
 
+    OpenACCDefaultClauseKind getDefaultClauseKind() const {
+      assert(ClauseKind == OpenACCClauseKind::Default);
+      return std::get<DefaultDetails>(Details).DefaultClauseKind;
+    }
+
     void setLParenLoc(SourceLocation EndLoc) { LParenLoc = EndLoc; }
     void setEndLoc(SourceLocation EndLoc) { ClauseRange.setEnd(EndLoc); }
+
+    void setDefaultDetails(OpenACCDefaultClauseKind DefKind) {
+      assert(ClauseKind == OpenACCClauseKind::Default);
+      Details = DefaultDetails{DefKind};
+    }
   };
 
   SemaOpenACC(Sema &S);
diff --git a/clang/lib/AST/OpenACCClause.cpp b/clang/lib/AST/OpenACCClause.cpp
index e1db872f25c322..24d667d8ba5665 100644
--- a/clang/lib/AST/OpenACCClause.cpp
+++ b/clang/lib/AST/OpenACCClause.cpp
@@ -15,3 +15,22 @@
 #include "clang/AST/ASTContext.h"
 
 using namespace clang;
+
+OpenACCDefaultClause *OpenACCDefaultClause::Create(const ASTContext &C,
+                                                   OpenACCDefaultClauseKind K,
+                                                   SourceLocation BeginLoc,
+                                                   SourceLocation LParenLoc,
+                                                   SourceLocation EndLoc) {
+  void *Mem =
+      C.Allocate(sizeof(OpenACCDefaultClause), alignof(OpenACCDefaultClause));
+
+  return new (Mem) OpenACCDefaultClause(K, BeginLoc, LParenLoc, EndLoc);
+}
+
+//===----------------------------------------------------------------------===//
+//  OpenACC clauses printing methods
+//===----------------------------------------------------------------------===//
+void OpenACCClausePrinter::VisitOpenACCDefaultClause(
+    const OpenACCDefaultClause &C) {
+  OS << "default(" << C.getDefaultClauseKind();
+}
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index bec8bc71f5554c..be3dd4b673cf98 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2456,7 +2456,12 @@ class OpenACCClauseProfiler
       Visit(Clause);
     }
   }
+  void VisitOpenACCDefaultClause(const OpenACCDefaultClause &Clause);
 };
+
+/// Nothing to do here, there are no sub-statements.
+void OpenACCClauseProfiler::VisitOpenACCDefaultClause(
+    const OpenACCDefaultClause &Clause) {}
 } // namespace
 
 void StmtProfiler::VisitOpenACCComputeConstruct(
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 431f5d8bdb2b5f..085a7f51ce99ad 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -390,6 +390,17 @@ void TextNodeDumper::Visit(const OpenACCClause *C) {
   {
     ColorScope Color(OS, ShowColors, AttrColor);
     OS << C->getClauseKind();
+
+    // Handle clauses with parens for types that have no children, likely
+    // because there is no sub expression.
+    switch (C->getClauseKind()) {
+    case OpenACCClauseKind::Default:
+      OS << '(' << cast<OpenACCDefaultClause>(C)->getDefaultClauseKind() << ')';
+      break;
+    default:
+      // Nothing to do here.
+      break;
+    }
   }
   dumpPointer(C);
   dumpSourceRange(SourceRange(C->getBeginLoc(), C->getEndLoc()));
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index f434e1542c801f..59a4a5f5346762 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -831,9 +831,13 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams(
 
       ConsumeToken();
 
-      if (getOpenACCDefaultClauseKind(DefKindTok) ==
-          OpenACCDefaultClauseKind::Invalid)
+      OpenACCDefaultClauseKind DefKind =
+          getOpenACCDefaultClauseKind(DefKindTok);
+
+      if (DefKind == OpenACCDefaultClauseKind::Invalid)
         Diag(DefKindTok, diag::err_acc_invalid_default_clause_kind);
+      else
+        ParsedClause.setDefaultDetails(DefKind);
 
       break;
     }
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 2ba1e49b5739db..2c806f5f72b7b3 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -39,11 +39,27 @@ bool diagnoseConstructAppertainment(SemaOpenACC &S, OpenACCDirectiveKind K,
 
 bool doesClauseApplyToDirective(OpenACCDirectiveKind DirectiveKind,
                                 OpenACCClauseKind ClauseKind) {
-  // FIXME: For each clause as we implement them, we can add the
-  // 'legalization' list here.
-
-  // Do nothing so we can go to the 'unimplemented' diagnostic instead.
-  return true;
+  switch (ClauseKind) {
+    // FIXME: For each clause as we implement them, we can add the
+    // 'legalization' list here.
+  case OpenACCClauseKind::Default:
+    switch (DirectiveKind) {
+    case OpenACCDirectiveKind::Parallel:
+    case OpenACCDirectiveKind::Serial:
+    case OpenACCDirectiveKind::Kernels:
+    case OpenACCDirectiveKind::ParallelLoop:
+    case OpenACCDirectiveKind::SerialLoop:
+    case OpenACCDirectiveKind::KernelsLoop:
+    case OpenACCDirectiveKind::Data:
+      return true;
+    default:
+      return false;
+    }
+  default:
+    // Do nothing so we can go to the 'unimplemented' diagnostic instead.
+    return true;
+  }
+  llvm_unreachable("Invalid clause kind");
 }
 } // namespace
 
@@ -63,8 +79,43 @@ SemaOpenACC::ActOnClause(ArrayRef<const OpenACCClause *> ExistingClauses,
     return nullptr;
   }
 
-  // TODO OpenACC: Switch over the clauses we implement here and 'create'
-  // them.
+  switch (Clause.getClauseKind()) {
+  case OpenACCClauseKind::Default: {
+    // Restrictions only properly implemented on 'compute' constructs, and
+    // 'compute' constructs are the only construct that can do anything with
+    // this yet, so skip/treat as unimplemented in this case.
+    if (Clause.getDirectiveKind() != OpenACCDirectiveKind::Parallel &&
+        Clause.getDirectiveKind() != OpenACCDirectiveKind::Serial &&
+        Clause.getDirectiveKind() != OpenACCDirectiveKind::Kernels)
+      break;
+
+    // Don't add an invalid clause to the AST.
+    if (Clause.getDefaultClauseKind() == OpenACCDefaultClauseKind::Invalid)
+      return nullptr;
+
+    // OpenACC 3.3, Section 2.5.4:
+    // At most one 'default' clause may appear, and it must have a value of
+    // either 'none' or 'present'.
+    // Second half of the sentence is diagnosed during parsing.
+    auto Itr = llvm::find_if(ExistingClauses, [](const OpenACCClause *C) {
+      return C->getClauseKind() == OpenACCClauseKind::Default;
+    });
+
+    if (Itr != ExistingClauses.end()) {
+      SemaRef.Diag(Clause.getBeginLoc(),
+                   diag::err_acc_duplicate_clause_diallowed)
+          << Clause.getDirectiveKind() << Clause.getClauseKind();
+      SemaRef.Diag((*Itr)->getBeginLoc(), diag::note_acc_previous_clause_here);
+      return nullptr;
+    }
+
+    return OpenACCDefaultClause::Create(
+        getASTContext(), Clause.getDefaultClauseKind(), Clause.getBeginLoc(),
+        Clause.getLParenLoc(), Clause.getEndLoc());
+  }
+  default:
+    break;
+  }
 
   Diag(Clause.getBeginLoc(), diag::warn_acc_clause_unimplemented)
       << Clause.getClauseKind();
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 13d7b00430d523..25e011a8ba6d84 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4034,6 +4034,11 @@ class TreeTransform {
   llvm::SmallVector<OpenACCClause *>
   TransformOpenACCClauseList(OpenACCDirectiveKind DirKind,
                              ArrayRef<const OpenACCClause *> OldClauses);
+
+  OpenACCClause *
+  TransformOpenACCClause(ArrayRef<const OpenACCClause *> ExistingClauses,
+                         OpenACCDirectiveKind DirKind,
+                         const OpenACCClause *OldClause);
 };
 
 template <typename Derived>
@@ -11074,13 +11079,44 @@ OMPClause *TreeTransform<Derived>::TransformOMPXBareClause(OMPXBareClause *C) {
 //===----------------------------------------------------------------------===//
 // OpenACC transformation
 //===----------------------------------------------------------------------===//
+template <typename Derived>
+OpenACCClause *TreeTransform<Derived>::TransformOpenACCClause(
+    ArrayRef<const OpenACCClause *> ExistingClauses,
+    OpenACCDirectiveKind DirKind, const OpenACCClause *OldClause) {
+
+  SemaOpenACC::OpenACCParsedClause ParsedClause(
+      DirKind, OldClause->getClauseKind(), OldClause->getBeginLoc());
+  ParsedClause.setEndLoc(OldClause->getEndLoc());
+
+  if (const auto *WithParms = dyn_cast<OpenACCClauseWithParams>(OldClause))
+    ParsedClause.setLParenLoc(WithParms->getLParenLoc());
+
+  switch (OldClause->getClauseKind()) {
+  case OpenACCClauseKind::Default:
+    // There is nothing to do here as nothing dependent can appear in this
+    // clause. So just set the values so Sema can set the right value.
+    ParsedClause.setDefaultDetails(
+        cast<OpenACCDefaultClause>(OldClause)->getDefaultClauseKind());
+    break;
+  default:
+    assert(false && "Unhandled OpenACC clause in TreeTransform");
+    return nullptr;
+  }
+
+  return getSema().OpenACC().ActOnClause(ExistingClauses, ParsedClause);
+}
+
 template <typename Derived>
 llvm::SmallVector<OpenACCClause *>
 TreeTransform<Derived>::TransformOpenACCClauseList(
     OpenACCDirectiveKind DirKind, ArrayRef<const OpenACCClause *> OldClauses) {
-  // TODO OpenACC: Ensure we loop through the list and transform the individual
-  // clauses.
-  return {};
+  llvm::SmallVector<OpenACCClause *> TransformedClauses;
+  for (const auto *Clause : OldClauses) {
+    if (OpenACCClause *TransformedClause = getDerived().TransformOpenACCClause(
+            TransformedClauses, DirKind, Clause))
+      TransformedClauses.push_back(TransformedClause);
+  }
+  return TransformedClauses;
 }
 
 template <typename Derived>
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index fa5bb9f2d5435a..679302e7a838f1 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -11763,7 +11763,12 @@ OpenACCClause *ASTRecordReader::readOpenACCClause() {
   [[maybe_unused]] SourceLocation EndLoc = readSourceLocation();
 
   switch (ClauseKind) {
-  case OpenACCClauseKind::Default:
+  case OpenACCClauseKind::Default: {
+    SourceLocation LParenLoc = readSourceLocation();
+    OpenACCDefaultClauseKind DCK = readEnum<OpenACCDefaultClauseKind>();
+    return OpenACCDefaultClause::Create(getContext(), DCK, BeginLoc, LParenLoc,
+                                        EndLoc);
+  }
   case OpenACCClauseKind::Finalize:
   case OpenACCClauseKind::IfPresent:
   case OpenACCClauseKind::Seq:
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index baf03f69d73065..4cd74b1ba9d72d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -7406,7 +7406,12 @@ void ASTRecordWriter::writeOpenACCClause(const OpenACCClause *C) {
   writeSourceLocation(C->getEndLoc());
 
   switch (C->getClauseKind()) {
-  case OpenACCClauseKind::Default:
+  case OpenACCClauseKind::Default: {
+    const auto *DC = cast<OpenACCDefaultClause>(C);
+    writeSourceLocation(DC->getLParenLoc());
+    writeEnum(DC->getDefaultClauseKind());
+    return;
+  }
   case OpenACCClauseKind::Finalize:
   case OpenACCClauseKind::IfPresent:
   case OpenACCClauseKind::Seq:
diff --git a/clang/test/ParserOpenACC/parse-clauses.c b/clang/test/ParserOpenACC/parse-clauses.c
index b58b332ad3245c..b363a0cb1362b0 100644
--- a/clang/test/ParserOpenACC/parse-clauses.c
+++ b/clang/test/ParserOpenACC/parse-clauses.c
@@ -173,45 +173,37 @@ void DefaultClause() {
 #pragma acc serial default), seq
   for(;;){}
 
-  // expected-error@+2{{expected identifier}}
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+1{{expected identifier}}
 #pragma acc serial default()
   for(;;){}
 
-  // expected-error@+3{{expected identifier}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{expected identifier}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default() seq
   for(;;){}
 
-  // expected-error@+3{{expected identifier}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{expected identifier}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(), seq
   for(;;){}
 
-  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+1{{invalid value for 'default' clause; expected 'present' or 'none'}}
 #pragma acc serial default(invalid)
   for(;;){}
 
-  // expected-error@+3{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(auto) seq
   for(;;){}
 
-  // expected-error@+3{{invalid value for 'default' clause; expected 'present' or 'none'}}
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
+  // expected-error@+2{{invalid value for 'default' clause; expected 'present' or 'none'}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(invalid), seq
   for(;;){}
 
-  // expected-warning@+1{{OpenACC clause 'default' not yet implemented, clause ignored}}
 #pragma acc serial default(none)
   for(;;){}
 
-  // expected-warning@+2{{OpenACC clause 'default' not yet implemented, clause ignored}}
   // expected-warning@+1{{OpenACC clause 'seq' not yet implemented, clause ignored}}
 #pragma acc serial default(present), seq
   for(;;){}
diff --git a/clang/test/SemaOpenACC/compute-construct-ast.cpp b/clang/test/SemaOpenACC/compute-construct-ast.cpp
index 55c080838a1886..e632522f877b53 100644
--- a/clang/test/SemaOpenACC/compute-construct-ast.cpp
+++ b/clang/test/SemaOpenACC/compute-construct-ast.cpp
@@ -12,14 +12,16 @@ void NormalFunc() {
   // CHECK-LABEL: NormalFunc
   // CHECK-NEXT: CompoundStmt
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+  // CHECK-NEXT: default(none)
   // CHECK-NEXT: CompoundStmt
-#pragma acc parallel
+#pragma acc parallel default(none)
   {
 #pragma acc parallel
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
   // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+  // CHECK-NEXT: default(present)
   // CHECK-NEXT: CompoundStmt
-#pragma acc parallel
+#pragma acc parallel default(present)
     {}
   }
   // FIXME: Add a test once we have clauses for this.
@@ -50,12 +52,12 @@ void ...
[truncated]

clang/include/clang/AST/OpenACCClause.h Outdated Show resolved Hide resolved
clang/include/clang/AST/OpenACCClause.h Outdated Show resolved Hide resolved
clang/include/clang/Basic/OpenACCKinds.h Outdated Show resolved Hide resolved
clang/include/clang/Sema/SemaOpenACC.h Outdated Show resolved Hide resolved
clang/include/clang/Sema/SemaOpenACC.h Outdated Show resolved Hide resolved
erichkeane added a commit that referenced this pull request Apr 9, 2024
Brought up in #88135, I inadvertently mis-named these functions, so
correcting them here.
Copy link
Contributor

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

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

Just happened to notice a few typos.

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/lib/Sema/SemaOpenACC.cpp Outdated Show resolved Hide resolved
//===----------------------------------------------------------------------===//
void OpenACCClausePrinter::VisitOpenACCDefaultClause(
const OpenACCDefaultClause &C) {
OS << "default(" << C.getDefaultClauseKind();
Copy link
Member

Choose a reason for hiding this comment

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

Do you output closing ')' in some other place?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yikes, guess not! Good catch.

cast<OpenACCDefaultClause>(OldClause)->getDefaultClauseKind());
break;
default:
assert(false && "Unhandled OpenACC clause in TreeTransform");
Copy link
Member

Choose a reason for hiding this comment

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

llvm_unreachable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to Aaron Ballman, this isn't really what 'unreachable' is for.

There is a pretty extensive discussion somewhere between 'assert(false...) ' and 'llvm_unreachable', as the latter performs optimizations on release builds that can result in an unstable compiler.

In this case, I'm using it somewhat for potential flow-control during development, but once I'm sure I have all the cases covered, this will either go away or switch to an unreachable.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I do remember something related. That's why it is just a question. :)

Copy link
Member

@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

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

LG

cast<OpenACCDefaultClause>(OldClause)->getDefaultClauseKind());
break;
default:
assert(false && "Unhandled OpenACC clause in TreeTransform");
Copy link
Member

Choose a reason for hiding this comment

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

Yes, I do remember something related. That's why it is just a question. :)

@erichkeane erichkeane merged commit 0c7b92a into llvm:main Apr 10, 2024
3 of 4 checks passed
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:modules C++20 modules and Clang Header Modules 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

4 participants