llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Erich Keane (erichkeane) <details> <summary>Changes</summary> The private clause is the first that takes a 'var-list', thus this has a lot of additional work to enable the var-list type. A 'var' is a traditional variable reference, subscript, member-expression, or array-section, so checking of these is pretty minor. Note: This ran into some issues with array-sections (aka sub-arrays) that will be fixed in a follow-up patch. --- Patch is 73.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90521.diff 21 Files Affected: - (modified) clang/include/clang/AST/OpenACCClause.h (+59-26) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/include/clang/Basic/OpenACCClauses.def (+1) - (modified) clang/include/clang/Parse/Parser.h (+5-4) - (modified) clang/include/clang/Sema/SemaOpenACC.h (+34-2) - (modified) clang/include/clang/Serialization/ASTRecordReader.h (+3) - (modified) clang/include/clang/Serialization/ASTRecordWriter.h (+3) - (modified) clang/lib/AST/OpenACCClause.cpp (+24) - (modified) clang/lib/AST/StmtProfile.cpp (+6) - (modified) clang/lib/AST/TextNodeDumper.cpp (+1) - (modified) clang/lib/Parse/ParseOpenACC.cpp (+56-62) - (modified) clang/lib/Sema/SemaOpenACC.cpp (+73) - (modified) clang/lib/Sema/TreeTransform.h (+30-2) - (modified) clang/lib/Serialization/ASTReader.cpp (+14-1) - (modified) clang/lib/Serialization/ASTWriter.cpp (+12-1) - (modified) clang/test/ParserOpenACC/parse-cache-construct.c (+2) - (modified) clang/test/ParserOpenACC/parse-clauses.c (+40-20) - (added) clang/test/SemaOpenACC/compute-construct-private-clause.c (+138) - (added) clang/test/SemaOpenACC/compute-construct-private-clause.cpp (+161) - (added) clang/test/SemaOpenACC/compute-construct-varlist-ast.cpp (+552) - (modified) clang/tools/libclang/CIndex.cpp (+9) ``````````diff diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h index 277a351c49fcb8..dafcad4179a37e 100644 --- a/clang/include/clang/AST/OpenACCClause.h +++ b/clang/include/clang/AST/OpenACCClause.h @@ -156,51 +156,50 @@ class OpenACCSelfClause : public OpenACCClauseWithCondition { Expr *ConditionExpr, SourceLocation EndLoc); }; -/// Represents a clause that has one or more IntExprs. It does not own the -/// IntExprs, but provides 'children' and other accessors. -class OpenACCClauseWithIntExprs : public OpenACCClauseWithParams { - MutableArrayRef<Expr *> IntExprs; +/// Represents a clause that has one or more expressions associated with it. +class OpenACCClauseWithExprs : public OpenACCClauseWithParams { + MutableArrayRef<Expr *> Exprs; protected: - OpenACCClauseWithIntExprs(OpenACCClauseKind K, SourceLocation BeginLoc, - SourceLocation LParenLoc, SourceLocation EndLoc) + OpenACCClauseWithExprs(OpenACCClauseKind K, SourceLocation BeginLoc, + SourceLocation LParenLoc, SourceLocation EndLoc) : OpenACCClauseWithParams(K, BeginLoc, LParenLoc, EndLoc) {} /// Used only for initialization, the leaf class can initialize this to /// trailing storage. - void setIntExprs(MutableArrayRef<Expr *> NewIntExprs) { - assert(IntExprs.empty() && "Cannot change IntExprs list"); - IntExprs = NewIntExprs; + void setExprs(MutableArrayRef<Expr *> NewExprs) { + assert(Exprs.empty() && "Cannot change Exprs list"); + Exprs = NewExprs; } - /// Gets the entire list of integer expressions, but leave it to the + /// Gets the entire list of expressions, but leave it to the /// individual clauses to expose this how they'd like. - llvm::ArrayRef<Expr *> getIntExprs() const { return IntExprs; } + llvm::ArrayRef<Expr *> getExprs() const { return Exprs; } public: child_range children() { - return child_range(reinterpret_cast<Stmt **>(IntExprs.begin()), - reinterpret_cast<Stmt **>(IntExprs.end())); + return child_range(reinterpret_cast<Stmt **>(Exprs.begin()), + reinterpret_cast<Stmt **>(Exprs.end())); } const_child_range children() const { child_range Children = - const_cast<OpenACCClauseWithIntExprs *>(this)->children(); + const_cast<OpenACCClauseWithExprs *>(this)->children(); return const_child_range(Children.begin(), Children.end()); } }; class OpenACCNumGangsClause final - : public OpenACCClauseWithIntExprs, + : public OpenACCClauseWithExprs, public llvm::TrailingObjects<OpenACCNumGangsClause, Expr *> { OpenACCNumGangsClause(SourceLocation BeginLoc, SourceLocation LParenLoc, ArrayRef<Expr *> IntExprs, SourceLocation EndLoc) - : OpenACCClauseWithIntExprs(OpenACCClauseKind::NumGangs, BeginLoc, - LParenLoc, EndLoc) { + : OpenACCClauseWithExprs(OpenACCClauseKind::NumGangs, BeginLoc, LParenLoc, + EndLoc) { std::uninitialized_copy(IntExprs.begin(), IntExprs.end(), getTrailingObjects<Expr *>()); - setIntExprs(MutableArrayRef(getTrailingObjects<Expr *>(), IntExprs.size())); + setExprs(MutableArrayRef(getTrailingObjects<Expr *>(), IntExprs.size())); } public: @@ -209,35 +208,35 @@ class OpenACCNumGangsClause final ArrayRef<Expr *> IntExprs, SourceLocation EndLoc); llvm::ArrayRef<Expr *> getIntExprs() { - return OpenACCClauseWithIntExprs::getIntExprs(); + return OpenACCClauseWithExprs::getExprs(); } llvm::ArrayRef<Expr *> getIntExprs() const { - return OpenACCClauseWithIntExprs::getIntExprs(); + return OpenACCClauseWithExprs::getExprs(); } }; /// Represents one of a handful of clauses that have a single integer /// expression. -class OpenACCClauseWithSingleIntExpr : public OpenACCClauseWithIntExprs { +class OpenACCClauseWithSingleIntExpr : public OpenACCClauseWithExprs { Expr *IntExpr; protected: OpenACCClauseWithSingleIntExpr(OpenACCClauseKind K, SourceLocation BeginLoc, SourceLocation LParenLoc, Expr *IntExpr, SourceLocation EndLoc) - : OpenACCClauseWithIntExprs(K, BeginLoc, LParenLoc, EndLoc), + : OpenACCClauseWithExprs(K, BeginLoc, LParenLoc, EndLoc), IntExpr(IntExpr) { - setIntExprs(MutableArrayRef<Expr *>{&this->IntExpr, 1}); + setExprs(MutableArrayRef<Expr *>{&this->IntExpr, 1}); } public: - bool hasIntExpr() const { return !getIntExprs().empty(); } + bool hasIntExpr() const { return !getExprs().empty(); } const Expr *getIntExpr() const { - return hasIntExpr() ? getIntExprs()[0] : nullptr; + return hasIntExpr() ? getExprs()[0] : nullptr; } - Expr *getIntExpr() { return hasIntExpr() ? getIntExprs()[0] : nullptr; }; + Expr *getIntExpr() { return hasIntExpr() ? getExprs()[0] : nullptr; }; }; class OpenACCNumWorkersClause : public OpenACCClauseWithSingleIntExpr { @@ -261,6 +260,40 @@ class OpenACCVectorLengthClause : public OpenACCClauseWithSingleIntExpr { Expr *IntExpr, SourceLocation EndLoc); }; +/// Represents a clause with one or more 'var' objects, represented as an expr, +/// as its arguments. Var-list is expected to be stored in trailing storage. +/// For now, we're just storing the original expression in its entirety, unlike +/// OMP which has to do a bunch of work to create a private. +class OpenACCClauseWithVarList : public OpenACCClauseWithExprs { +protected: + OpenACCClauseWithVarList(OpenACCClauseKind K, SourceLocation BeginLoc, + SourceLocation LParenLoc, SourceLocation EndLoc) + : OpenACCClauseWithExprs(K, BeginLoc, LParenLoc, EndLoc) {} + +public: + ArrayRef<Expr *> getVarList() { return getExprs(); } + ArrayRef<Expr *> getVarList() const { return getExprs(); } +}; + +class OpenACCPrivateClause final + : public OpenACCClauseWithVarList, + public llvm::TrailingObjects<OpenACCPrivateClause, Expr *> { + + OpenACCPrivateClause(SourceLocation BeginLoc, SourceLocation LParenLoc, + ArrayRef<Expr *> VarList, SourceLocation EndLoc) + : OpenACCClauseWithVarList(OpenACCClauseKind::Private, BeginLoc, + LParenLoc, EndLoc) { + std::uninitialized_copy(VarList.begin(), VarList.end(), + getTrailingObjects<Expr *>()); + setExprs(MutableArrayRef(getTrailingObjects<Expr *>(), VarList.size())); + } + +public: + static OpenACCPrivateClause * + Create(const ASTContext &C, SourceLocation BeginLoc, SourceLocation LParenLoc, + ArrayRef<Expr *> VarList, SourceLocation EndLoc); +}; + template <class Impl> class OpenACCClauseVisitor { Impl &getDerived() { return static_cast<Impl &>(*this); } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f72d5c252b863e..b58ef1d451a3ae 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12305,4 +12305,7 @@ def err_acc_num_gangs_num_args "OpenACC 'num_gangs' " "%select{|clause: '%1' directive expects maximum of %2, %3 were " "provided}0">; +def err_acc_not_a_var_ref + : Error<"OpenACC variable is not a valid variable name, sub-array, array " + "element, or composite variable member">; } // end of sema component. diff --git a/clang/include/clang/Basic/OpenACCClauses.def b/clang/include/clang/Basic/OpenACCClauses.def index dd5792e7ca8c39..6c3c2db66ef0cf 100644 --- a/clang/include/clang/Basic/OpenACCClauses.def +++ b/clang/include/clang/Basic/OpenACCClauses.def @@ -20,6 +20,7 @@ VISIT_CLAUSE(If) VISIT_CLAUSE(Self) VISIT_CLAUSE(NumGangs) VISIT_CLAUSE(NumWorkers) +VISIT_CLAUSE(Private) VISIT_CLAUSE(VectorLength) #undef VISIT_CLAUSE diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index fb117bf04087ee..ae82ebd0586f72 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3645,11 +3645,12 @@ class Parser : public CodeCompletionHandler { ExprResult ParseOpenACCIDExpression(); /// Parses the variable list for the `cache` construct. void ParseOpenACCCacheVarList(); + + using OpenACCVarParseResult = std::pair<ExprResult, OpenACCParseCanContinue>; /// Parses a single variable in a variable list for OpenACC. - bool ParseOpenACCVar(); - /// Parses the variable list for the variety of clauses that take a var-list, - /// including the optional Special Token listed for some,based on clause type. - bool ParseOpenACCClauseVarList(OpenACCClauseKind Kind); + OpenACCVarParseResult ParseOpenACCVar(); + /// Parses the variable list for the variety of places that take a var-list. + llvm::SmallVector<Expr *> ParseOpenACCVarList(); /// Parses any parameters for an OpenACC Clause, including required/optional /// parens. OpenACCClauseParseResult diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h index da19503c2902fd..9c4ca40e34c5c4 100644 --- a/clang/include/clang/Sema/SemaOpenACC.h +++ b/clang/include/clang/Sema/SemaOpenACC.h @@ -48,8 +48,12 @@ class SemaOpenACC : public SemaBase { SmallVector<Expr *> IntExprs; }; + struct VarListDetails { + SmallVector<Expr *> VarList; + }; + std::variant<std::monostate, DefaultDetails, ConditionDetails, - IntExprDetails> + IntExprDetails, VarListDetails> Details = std::monostate{}; public: @@ -112,6 +116,18 @@ class SemaOpenACC : public SemaBase { return const_cast<OpenACCParsedClause *>(this)->getIntExprs(); } + // Non-const version that permits modifying of the VarList for the purposes + // of Sema enforcement. + SmallVector<Expr *> &getVarList() { + assert(ClauseKind == OpenACCClauseKind::Private && + "Parsed clause kind does not have a var-list"); + return std::get<VarListDetails>(Details).VarList; + } + + ArrayRef<Expr *> getVarList() const { + return const_cast<OpenACCParsedClause *>(this)->getVarList(); + } + void setLParenLoc(SourceLocation EndLoc) { LParenLoc = EndLoc; } void setEndLoc(SourceLocation EndLoc) { ClauseRange.setEnd(EndLoc); } @@ -147,7 +163,19 @@ class SemaOpenACC : public SemaBase { ClauseKind == OpenACCClauseKind::NumWorkers || ClauseKind == OpenACCClauseKind::VectorLength) && "Parsed clause kind does not have a int exprs"); - Details = IntExprDetails{IntExprs}; + Details = IntExprDetails{std::move(IntExprs)}; + } + + void setVarListDetails(ArrayRef<Expr *> VarList) { + assert(ClauseKind == OpenACCClauseKind::Private && + "Parsed clause kind does not have a var-list"); + Details = VarListDetails{{VarList.begin(), VarList.end()}}; + } + + void setVarListDetails(llvm::SmallVector<Expr *> &&VarList) { + assert(ClauseKind == OpenACCClauseKind::Private && + "Parsed clause kind does not have a var-list"); + Details = VarListDetails{std::move(VarList)}; } }; @@ -194,6 +222,10 @@ class SemaOpenACC : public SemaBase { ExprResult ActOnIntExpr(OpenACCDirectiveKind DK, OpenACCClauseKind CK, SourceLocation Loc, Expr *IntExpr); + /// Called when encountering a 'var' for OpenACC, ensures it is actually a + /// declaration reference to a variable of the correct type. + ExprResult ActOnVar(Expr *VarExpr); + /// Checks and creates an Array Section used in an OpenACC construct/clause. ExprResult ActOnArraySectionExpr(Expr *Base, SourceLocation LBLoc, Expr *LowerBound, diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h index 06b80f266a9441..1e11d2d5e42f95 100644 --- a/clang/include/clang/Serialization/ASTRecordReader.h +++ b/clang/include/clang/Serialization/ASTRecordReader.h @@ -269,6 +269,9 @@ class ASTRecordReader /// Read an OpenMP children, advancing Idx. void readOMPChildren(OMPChildren *Data); + /// Read a list of Exprs used for a var-list. + llvm::SmallVector<Expr *> readOpenACCVarList(); + /// Read an OpenACC clause, advancing Idx. OpenACCClause *readOpenACCClause(); diff --git a/clang/include/clang/Serialization/ASTRecordWriter.h b/clang/include/clang/Serialization/ASTRecordWriter.h index 1feb8fcbacf772..8b1da49bd4c576 100644 --- a/clang/include/clang/Serialization/ASTRecordWriter.h +++ b/clang/include/clang/Serialization/ASTRecordWriter.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_SERIALIZATION_ASTRECORDWRITER_H #include "clang/AST/AbstractBasicWriter.h" +#include "clang/AST/OpenACCClause.h" #include "clang/AST/OpenMPClause.h" #include "clang/Serialization/ASTWriter.h" #include "clang/Serialization/SourceLocationEncoding.h" @@ -293,6 +294,8 @@ class ASTRecordWriter /// Writes data related to the OpenMP directives. void writeOMPChildren(OMPChildren *Data); + void writeOpenACCVarList(const OpenACCClauseWithVarList *C); + /// Writes out a single OpenACC Clause. void writeOpenACCClause(const OpenACCClause *C); diff --git a/clang/lib/AST/OpenACCClause.cpp b/clang/lib/AST/OpenACCClause.cpp index 6cd5b28802187d..208a51c82399cc 100644 --- a/clang/lib/AST/OpenACCClause.cpp +++ b/clang/lib/AST/OpenACCClause.cpp @@ -134,6 +134,24 @@ OpenACCNumGangsClause *OpenACCNumGangsClause::Create(const ASTContext &C, return new (Mem) OpenACCNumGangsClause(BeginLoc, LParenLoc, IntExprs, EndLoc); } +OpenACCPrivateClause *OpenACCPrivateClause::Create(const ASTContext &C, + SourceLocation BeginLoc, + SourceLocation LParenLoc, + ArrayRef<Expr *> VarList, + SourceLocation EndLoc) { + void *Mem = C.Allocate( + OpenACCPrivateClause::totalSizeToAlloc<Expr *>(VarList.size())); + return new (Mem) OpenACCPrivateClause(BeginLoc, LParenLoc, VarList, EndLoc); +} + +// ValueDecl *getDeclFromExpr(Expr *RefExpr) { +// //RefExpr = RefExpr->IgnoreParenImpCasts(); +// +// ////while (isa<ArraySubscriptExpr, ArraySectionExpr>(RefExpr)) { +// ////} +// // TODO: +// } + //===----------------------------------------------------------------------===// // OpenACC clauses printing methods //===----------------------------------------------------------------------===// @@ -166,3 +184,9 @@ void OpenACCClausePrinter::VisitVectorLengthClause( const OpenACCVectorLengthClause &C) { OS << "vector_length(" << C.getIntExpr() << ")"; } + +void OpenACCClausePrinter::VisitPrivateClause(const OpenACCPrivateClause &C) { + OS << "private("; + llvm::interleaveComma(C.getVarList(), OS); + OS << ")"; +} diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index a95f5c6103e24d..973f6f97bae0bf 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -2509,6 +2509,12 @@ void OpenACCClauseProfiler::VisitNumWorkersClause( Profiler.VisitStmt(Clause.getIntExpr()); } +void OpenACCClauseProfiler::VisitPrivateClause( + const OpenACCPrivateClause &Clause) { + for (auto *E : Clause.getVarList()) + Profiler.VisitStmt(E); +} + void OpenACCClauseProfiler::VisitVectorLengthClause( const OpenACCVectorLengthClause &Clause) { assert(Clause.hasIntExpr() && diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index 8f0a9a9b0ed0bc..89f50d6dacfd23 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -401,6 +401,7 @@ void TextNodeDumper::Visit(const OpenACCClause *C) { case OpenACCClauseKind::Self: case OpenACCClauseKind::NumGangs: case OpenACCClauseKind::NumWorkers: + case OpenACCClauseKind::Private: case OpenACCClauseKind::VectorLength: // The condition expression will be printed as a part of the 'children', // but print 'clause' here so it is clear what is happening from the dump. diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp index 29326f5d993a9d..a12ffad699755f 100644 --- a/clang/lib/Parse/ParseOpenACC.cpp +++ b/clang/lib/Parse/ParseOpenACC.cpp @@ -86,6 +86,10 @@ OpenACCClauseKind getOpenACCClauseKind(Token Tok) { if (Tok.is(tok::kw_if)) return OpenACCClauseKind::If; + // 'private' is also a keyword, make sure we pare it correctly. + if (Tok.is(tok::kw_private)) + return OpenACCClauseKind::Private; + if (!Tok.is(tok::identifier)) return OpenACCClauseKind::Invalid; @@ -682,28 +686,6 @@ bool Parser::ParseOpenACCIntExprList(OpenACCDirectiveKind DK, return false; } -bool Parser::ParseOpenACCClauseVarList(OpenACCClauseKind Kind) { - // FIXME: Future clauses will require 'special word' parsing, check for one, - // then parse it based on whether it is a clause that requires a 'special - // word'. - (void)Kind; - - // If the var parsing fails, skip until the end of the directive as this is - // an expression and gets messy if we try to continue otherwise. - if (ParseOpenACCVar()) - return true; - - while (!getCurToken().isOneOf(tok::r_paren, tok::annot_pragma_openacc_end)) { - ExpectAndConsume(tok::comma); - - // If the var parsing fails, skip until the end of the directive as this is - // an expression and gets messy if we try to continue otherwise. - if (ParseOpenACCVar()) - return true; - } - return false; -} - /// OpenACC 3.3 Section 2.4: /// The argument to the device_type clause is a comma-separated list of one or /// more device architecture name identifiers, or an asterisk. @@ -917,28 +899,19 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams( case OpenACCClauseKind::CopyIn: tryParseAndConsumeSpecialTokenKind( *this, OpenACCSpecialTokenKind::ReadOnly, ClauseKind); - if (ParseOpenACCClauseVarList(ClauseKind)) { - Parens.skipToEnd(); - return OpenACCCanContinue(); - } + ParseOpenACCVarList(); break; case OpenACCClauseKind::Create: case OpenACCClauseKind::CopyOut: tryParseAndConsumeSpecialTokenKind(*this, OpenACCSpecialTokenKind::Zero, ClauseKind); - if (ParseOpenACCClauseVarList(ClauseKind)) { - Parens.skipToEnd(); - return OpenACCCanContinue(); - } + ParseOpenACCVarList(); break; case OpenACCClauseKind::Reduction: // If we're missing a clause-kind (or it is invalid), see if we can parse // the var-list anyway. ParseReductionOperator(*this); - if (ParseOpenACCClauseVarList(ClauseKind)) { - Parens.skipToEnd(); - return OpenACCCanContinue(); - } + ParseOpenACCVarList(); break; case OpenACCClauseKind::Self: // The 'self' clause is a var-list instead of a 'condition' in the case of @@ -958,13 +931,14 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams( case OpenACCClauseKind::Link: case OpenACCClauseKind::NoCreate: case OpenACCClauseKind::Present: - case OpenACCClauseKind::Private: case OpenACCClauseKind::UseDevice: - if (ParseOpenACCClauseVarList(ClauseKind)) { - Parens.skipToEnd(); - return OpenACCCanContinue(); - } + ParseOpenACCVarList(); + break; + case OpenACCClauseKind::Private: { + llvm::SmallVector<Expr *> Vars = ParseOpenACCVarList(); + ParsedClause.setVarListDetails(std::move(Vars)); break; + } case OpenACCClauseKind::Collapse: { tryParseAndConsumeSpecialTokenKind(*this, OpenACCSpecialTokenKind::Force, ClauseKind); @@ -1227,16 +1201,51 @@ ExprResult Parser::ParseOpenACCBindClauseArgument() { /// OpenACC 3.3, section 1.6: /// In this spec, a 'var' (in italics) is one of the following: -/// - a variable name (a scalar, array, or compisite variable name) +/// - a variable name (a scalar, array, or composite variable name) /// - a subarray specification with subscript ranges /// - an array element /// - a member of a composite variable /// - a ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/90521 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
