This revision was automatically updated to reflect the committed changes. Closed by commit rG89167e3c5b00: [OpenMP][NFC] Refactor code for interop parts of 'init' and 'append_args'… (authored by mikerice). Herald added a project: clang. Herald added a subscriber: cfe-commits.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132171/new/ https://reviews.llvm.org/D132171 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/Parse/Parser.h clang/include/clang/Sema/Sema.h clang/lib/AST/OpenMPClause.cpp clang/lib/Parse/ParseOpenMP.cpp clang/lib/Sema/SemaOpenMP.cpp clang/lib/Sema/TreeTransform.h
Index: clang/lib/Sema/TreeTransform.h =================================================================== --- clang/lib/Sema/TreeTransform.h +++ clang/lib/Sema/TreeTransform.h @@ -2216,15 +2216,13 @@ /// /// By default, performs semantic analysis to build the new OpenMP clause. /// Subclasses may override this routine to provide different behavior. - OMPClause *RebuildOMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs, - bool IsTarget, bool IsTargetSync, + OMPClause *RebuildOMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo, SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation VarLoc, SourceLocation EndLoc) { - return getSema().ActOnOpenMPInitClause(InteropVar, PrefExprs, IsTarget, - IsTargetSync, StartLoc, LParenLoc, - VarLoc, EndLoc); + return getSema().ActOnOpenMPInitClause(InteropVar, InteropInfo, StartLoc, + LParenLoc, VarLoc, EndLoc); } /// Build a new OpenMP 'use' clause. @@ -9673,17 +9671,19 @@ if (IVR.isInvalid()) return nullptr; - llvm::SmallVector<Expr *, 8> PrefExprs; - PrefExprs.reserve(C->varlist_size() - 1); + OMPInteropInfo InteropInfo; + InteropInfo.IsTarget = C->getIsTarget(); + InteropInfo.IsTargetSync = C->getIsTargetSync(); + InteropInfo.PreferTypes.reserve(C->varlist_size() - 1); for (Expr *E : llvm::drop_begin(C->varlists())) { ExprResult ER = getDerived().TransformExpr(cast<Expr>(E)); if (ER.isInvalid()) return nullptr; - PrefExprs.push_back(ER.get()); + InteropInfo.PreferTypes.push_back(ER.get()); } - return getDerived().RebuildOMPInitClause( - IVR.get(), PrefExprs, C->getIsTarget(), C->getIsTargetSync(), - C->getBeginLoc(), C->getLParenLoc(), C->getVarLoc(), C->getEndLoc()); + return getDerived().RebuildOMPInitClause(IVR.get(), InteropInfo, + C->getBeginLoc(), C->getLParenLoc(), + C->getVarLoc(), C->getEndLoc()); } template <typename Derived> Index: clang/lib/Sema/SemaOpenMP.cpp =================================================================== --- clang/lib/Sema/SemaOpenMP.cpp +++ clang/lib/Sema/SemaOpenMP.cpp @@ -17344,8 +17344,7 @@ } OMPClause * -Sema::ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs, - bool IsTarget, bool IsTargetSync, +Sema::ActOnOpenMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo, SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation VarLoc, SourceLocation EndLoc) { @@ -17354,7 +17353,7 @@ // Check prefer_type values. These foreign-runtime-id values are either // string literals or constant integral expressions. - for (const Expr *E : PrefExprs) { + for (const Expr *E : InteropInfo.PreferTypes) { if (E->isValueDependent() || E->isTypeDependent() || E->isInstantiationDependent() || E->containsUnexpandedParameterPack()) continue; @@ -17366,9 +17365,8 @@ return nullptr; } - return OMPInitClause::Create(Context, InteropVar, PrefExprs, IsTarget, - IsTargetSync, StartLoc, LParenLoc, VarLoc, - EndLoc); + return OMPInitClause::Create(Context, InteropVar, InteropInfo, StartLoc, + LParenLoc, VarLoc, EndLoc); } OMPClause *Sema::ActOnOpenMPUseClause(Expr *InteropVar, SourceLocation StartLoc, Index: clang/lib/Parse/ParseOpenMP.cpp =================================================================== --- clang/lib/Parse/ParseOpenMP.cpp +++ clang/lib/Parse/ParseOpenMP.cpp @@ -1502,54 +1502,6 @@ (void)ConsumeAnnotationToken(); } -/// Parse a list of interop-types. These are 'target' and 'targetsync'. Both -/// are allowed but duplication of either is not meaningful. -static Optional<OMPDeclareVariantAttr::InteropType> -parseInteropTypeList(Parser &P) { - const Token &Tok = P.getCurToken(); - bool HasError = false; - bool IsTarget = false; - bool IsTargetSync = false; - - while (Tok.is(tok::identifier)) { - if (Tok.getIdentifierInfo()->isStr("target")) { - // OpenMP 5.1 [2.15.1, interop Construct, Restrictions] - // Each interop-type may be specified on an action-clause at most - // once. - if (IsTarget) - P.Diag(Tok, diag::warn_omp_more_one_interop_type) << "target"; - IsTarget = true; - } else if (Tok.getIdentifierInfo()->isStr("targetsync")) { - if (IsTargetSync) - P.Diag(Tok, diag::warn_omp_more_one_interop_type) << "targetsync"; - IsTargetSync = true; - } else { - HasError = true; - P.Diag(Tok, diag::err_omp_expected_interop_type); - } - P.ConsumeToken(); - - if (!Tok.is(tok::comma)) - break; - P.ConsumeToken(); - } - if (HasError) - return None; - - if (!IsTarget && !IsTargetSync) { - P.Diag(Tok, diag::err_omp_expected_interop_type); - return None; - } - - // As of OpenMP 5.1,there are two interop-types, "target" and - // "targetsync". Either or both are allowed for a single interop. - if (IsTarget && IsTargetSync) - return OMPDeclareVariantAttr::Target_TargetSync; - if (IsTarget) - return OMPDeclareVariantAttr::Target; - return OMPDeclareVariantAttr::TargetSync; -} - bool Parser::parseOpenMPAppendArgs( SmallVectorImpl<OMPDeclareVariantAttr::InteropType> &InterOpTypes) { bool HasError = false; @@ -1568,12 +1520,21 @@ if (IT.expectAndConsume(diag::err_expected_lparen_after, "interop")) return true; - // Parse the interop-types. - if (Optional<OMPDeclareVariantAttr::InteropType> IType = - parseInteropTypeList(*this)) - InterOpTypes.push_back(*IType); - else + OMPInteropInfo InteropInfo; + if (ParseOMPInteropInfo(InteropInfo, OMPC_append_args)) { HasError = true; + } else { + OMPDeclareVariantAttr::InteropType IT; + // As of OpenMP 5.1, there are two interop-types, "target" and + // "targetsync". Either or both are allowed for a single interop. + if (InteropInfo.IsTarget && InteropInfo.IsTargetSync) + IT = OMPDeclareVariantAttr::Target_TargetSync; + else if (InteropInfo.IsTarget) + IT = OMPDeclareVariantAttr::Target; + else + IT = OMPDeclareVariantAttr::TargetSync; + InterOpTypes.push_back(IT); + } IT.consumeClose(); if (Tok.is(tok::comma)) @@ -3496,6 +3457,90 @@ return false; } +/// Parses a comma-separated list of interop-types and a prefer_type list. +/// +bool Parser::ParseOMPInteropInfo(OMPInteropInfo &InteropInfo, + OpenMPClauseKind Kind) { + const Token &Tok = getCurToken(); + bool HasError = false; + bool IsTarget = false; + bool IsTargetSync = false; + + while (Tok.is(tok::identifier)) { + // Currently prefer_type is only allowed with 'init' and it must be first. + bool PreferTypeAllowed = Kind == OMPC_init && + InteropInfo.PreferTypes.empty() && !IsTarget && + !IsTargetSync; + if (Tok.getIdentifierInfo()->isStr("target")) { + // OpenMP 5.1 [2.15.1, interop Construct, Restrictions] + // Each interop-type may be specified on an action-clause at most + // once. + if (IsTarget) + Diag(Tok, diag::warn_omp_more_one_interop_type) << "target"; + IsTarget = true; + ConsumeToken(); + } else if (Tok.getIdentifierInfo()->isStr("targetsync")) { + if (IsTargetSync) + Diag(Tok, diag::warn_omp_more_one_interop_type) << "targetsync"; + IsTargetSync = true; + ConsumeToken(); + } else if (Tok.getIdentifierInfo()->isStr("prefer_type") && + PreferTypeAllowed) { + ConsumeToken(); + BalancedDelimiterTracker PT(*this, tok::l_paren, + tok::annot_pragma_openmp_end); + if (PT.expectAndConsume(diag::err_expected_lparen_after, "prefer_type")) + HasError = true; + + while (Tok.isNot(tok::r_paren)) { + SourceLocation Loc = Tok.getLocation(); + ExprResult LHS = ParseCastExpression(AnyCastExpr); + ExprResult PTExpr = Actions.CorrectDelayedTyposInExpr( + ParseRHSOfBinaryExpression(LHS, prec::Conditional)); + PTExpr = Actions.ActOnFinishFullExpr(PTExpr.get(), Loc, + /*DiscardedValue=*/false); + if (PTExpr.isUsable()) { + InteropInfo.PreferTypes.push_back(PTExpr.get()); + } else { + HasError = true; + SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end, + StopBeforeMatch); + } + + if (Tok.is(tok::comma)) + ConsumeToken(); + } + PT.consumeClose(); + } else { + HasError = true; + Diag(Tok, diag::err_omp_expected_interop_type); + ConsumeToken(); + } + if (!Tok.is(tok::comma)) + break; + ConsumeToken(); + } + + if (!HasError && !IsTarget && !IsTargetSync) { + Diag(Tok, diag::err_omp_expected_interop_type); + HasError = true; + } + + if (Kind == OMPC_init) { + if (Tok.isNot(tok::colon) && (IsTarget || IsTargetSync)) + Diag(Tok, diag::warn_pragma_expected_colon) << "interop types"; + if (Tok.is(tok::colon)) + ConsumeToken(); + } + + // As of OpenMP 5.1,there are two interop-types, "target" and + // "targetsync". Either or both are allowed for a single interop. + InteropInfo.IsTarget = IsTarget; + InteropInfo.IsTargetSync = IsTargetSync; + + return HasError; +} + /// Parsing of OpenMP clauses that use an interop-var. /// /// init-clause: @@ -3528,57 +3573,10 @@ getOpenMPClauseName(Kind).data())) return nullptr; - bool IsTarget = false; - bool IsTargetSync = false; - SmallVector<Expr *, 4> Prefs; - - if (Kind == OMPC_init) { - - // Parse optional interop-modifier. - if (Tok.is(tok::identifier) && PP.getSpelling(Tok) == "prefer_type") { - ConsumeToken(); - BalancedDelimiterTracker PT(*this, tok::l_paren, - tok::annot_pragma_openmp_end); - if (PT.expectAndConsume(diag::err_expected_lparen_after, "prefer_type")) - return nullptr; - - while (Tok.isNot(tok::r_paren)) { - SourceLocation Loc = Tok.getLocation(); - ExprResult LHS = ParseCastExpression(AnyCastExpr); - ExprResult PTExpr = Actions.CorrectDelayedTyposInExpr( - ParseRHSOfBinaryExpression(LHS, prec::Conditional)); - PTExpr = Actions.ActOnFinishFullExpr(PTExpr.get(), Loc, - /*DiscardedValue=*/false); - if (PTExpr.isUsable()) - Prefs.push_back(PTExpr.get()); - else - SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end, - StopBeforeMatch); - - if (Tok.is(tok::comma)) - ConsumeToken(); - } - PT.consumeClose(); - } - - if (!Prefs.empty()) { - if (Tok.is(tok::comma)) - ConsumeToken(); - else - Diag(Tok, diag::err_omp_expected_punc_after_interop_mod); - } - - // Parse the interop-types. - if (Optional<OMPDeclareVariantAttr::InteropType> IType = - parseInteropTypeList(*this)) { - IsTarget = IType != OMPDeclareVariantAttr::TargetSync; - IsTargetSync = IType != OMPDeclareVariantAttr::Target; - if (Tok.isNot(tok::colon)) - Diag(Tok, diag::warn_pragma_expected_colon) << "interop types"; - } - if (Tok.is(tok::colon)) - ConsumeToken(); - } + bool InteropError = false; + OMPInteropInfo InteropInfo; + if (Kind == OMPC_init) + InteropError = ParseOMPInteropInfo(InteropInfo, OMPC_init); // Parse the variable. SourceLocation VarLoc = Tok.getLocation(); @@ -3594,14 +3592,12 @@ if (!T.consumeClose()) RLoc = T.getCloseLocation(); - if (ParseOnly || !InteropVarExpr.isUsable() || - (Kind == OMPC_init && !IsTarget && !IsTargetSync)) + if (ParseOnly || !InteropVarExpr.isUsable() || InteropError) return nullptr; if (Kind == OMPC_init) - return Actions.ActOnOpenMPInitClause(InteropVarExpr.get(), Prefs, IsTarget, - IsTargetSync, Loc, T.getOpenLocation(), - VarLoc, RLoc); + return Actions.ActOnOpenMPInitClause(InteropVarExpr.get(), InteropInfo, Loc, + T.getOpenLocation(), VarLoc, RLoc); if (Kind == OMPC_use) return Actions.ActOnOpenMPUseClause(InteropVarExpr.get(), Loc, T.getOpenLocation(), VarLoc, RLoc); Index: clang/lib/AST/OpenMPClause.cpp =================================================================== --- clang/lib/AST/OpenMPClause.cpp +++ clang/lib/AST/OpenMPClause.cpp @@ -1626,18 +1626,19 @@ } OMPInitClause *OMPInitClause::Create(const ASTContext &C, Expr *InteropVar, - ArrayRef<Expr *> PrefExprs, bool IsTarget, - bool IsTargetSync, SourceLocation StartLoc, + OMPInteropInfo &InteropInfo, + SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation VarLoc, SourceLocation EndLoc) { - void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(PrefExprs.size() + 1)); - auto *Clause = - new (Mem) OMPInitClause(IsTarget, IsTargetSync, StartLoc, LParenLoc, - VarLoc, EndLoc, PrefExprs.size() + 1); + void *Mem = + C.Allocate(totalSizeToAlloc<Expr *>(InteropInfo.PreferTypes.size() + 1)); + auto *Clause = new (Mem) OMPInitClause( + InteropInfo.IsTarget, InteropInfo.IsTargetSync, StartLoc, LParenLoc, + VarLoc, EndLoc, InteropInfo.PreferTypes.size() + 1); Clause->setInteropVar(InteropVar); - llvm::copy(PrefExprs, Clause->getTrailingObjects<Expr *>() + 1); + llvm::copy(InteropInfo.PreferTypes, Clause->getTrailingObjects<Expr *>() + 1); return Clause; } Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -11621,12 +11621,10 @@ SourceLocation EndLoc); /// Called on well-formed 'init' clause. - OMPClause *ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs, - bool IsTarget, bool IsTargetSync, - SourceLocation StartLoc, - SourceLocation LParenLoc, - SourceLocation VarLoc, - SourceLocation EndLoc); + OMPClause * + ActOnOpenMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo, + SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation VarLoc, SourceLocation EndLoc); /// Called on well-formed 'use' clause. OMPClause *ActOnOpenMPUseClause(Expr *InteropVar, SourceLocation StartLoc, Index: clang/include/clang/Parse/Parser.h =================================================================== --- clang/include/clang/Parse/Parser.h +++ clang/include/clang/Parse/Parser.h @@ -3332,6 +3332,9 @@ /// '(' { <allocator> [ '(' <allocator_traits> ')' ] }+ ')' OMPClause *ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind); + /// Parses the 'interop' parts of the 'append_args' and 'init' clauses. + bool ParseOMPInteropInfo(OMPInteropInfo &InteropInfo, OpenMPClauseKind Kind); + /// Parses clause with an interop variable of kind \a Kind. /// /// \param Kind Kind of current clause. Index: clang/include/clang/AST/OpenMPClause.h =================================================================== --- clang/include/clang/AST/OpenMPClause.h +++ clang/include/clang/AST/OpenMPClause.h @@ -7709,6 +7709,13 @@ } }; +/// Contains 'interop' data for 'append_args' and 'init' clauses. +struct OMPInteropInfo final { + bool IsTarget = false; + bool IsTargetSync = false; + llvm::SmallVector<Expr *, 4> PreferTypes; +}; + /// This represents the 'init' clause in '#pragma omp ...' directives. /// /// \code @@ -7763,16 +7770,14 @@ /// /// \param C AST context. /// \param InteropVar The interop variable. - /// \param PrefExprs The list of preference expressions. - /// \param IsTarget Uses the 'target' interop-type. - /// \param IsTargetSync Uses the 'targetsync' interop-type. + /// \param InteropInfo The interop-type and prefer_type list. /// \param StartLoc Starting location of the clause. /// \param LParenLoc Location of '('. /// \param VarLoc Location of the interop variable. /// \param EndLoc Ending location of the clause. static OMPInitClause *Create(const ASTContext &C, Expr *InteropVar, - ArrayRef<Expr *> PrefExprs, bool IsTarget, - bool IsTargetSync, SourceLocation StartLoc, + OMPInteropInfo &InteropInfo, + SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation VarLoc, SourceLocation EndLoc);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits