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

[Bataev, Alexey]

> Would it make sense to return Ptr here instead so that further
> diagnostics can be reported?
I think you're right. Will be fixed.

Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team

08.07.2015 23:55, Aaron Ballman пишет:
> LGTM with one question below. I would wait for review from Richard or
> Hal before committing.
>
>> 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
>
> On Wed, Jul 8, 2015 at 12:35 AM, Alexey Bataev <a.bataev at hotmail.com> wrote:
>>
>> Update after review
>>
>>
>> http://reviews.llvm.org/D10599
>>
>> Files:
>>    include/clang/AST/ASTMutationListener.h
>>    include/clang/Basic/Attr.td
>>    include/clang/Basic/AttrDocs.td
>>    include/clang/Basic/DiagnosticParseKinds.td
>>    include/clang/Basic/DiagnosticSemaKinds.td
>>    include/clang/Basic/OpenMPKinds.def
>>    include/clang/Parse/Parser.h
>>    include/clang/Sema/Sema.h
>>    include/clang/Serialization/ASTWriter.h
>>    lib/AST/DeclPrinter.cpp
>>    lib/Basic/OpenMPKinds.cpp
>>    lib/Frontend/MultiplexConsumer.cpp
>>    lib/Parse/ParseDeclCXX.cpp
>>    lib/Parse/ParseOpenMP.cpp
>>    lib/Parse/Parser.cpp
>>    lib/Sema/SemaOpenMP.cpp
>>    lib/Serialization/ASTCommon.h
>>    lib/Serialization/ASTReaderDecl.cpp
>>    lib/Serialization/ASTWriter.cpp
>>    test/OpenMP/declare_simd_ast_print.c
>>    test/OpenMP/declare_simd_ast_print.cpp
>>    test/OpenMP/declare_simd_messages.cpp
>>