[PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

[hfinkel at anl.gov]

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