hfinkel added a comment. In http://reviews.llvm.org/D10599#201312, @aaron.ballman wrote:
> LGTM with one question below. I would wait for review from Richard or > Hal before committing. I'm not really the right person to okay this patch. Richard, can you please look at this? > > > > Index: lib/Parse/ParseOpenMP.cpp > > > =================================================================== > > > > > > - lib/Parse/ParseOpenMP.cpp +++ lib/Parse/ParseOpenMP.cpp @@ -30,6 +30,7 > > @@ // E.g.: OMPD_for OMPD_simd ===> OMPD_for_simd // TODO: add other > > combined directives in topological order. const OpenMPDirectiveKind F[][3] > > = { + {OMPD_unknown /*declare*/, OMPD_simd, OMPD_declare_simd}, > > {OMPD_unknown /*cancellation*/, OMPD_unknown /*point*/, > > OMPD_cancellation_point}, {OMPD_for, OMPD_simd, OMPD_for_simd}, @@ -43,25 > > +44,25 @@ : getOpenMPDirectiveKind(P.getPreprocessor().getSpelling(Tok)); > > bool TokenMatched = false; for (unsigned i = 0; i < > > llvm::array_lengthof(F); ++i) { > > > - if (!Tok.isAnnotation() && DKind == OMPD_unknown) { + if > > (!Tok.isAnnotation() && DKind == OMPD_unknown) TokenMatched = > > > - (i == 0) && > > > - !P.getPreprocessor().getSpelling(Tok).compare("cancellation"); > > > - } else { + ((i == 0) && + > > !P.getPreprocessor().getSpelling(Tok).compare("declare")) || + ((i > > == 1) && + > > !P.getPreprocessor().getSpelling(Tok).compare("cancellation")); + else > > TokenMatched = DKind == F[i][0] && DKind != OMPD_unknown; > > > - } if (TokenMatched) { Tok = P.getPreprocessor().LookAhead(0); auto SDKind > > = Tok.isAnnotation() ? OMPD_unknown : > > getOpenMPDirectiveKind(P.getPreprocessor().getSpelling(Tok)); > > > - if (!Tok.isAnnotation() && DKind == OMPD_unknown) { + if > > (!Tok.isAnnotation() && SDKind == OMPD_unknown) TokenMatched = > > > - (i == 0) && !P.getPreprocessor().getSpelling(Tok).compare("point"); > > > - } else { + (i == 1) && > > !P.getPreprocessor().getSpelling(Tok).compare("point"); + else > > TokenMatched = SDKind == F[i][1] && SDKind != OMPD_unknown; > > > - } if (TokenMatched) { P.ConsumeToken(); DKind = F[i][2]; @@ -75,14 +76,25 > > @@ /// /// threadprivate-directive: /// annot_pragma_openmp > > 'threadprivate' simple-variable-list +/// annot_pragma_omp_end /// > > -Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirective() { +/// > > declare-simd-directive: +/// annot_pragma_openmp 'declare simd' > > {<clause> [,]} +/// annot_pragma_omp_end +/// <function > > declaration/definition> +/// +Parser::DeclGroupPtrTy > > +Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(bool IsInTagDecl, + > > unsigned Level) { > > assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!"); > > ParenBraceBracketBalancer BalancerRAIIObj(*this); > > > > > > + auto AnnotationVal = > > reinterpret_cast<uintptr_t>(Tok.getAnnotationValue()); SourceLocation Loc = > > ConsumeToken(); SmallVector<Expr *, 5> Identifiers; > > > - auto DKind = ParseOpenMPDirectiveKind(*this); + OpenMPDirectiveKind > > DKind = + (AnnotationVal == 0) ? ParseOpenMPDirectiveKind(*this) + > > : static_cast<OpenMPDirectiveKind>(AnnotationVal); > > > > > > switch (DKind) { case OMPD_threadprivate: @@ -100,6 +112,86 @@ return > > Actions.ActOnOpenMPThreadprivateDirective(Loc, Identifiers); } break; + > > case OMPD_declare_simd: { + // The syntax is: + // { #pragma omp > > declare simd } + // <function-declaration-or-definition> + // + if > > (AnnotationVal == 0) + // Skip 'simd' if it was restored from cached > > tokens. + ConsumeToken(); + if (IsInTagDecl) { + > > LateParseOpenMPDeclarativeDirectiveWithTemplateFunction( + > > /*DKind=*/OMPD_declare_simd, Loc); + return DeclGroupPtrTy(); + } + > > + SmallVector<llvm::PointerIntPair<OMPClause *, 1, bool>, OMPC_unknown + > > 1> + FirstClauses(OMPC_unknown + 1); + SmallVector<OMPClause *, > > 4> Clauses; + SmallVector<Token, 8> CachedPragmas; + + while > > (Tok.isNot(tok::annot_pragma_openmp_end) && Tok.isNot(tok::eof)) { + > > CachedPragmas.push_back(Tok); + ConsumeAnyToken(); + } + > > CachedPragmas.push_back(Tok); + if (Tok.isNot(tok::! eof)) + ConsumeAnyToken(); + + DeclGroupPtrTy Ptr; + if (Tok.is(tok::annot_pragma_openmp)) { + Ptr = ParseOpenMPDeclarativeDirectiveWithExtDecl(IsInTagDecl, Level + 1); + } else { + // Here we expect to see some function declaration. + ParsedAttributesWithRange Attrs(AttrFactory); + MaybeParseCXX11Attributes(Attrs); + MaybeParseMicrosoftAttributes(Attrs); + ParsingDeclSpec PDS(*this); + Ptr = ParseExternalDeclaration(Attrs, &PDS); + } + if (!Ptr || Ptr.get().isNull()) + return DeclGroupPtrTy(); + if (Ptr.get().isDeclGroup()) { + Diag(Tok, diag::err_omp_single_decl_in_declare_simd); + return DeclGroupPtrTy(); > > > Would it make sense to return Ptr here instead so that further > diagnostics can be reported? > > ~Aaron ================ Comment at: lib/Parse/ParseOpenMP.cpp:149 @@ +148,3 @@ + MaybeParseCXX11Attributes(Attrs); + MaybeParseMicrosoftAttributes(Attrs); + ParsingDeclSpec PDS(*this); ---------------- This is not your fault, and should not be fixed by this patch, but this series of three: ParsedAttributesWithRange Attrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); MaybeParseMicrosoftAttributes(Attrs); occurs a lot. It would be nice to factor this into one function to reduce the amount of repeated logic. http://reviews.llvm.org/D10599 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits