llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Erich Keane (erichkeane) <details> <summary>Changes</summary> This clause just takes an 'int expr', which is not optional. This patch implements the clause on compute constructs. --- Patch is 44.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89151.diff 18 Files Affected: - (modified) clang/include/clang/AST/OpenACCClause.h (+56) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+12) - (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/lib/AST/OpenACCClause.cpp (+26) - (modified) clang/lib/AST/StmtProfile.cpp (+7) - (modified) clang/lib/AST/TextNodeDumper.cpp (+1) - (modified) clang/lib/Parse/ParseOpenACC.cpp (+40-17) - (modified) clang/lib/Sema/SemaOpenACC.cpp (+122) - (modified) clang/lib/Sema/TreeTransform.h (+23) - (modified) clang/lib/Serialization/ASTReader.cpp (+6-1) - (modified) clang/lib/Serialization/ASTWriter.cpp (+6-1) - (modified) clang/test/ParserOpenACC/parse-clauses.c (-2) - (added) clang/test/SemaOpenACC/compute-construct-intexpr-clause-ast.cpp (+255) - (added) clang/test/SemaOpenACC/compute-construct-num_workers-clause.c (+33) - (added) clang/test/SemaOpenACC/compute-construct-num_workers-clause.cpp (+133) - (modified) clang/tools/libclang/CIndex.cpp (+4) ``````````diff diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h index 07587849eb1219..7a60620d5875c5 100644 --- a/clang/include/clang/AST/OpenACCClause.h +++ b/clang/include/clang/AST/OpenACCClause.h @@ -156,6 +156,62 @@ class OpenACCSelfClause : public OpenACCClauseWithCondition { Expr *ConditionExpr, SourceLocation EndLoc); }; +/// Represents one of a handful of classes that have integer expressions. +/// Semantically, many only permit a single expression, with a few that permit +/// up to 3. +class OpenACCClauseWithIntExprs : public OpenACCClauseWithParams { + llvm::SmallVector<Expr *> IntExprs; + + protected: + OpenACCClauseWithIntExprs(OpenACCClauseKind K, SourceLocation BeginLoc, + SourceLocation LParenLoc, + ArrayRef<Expr *> IntExprs, SourceLocation EndLoc) + : OpenACCClauseWithParams(K, BeginLoc, LParenLoc, EndLoc), + IntExprs(IntExprs) {} + + /// Gets the entire list of integer expressions, but leave it to the + /// individual clauses to expose this how they'd like. + llvm::ArrayRef<Expr *> getIntExprs() const { return IntExprs; } + + public: + child_range children() { + return child_range(reinterpret_cast<Stmt **>(IntExprs.begin()), + reinterpret_cast<Stmt **>(IntExprs.end())); + } + + const_child_range children() const { + child_range Children = + const_cast<OpenACCClauseWithIntExprs *>(this)->children(); + return const_child_range(Children.begin(), Children.end()); + } +}; + +/// A more restrictive version of the IntExprs version that exposes a single +/// integer expression. +class OpenACCClauseWithSingleIntExpr : public OpenACCClauseWithIntExprs { + protected: + OpenACCClauseWithSingleIntExpr(OpenACCClauseKind K, SourceLocation BeginLoc, + SourceLocation LParenLoc, Expr *IntExpr, + SourceLocation EndLoc) + : OpenACCClauseWithIntExprs(K, BeginLoc, LParenLoc, IntExpr, EndLoc) {} + + public: + bool hasIntExpr() const { return !getIntExprs().empty(); } + const Expr *getIntExpr() const { + return hasIntExpr() ? getIntExprs()[0] : nullptr; + } + Expr *getIntExpr() { return hasIntExpr() ? getIntExprs()[0] : nullptr; } +}; + +class OpenACCNumWorkersClause : public OpenACCClauseWithSingleIntExpr { + OpenACCNumWorkersClause(SourceLocation BeginLoc, SourceLocation LParenLoc, + Expr *IntExpr, SourceLocation EndLoc); + public: + static OpenACCNumWorkersClause * + Create(const ASTContext &C, SourceLocation BeginLoc, + SourceLocation LParenLoc, Expr *IntExpr, 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 30a8543489f48e..5ac1b3dc6233a3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12268,4 +12268,16 @@ def warn_acc_if_self_conflict : Warning<"OpenACC construct 'self' has no effect when an 'if' clause " "evaluates to true">, InGroup<DiagGroup<"openacc-self-if-potential-conflict">>; +def err_acc_int_expr_requires_integer + : Error<"OpenACC %select{clause|directive}0 '%1' requires expression of " + "integer type (%2 invalid)">; +def err_acc_int_expr_incomplete_class_type + : Error<"OpenACC integer expression has incomplete class type %0">; +def err_acc_int_expr_explicit_conversion + : Error<"OpenACC integer expression type %0 requires explicit conversion " + "to %1">; +def note_acc_int_expr_conversion + : Note<"conversion to %select{integral|enumeration}0 type %1">; +def err_acc_int_expr_multiple_conversions + : Error<"multiple conversions from expression type %0 to an integral type">; } // end of sema component. diff --git a/clang/include/clang/Basic/OpenACCClauses.def b/clang/include/clang/Basic/OpenACCClauses.def index 378495d2c0909a..d1a95cbe613944 100644 --- a/clang/include/clang/Basic/OpenACCClauses.def +++ b/clang/include/clang/Basic/OpenACCClauses.def @@ -18,5 +18,6 @@ VISIT_CLAUSE(Default) VISIT_CLAUSE(If) VISIT_CLAUSE(Self) +VISIT_CLAUSE(NumWorkers) #undef VISIT_CLAUSE diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 23b268126de4e0..72b2f958a5e622 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3640,13 +3640,14 @@ class Parser : public CodeCompletionHandler { /// Parses the clause-list for an OpenACC directive. SmallVector<OpenACCClause *> ParseOpenACCClauseList(OpenACCDirectiveKind DirKind); - bool ParseOpenACCWaitArgument(); + bool ParseOpenACCWaitArgument(SourceLocation Loc, bool IsDirective); /// Parses the clause of the 'bind' argument, which can be a string literal or /// an ID expression. ExprResult ParseOpenACCBindClauseArgument(); /// Parses the clause kind of 'int-expr', which can be any integral /// expression. - ExprResult ParseOpenACCIntExpr(); + ExprResult ParseOpenACCIntExpr(OpenACCDirectiveKind DK, OpenACCClauseKind CK, + SourceLocation Loc); /// Parses the 'device-type-list', which is a list of identifiers. bool ParseOpenACCDeviceTypeList(); /// Parses the 'async-argument', which is an integral value with two @@ -3657,9 +3658,9 @@ class Parser : public CodeCompletionHandler { /// Parses a comma delimited list of 'size-expr's. bool ParseOpenACCSizeExprList(); /// Parses a 'gang-arg-list', used for the 'gang' clause. - bool ParseOpenACCGangArgList(); + bool ParseOpenACCGangArgList(SourceLocation GangLoc); /// Parses a 'gang-arg', used for the 'gang' clause. - bool ParseOpenACCGangArg(); + bool ParseOpenACCGangArg(SourceLocation GangLoc); /// Parses a 'condition' expr, ensuring it results in a ExprResult ParseOpenACCConditionExpr(); diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h index 329dc3945fa2a6..eb461fa7dbd541 100644 --- a/clang/include/clang/Sema/SemaOpenACC.h +++ b/clang/include/clang/Sema/SemaOpenACC.h @@ -44,8 +44,13 @@ class SemaOpenACC : public SemaBase { Expr *ConditionExpr; }; - std::variant<std::monostate, DefaultDetails, ConditionDetails> Details = - std::monostate{}; + struct IntExprDetails { + SmallVector<Expr *> IntExprs; + }; + + std::variant<std::monostate, DefaultDetails, ConditionDetails, + IntExprDetails> + Details = std::monostate{}; public: OpenACCParsedClause(OpenACCDirectiveKind DirKind, @@ -87,6 +92,22 @@ class SemaOpenACC : public SemaBase { return std::get<ConditionDetails>(Details).ConditionExpr; } + unsigned getNumIntExprs() const { + assert(ClauseKind == OpenACCClauseKind::NumWorkers && + "Parsed clause kind does not have a int exprs"); + return std::get<IntExprDetails>(Details).IntExprs.size(); + } + + ArrayRef<Expr *> getIntExprs() { + assert(ClauseKind == OpenACCClauseKind::NumWorkers && + "Parsed clause kind does not have a int exprs"); + return std::get<IntExprDetails>(Details).IntExprs; + } + + ArrayRef<Expr *> getIntExprs() const { + return const_cast<OpenACCParsedClause*>(this)->getIntExprs(); + } + void setLParenLoc(SourceLocation EndLoc) { LParenLoc = EndLoc; } void setEndLoc(SourceLocation EndLoc) { ClauseRange.setEnd(EndLoc); } @@ -109,6 +130,12 @@ class SemaOpenACC : public SemaBase { Details = ConditionDetails{ConditionExpr}; } + + void setIntExprDetails(ArrayRef<Expr *> IntExprs) { + assert(ClauseKind == OpenACCClauseKind::NumWorkers && + "Parsed clause kind does not have a int exprs"); + Details = IntExprDetails{{IntExprs.begin(), IntExprs.end()}}; + } }; SemaOpenACC(Sema &S); @@ -148,6 +175,11 @@ class SemaOpenACC : public SemaBase { /// Called after the directive has been completely parsed, including the /// declaration group or associated statement. DeclGroupRef ActOnEndDeclDirective(); + + /// Called when encountering an 'int-expr' for OpenACC, and manages + /// conversions and diagnostics to 'int'. + ExprResult ActOnIntExpr(OpenACCDirectiveKind DK, OpenACCClauseKind CK, + SourceLocation Loc, Expr *IntExpr); }; } // namespace clang diff --git a/clang/lib/AST/OpenACCClause.cpp b/clang/lib/AST/OpenACCClause.cpp index 9c259c8f9bd0a1..3fff02fa33f28d 100644 --- a/clang/lib/AST/OpenACCClause.cpp +++ b/clang/lib/AST/OpenACCClause.cpp @@ -82,6 +82,27 @@ OpenACCClause::child_range OpenACCClause::children() { return child_range(child_iterator(), child_iterator()); } +OpenACCNumWorkersClause::OpenACCNumWorkersClause(SourceLocation BeginLoc, + SourceLocation LParenLoc, + Expr *IntExpr, + SourceLocation EndLoc) + : OpenACCClauseWithSingleIntExpr(OpenACCClauseKind::NumWorkers, BeginLoc, + LParenLoc, IntExpr, EndLoc) { + assert((!IntExpr || IntExpr->isInstantiationDependent() || + IntExpr->getType()->isIntegerType()) && + "Condition expression type not scalar/dependent"); +} + +OpenACCNumWorkersClause * +OpenACCNumWorkersClause::Create(const ASTContext &C, SourceLocation BeginLoc, + SourceLocation LParenLoc, Expr *IntExpr, + SourceLocation EndLoc) { + void *Mem = C.Allocate(sizeof(OpenACCNumWorkersClause), + alignof(OpenACCNumWorkersClause)); + return new (Mem) + OpenACCNumWorkersClause(BeginLoc, LParenLoc, IntExpr, EndLoc); +} + //===----------------------------------------------------------------------===// // OpenACC clauses printing methods //===----------------------------------------------------------------------===// @@ -98,3 +119,8 @@ void OpenACCClausePrinter::VisitSelfClause(const OpenACCSelfClause &C) { if (const Expr *CondExpr = C.getConditionExpr()) OS << "(" << CondExpr << ")"; } + +void OpenACCClausePrinter::VisitNumWorkersClause( + const OpenACCNumWorkersClause &C) { + OS << "num_workers(" << C.getIntExpr() << ")"; +} diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index b26d804c6f079b..ab7d4c5f930112 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -2496,6 +2496,13 @@ void OpenACCClauseProfiler::VisitSelfClause(const OpenACCSelfClause &Clause) { if (Clause.hasConditionExpr()) Profiler.VisitStmt(Clause.getConditionExpr()); } + +void OpenACCClauseProfiler::VisitNumWorkersClause( + const OpenACCNumWorkersClause &Clause) { + assert(Clause.hasIntExpr() && "if clause requires a valid condition expr"); + Profiler.VisitStmt(Clause.getIntExpr()); +} + } // namespace void StmtProfiler::VisitOpenACCComputeConstruct( diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index ff5b3df2d6dfac..9d1b73cb7a0784 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -399,6 +399,7 @@ void TextNodeDumper::Visit(const OpenACCClause *C) { break; case OpenACCClauseKind::If: case OpenACCClauseKind::Self: + case OpenACCClauseKind::NumWorkers: // 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. OS << " clause"; diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp index 123be476e928ee..096e0863ed47c7 100644 --- a/clang/lib/Parse/ParseOpenACC.cpp +++ b/clang/lib/Parse/ParseOpenACC.cpp @@ -632,10 +632,16 @@ Parser::ParseOpenACCClauseList(OpenACCDirectiveKind DirKind) { return Clauses; } -ExprResult Parser::ParseOpenACCIntExpr() { - // FIXME: this is required to be an integer expression (or dependent), so we - // should ensure that is the case by passing this to SEMA here. - return getActions().CorrectDelayedTyposInExpr(ParseAssignmentExpression()); +ExprResult Parser::ParseOpenACCIntExpr(OpenACCDirectiveKind DK, + OpenACCClauseKind CK, + SourceLocation Loc) { + ExprResult ER = + getActions().CorrectDelayedTyposInExpr(ParseAssignmentExpression()); + + if (!ER.isUsable()) + return ER; + + return getActions().OpenACC().ActOnIntExpr(DK, CK, Loc, ER.get()); } bool Parser::ParseOpenACCClauseVarList(OpenACCClauseKind Kind) { @@ -739,7 +745,7 @@ bool Parser::ParseOpenACCSizeExprList() { /// [num:]int-expr /// dim:int-expr /// static:size-expr -bool Parser::ParseOpenACCGangArg() { +bool Parser::ParseOpenACCGangArg(SourceLocation GangLoc) { if (isOpenACCSpecialToken(OpenACCSpecialTokenKind::Static, getCurToken()) && NextToken().is(tok::colon)) { @@ -753,7 +759,9 @@ bool Parser::ParseOpenACCGangArg() { NextToken().is(tok::colon)) { ConsumeToken(); ConsumeToken(); - return ParseOpenACCIntExpr().isInvalid(); + return ParseOpenACCIntExpr(OpenACCDirectiveKind::Invalid, + OpenACCClauseKind::Gang, GangLoc) + .isInvalid(); } if (isOpenACCSpecialToken(OpenACCSpecialTokenKind::Num, getCurToken()) && @@ -763,11 +771,13 @@ bool Parser::ParseOpenACCGangArg() { // Fallthrough to the 'int-expr' handling for when 'num' is omitted. } // This is just the 'num' case where 'num' is optional. - return ParseOpenACCIntExpr().isInvalid(); + return ParseOpenACCIntExpr(OpenACCDirectiveKind::Invalid, + OpenACCClauseKind::Gang, GangLoc) + .isInvalid(); } -bool Parser::ParseOpenACCGangArgList() { - if (ParseOpenACCGangArg()) { +bool Parser::ParseOpenACCGangArgList(SourceLocation GangLoc) { + if (ParseOpenACCGangArg(GangLoc)) { SkipUntil(tok::r_paren, tok::annot_pragma_openacc_end, Parser::StopBeforeMatch); return false; @@ -776,7 +786,7 @@ bool Parser::ParseOpenACCGangArgList() { while (!getCurToken().isOneOf(tok::r_paren, tok::annot_pragma_openacc_end)) { ExpectAndConsume(tok::comma); - if (ParseOpenACCGangArg()) { + if (ParseOpenACCGangArg(GangLoc)) { SkipUntil(tok::r_paren, tok::annot_pragma_openacc_end, Parser::StopBeforeMatch); return false; @@ -941,11 +951,18 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams( case OpenACCClauseKind::DeviceNum: case OpenACCClauseKind::DefaultAsync: case OpenACCClauseKind::VectorLength: { - ExprResult IntExpr = ParseOpenACCIntExpr(); + ExprResult IntExpr = ParseOpenACCIntExpr(OpenACCDirectiveKind::Invalid, + ClauseKind, ClauseLoc); if (IntExpr.isInvalid()) { Parens.skipToEnd(); return OpenACCCanContinue(); } + + // TODO OpenACC: as we implement the 'rest' of the above, this 'if' should + // be removed leaving just the 'setIntExprDetails'. + if (ClauseKind == OpenACCClauseKind::NumWorkers) + ParsedClause.setIntExprDetails(IntExpr.get()); + break; } case OpenACCClauseKind::DType: @@ -998,7 +1015,8 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams( ? OpenACCSpecialTokenKind::Length : OpenACCSpecialTokenKind::Num, ClauseKind); - ExprResult IntExpr = ParseOpenACCIntExpr(); + ExprResult IntExpr = ParseOpenACCIntExpr(OpenACCDirectiveKind::Invalid, + ClauseKind, ClauseLoc); if (IntExpr.isInvalid()) { Parens.skipToEnd(); return OpenACCCanContinue(); @@ -1014,13 +1032,14 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams( break; } case OpenACCClauseKind::Gang: - if (ParseOpenACCGangArgList()) { + if (ParseOpenACCGangArgList(ClauseLoc)) { Parens.skipToEnd(); return OpenACCCanContinue(); } break; case OpenACCClauseKind::Wait: - if (ParseOpenACCWaitArgument()) { + if (ParseOpenACCWaitArgument(ClauseLoc, + /*IsDirective=*/false)) { Parens.skipToEnd(); return OpenACCCanContinue(); } @@ -1052,7 +1071,7 @@ ExprResult Parser::ParseOpenACCAsyncArgument() { /// In this section and throughout the specification, the term wait-argument /// means: /// [ devnum : int-expr : ] [ queues : ] async-argument-list -bool Parser::ParseOpenACCWaitArgument() { +bool Parser::ParseOpenACCWaitArgument(SourceLocation Loc, bool IsDirective) { // [devnum : int-expr : ] if (isOpenACCSpecialToken(OpenACCSpecialTokenKind::DevNum, Tok) && NextToken().is(tok::colon)) { @@ -1061,7 +1080,11 @@ bool Parser::ParseOpenACCWaitArgument() { // Consume colon. ConsumeToken(); - ExprResult IntExpr = ParseOpenACCIntExpr(); + ExprResult IntExpr = ParseOpenACCIntExpr( + IsDirective ? OpenACCDirectiveKind::Wait + : OpenACCDirectiveKind::Invalid, + IsDirective ? OpenACCClauseKind::Invalid : OpenACCClauseKind::Wait, + Loc); if (IntExpr.isInvalid()) return true; @@ -1245,7 +1268,7 @@ Parser::OpenACCDirectiveParseInfo Parser::ParseOpenACCDirective() { break; case OpenACCDirectiveKind::Wait: // OpenACC has an optional paren-wrapped 'wait-argument'. - if (ParseOpenACCWaitArgument()) + if (ParseOpenACCWaitArgument(StartLoc, /*IsDirective=*/true)) T.skipToEnd(); else T.consumeClose(); diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp index 59f65eaf47a6da..316b0f15b049be 100644 --- a/clang/lib/Sema/SemaOpenACC.cpp +++ b/clang/lib/Sema/SemaOpenACC.cpp @@ -14,6 +14,7 @@ #include "clang/Sema/SemaOpenACC.h" #include "clang/AST/StmtOpenACC.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/OpenACCKinds.h" #include "clang/Sema/Sema.h" #include "llvm/Support/Casting.h" @@ -90,6 +91,17 @@ bool doesClauseApplyToDirective(OpenACCDirectiveKind DirectiveKind, default: return false; } + case OpenACCClauseKind::NumWorkers: + switch (DirectiveKind) { + case OpenACCDirectiveKind::Parallel: + case OpenACCDirectiveKind::Kernels: + case OpenACCDirectiveKind::Update: + case OpenACCDirectiveKind::ParallelLoop: + case OpenACCDirectiveKind::KernelsLoop: + return true; + default: + return false; + } default: // Do nothing so we can go to the 'unimplemented' diagnostic instead. return true; @@ -218,6 +230,25 @@ SemaOpenACC::ActOnClause(ArrayRef<const OpenACCClause *> ExistingClauses, getASTContext(), Clause.getBeginLoc(), Clause.getLParenLoc(), Clause.getConditionExpr(), Clause.getEndLoc()); } + case OpenACCClauseKind::NumWorkers: { + // 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 (!isOpenACCComputeDirectiveKind(Clause.getDirectiveKind())) + break; + + // There is no prose in the standard that says duplicates aren't allowed, + // but this diagnostic is present in other compilers, as well as makes + // sense. + if (checkAlreadyHasClauseOfKind(*this, ExistingClauses, Clause)) + return nullptr; + + assert(Clause.getIntExprs().size() == 1 && + "Invalid number of expressions for NumWorkers"); + return OpenACCNumWorkersClause::Create( + getASTContext(), Clause.getBeginLoc(), Clause.getLParenLoc(), + Clause.getIntExprs()[0], Clause.getEndLoc()); + } default: break; } @@ -248,6 +279,97 @@ void SemaOpenACC::ActOnConstruct(OpenACCDirectiveKind K, } } +ExprResult SemaOpenACC::ActOnIntExpr(OpenACCDirectiveKind DK, + OpenACCClauseKind CK, + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/89151 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits