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
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
