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

Bataev, Alexey a.bataev at hotmail.com
Tue Jul 7 04:40:36 PDT 2015


> I tend to agree with hal that this should be a warning. It's low
> priority, definitely, but silently allowing the programmer to get away
> with incorrect code doesn't seem beneficial to anyone. It would also
> point out the two other places in your tests that have this exact same
> code pattern with no additional testing benefit. ;-)

This code is correct, OpenMP standard allows to mark the same function many times with the same pragma. Here is an excerpt:

#pragma omp declare simd [clause[[,] clause] ...] new-line
[#pragma omp declare simd [clause[[,] clause] ...] new-line]
[...]
function definition or declaration

There may be multiple declare simd directives for a function (C, C++, Fortran) or subroutine (Fortran).
  
> I would also like to see a test for #pragma omp declare simd that is
> at the end of the TU with no futher statements (to ensure it gets
> diagnosed).

Ok, will be added.

> Copy paste error?
Nope, I have to verify that the compiler allows to mark the same function several times with '#pragma omp declare simd' according to standard.

> This attribute has a pragma spelling where the namespace is openmp and
> the name is declare. It's the same pattern used by the loop hint and
> init_seg pragmas.

Can't add spellings, it breaks -ast-print mode (pragmas are printed just like attributes, directly after function declaration).

> Please document this attribute as it does have a pragma spelling.

Will add

> This attribute appears to be missing its Subjects clause as well.
> That's not information we currently care about for pragmas, but I
> would like the declarative part of the attribute to match the semantic
> expectations.

Will add too.

> I've not seen any tests for ObjC, so does this really apply to methods?
I'll fix this message.

> Capitalize the first word in the documentation part of the sentence.
Thanks, fixed.
  
> Drop the virtual, add override.
Fixed.

> Drop the "if"
Fixed

> Can you use a range-based for loop please?
Done

> This is not an implicit attribute because it's the direct result of
> the user typing something in source code.
Fixed.

> This really weirds me out. That means this function doesn't parse just
> the pragma, it also parses any external declaration.
Yes, we have to parse function right after pragma to be able correctly handle clauses associated with this pragma. These clauses may have refernces to function arguments and because of that we have to parse the function first and only then the pragma itself.

> The comments don't look correct here.
Missed this one, thanks.

> Is this safe to assert in a parsing method, or should this be diagnosed?
There always must be at least one token if the compiler works correctly - tok::annot_pragma_openmp. No need to diagnose.

> Has this situation already been diagnosed? If so, then why are we
> generating the tokens in a way that require this work? And if not, are
> we missing a diagnostic here?
Yes, this was diagnosed already. We need to remove cached tokens, so just skip them.

> Attribute is not implicit.
Fixed.
   

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

06.07.2015 16:13, Aaron Ballman пишет:
> On Mon, Jun 22, 2015 at 7:35 AM, Alexey Bataev <a.bataev at hotmail.com> wrote:
>> Hi rsmith, aaron.ballman, hfinkel, ejstotzer, fraggamuffin,
>>
>> Inital parsing/sema/serialization/deserialization support for '#pragma omp declare simd' directive.
>> The declare simd construct can be applied to a function to enable the creation of one or more versions that can process multiple arguments using SIMD instructions from a single invocation from a SIMD loop.
>> If the function has any declarations, then the declare simd construct for any declaration that has one must be equivalent to the one specified for the definition. Otherwise, the result is unspecified.
>> This pragma can be aplied many times to the same declaration.
>> Internally this pragma is represented as an attribute. But we need special processing for this pragma because it must be used before function declaration, this directive is appplied to.
>> If pragma is applied to method declared/defined in the class lexical context, then the delayed parsing procedure is used.
>> Also delayed parsing is used for poarsing a pragma itself, because this directive has several clauses that can reference arguments of the associated function/method.
>>
>> http://reviews.llvm.org/D10599
>>
>> Files:
>>    include/clang/AST/ASTMutationListener.h
>>    include/clang/Basic/Attr.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
>>
>> EMAIL PREFERENCES
>>    http://reviews.llvm.org/settings/panel/emailpreferences/
> Index: test/OpenMP/declare_simd_messages.cpp
> ===================================================================
> --- test/OpenMP/declare_simd_messages.cpp
> +++ test/OpenMP/declare_simd_messages.cpp
> @@ -0,0 +1,45 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp
> -x c++ -std=c++11 %s
> +
> +// expected-error at +1 {{expected an OpenMP directive}}
> +#pragma omp declare
> +
> +// expected-error at +2 {{'#pragma omp declare simd' can be applied to
> functions or methods only}}
> +#pragma omp declare simd
> +int a;
> +// expected-error at +2 {{'#pragma omp declare simd' can be applied to
> functions or methods only}}
> +#pragma omp declare simd
> +#pragma omp threadprivate(a)
> +int var;
> +#pragma omp threadprivate(var)
> +
> +// expected-error at +2 {{expected an OpenMP directive}}
> +#pragma omp declare simd
> +#pragma omp declare
> +
> +#pragma omp declare simd
> +#pragma omp declare simd
>
> I tend to agree with hal that this should be a warning. It's low
> priority, definitely, but silently allowing the programmer to get away
> with incorrect code doesn't seem beneficial to anyone. It would also
> point out the two other places in your tests that have this exact same
> code pattern with no additional testing benefit. ;-)
>
> +int main();
> +
> +#pragma omp declare simd
> +template <class C>
> +void h(C *hp, C *hp2, C *hq, C *lin) {
> +}
> +
> +#pragma omp declare simd
> +template <>
> +void h(int *hp, int *hp2, int *hq, int *lin) {
> +  h((float *)hp, (float *)hp2, (float *)hq, (float *)lin);
> +}
> +
> +template <class T>
> +struct St {
> +#pragma omp declare simd
> +  void h(T *hp) {
> +// expected-error at +1 {{unexpected OpenMP directive '#pragma omp declare simd'}}
> +#pragma omp declare simd
> +    *hp = *t;
> +  }
> +
> +private:
> +  T t;
> +};
>
> I would also like to see a test for #pragma omp declare simd that is
> at the end of the TU with no futher statements (to ensure it gets
> diagnosed).
>
> Index: test/OpenMP/declare_simd_ast_print.cpp
> ===================================================================
> --- test/OpenMP/declare_simd_ast_print.cpp
> +++ test/OpenMP/declare_simd_ast_print.cpp
> @@ -0,0 +1,103 @@
> +// RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++11 -ast-print %s |
> FileCheck %s
> +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
> +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -include-pch %t
> -fsyntax-only -verify %s -ast-print | FileCheck %s
> +// expected-no-diagnostics
> +
> +#ifndef HEADER
> +#define HEADER
> +
> +#pragma omp declare simd
> +void add_1(float *d);
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: void add_1(float *d);
> +//
> +
> +#pragma omp declare simd
> +template <class C> void h(C *hp, C *hp2, C *hq, C *lin) {
> +}
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: template <class C = int> void h(int *hp, int *hp2, int
> *hq, int *lin) {
> +// CHECK-NEXT: h((float *)hp, (float *)hp2, (float *)hq, (float *)lin);
> +// CHECK-NEXT: }
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: template <class C = float> void h(float *hp, float
> *hp2, float *hq, float *lin) {
> +// CHECK-NEXT: }
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK: template <class C> void h(C *hp, C *hp2, C *hq, C *lin) {
> +// CHECK-NEXT: }
> +//
> +
> +// Instatiate with <C=int> explicitly.
> +// Pragmas need to be same, otherwise standard says that's undefined behavior.
> +#pragma omp declare simd
> +template <>
> +void h(int *hp, int *hp2, int *hq, int *lin)
> +{
> +  // Instatiate with <C=float> implicitly.
> +  // This is special case where the directive is stored by Sema and is
> +  // generated together with the (pending) function instatiation.
> +  h((float*) hp, (float*) hp2, (float*) hq, (float*) lin);
> +}
> +
> +class VV {
> +  // CHECK: #pragma omp declare simd
> +  // CHECK-NEXT: int add(int a, int b)     {
> +  // CHECK-NEXT: return a + b;
> +  // CHECK-NEXT: }
> +  #pragma omp declare simd
> +  int add(int a, int b) { return a + b; }
> +
> +  // CHECK: #pragma omp declare simd
> +  // CHECK-NEXT: #pragma omp declare simd
> +  // CHECK-NEXT: float addpf(float *a, float *b)     {
> +  // CHECK-NEXT: return *a + *b;
> +  // CHECK-NEXT: }
> +  #pragma omp declare simd
> +  #pragma omp declare simd
>
> Copy paste error?
>
> +  float addpf(float *a, float *b) { return *a + *b; }
> +};
> +
> +// CHECK: template <int X = 16> class TVV {
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: int tadd(int a, int b);
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: float taddpf(float *a, float *b) {
> +// CHECK-NEXT: return *a + *b;
> +// CHECK-NEXT: }
> +// CHECK: }
> +template <int X>
> +class TVV {
> +public:
> +// CHECK: template <int X> class TVV {
> +  #pragma omp declare simd
> +  int tadd(int a, int b) { return a + b; }
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: int tadd(int a, int b) {
> +// CHECK-NEXT: return a + b;
> +// CHECK-NEXT: }
> +
> +
> +  #pragma omp declare simd
> +  float taddpf(float *a, float *b) { return *a + *b; }
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: float taddpf(float *a, float *b) {
> +// CHECK-NEXT: return *a + *b;
> +// CHECK-NEXT: }
> +};
> +// CHECK: };
> +
> +// CHECK: TVV<16> t16;
> +TVV<16> t16;
> +
> +void f() {
> +  float a = 1.0f, b = 2.0f;
> +  float r = t16.taddpf(&a, &b);
> +}
> +
> +#endif
> Index: test/OpenMP/declare_simd_ast_print.c
> ===================================================================
> --- test/OpenMP/declare_simd_ast_print.c
> +++ test/OpenMP/declare_simd_ast_print.c
> @@ -0,0 +1,17 @@
> +// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
> +// RUN: %clang_cc1 -fopenmp -emit-pch -o %t %s
> +// RUN: %clang_cc1 -fopenmp -include-pch %t -fsyntax-only -verify %s
> -ast-print | FileCheck %s
> +// expected-no-diagnostics
> +
> +#ifndef HEADER
> +#define HEADER
> +
> +#pragma omp declare simd
> +#pragma omp declare simd
>
> Copy paste error?
>
> +void add_1(float *d, float *s1, float *s2);
> +
> +// CHECK: #pragma omp declare simd
> +// CHECK-NEXT: #pragma omp declare simd
> +// CHECK-NEXT: void add_1(float *d, float *s1, float *s2)
> +
> +#endif
> Index: include/clang/Basic/OpenMPKinds.def
> ===================================================================
> --- include/clang/Basic/OpenMPKinds.def
> +++ include/clang/Basic/OpenMPKinds.def
> @@ -94,6 +94,7 @@
>   OPENMP_DIRECTIVE_EXT(parallel_for_simd, "parallel for simd")
>   OPENMP_DIRECTIVE_EXT(parallel_sections, "parallel sections")
>   OPENMP_DIRECTIVE_EXT(for_simd, "for simd")
> +OPENMP_DIRECTIVE_EXT(declare_simd, "declare simd")
>
>   // OpenMP clauses.
>   OPENMP_CLAUSE(if, OMPIfClause)
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -2044,3 +2044,19 @@
>     let SemaHandler = 0;
>     let Documentation = [Undocumented];
>   }
> +
> +def OMPDeclareSimdDecl : InheritableAttr {
> +  // This attribute has no spellings as it is only ever created implicitly.
> +  let Spellings = [];
>
> This attribute has a pragma spelling where the namespace is openmp and
> the name is declare. It's the same pattern used by the loop hint and
> init_seg pragmas.
>
> +  let SemaHandler = 0;
> +  let Documentation = [Undocumented];
>
> Please document this attribute as it does have a pragma spelling.
>
> +  let Args = [UnsignedArgument<"NumberOfDirectives">,
> BoolArgument<"Complete">];
> +  let AdditionalMembers = [{
> +  void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy) const {
> +    OS << "#pragma omp declare simd\n";
> +  }
> +  void setNumberOfDirectives(unsigned N) { numberOfDirectives = N; }
> +  void setComplete() { complete = true; }
>
> This attribute appears to be missing its Subjects clause as well.
> That's not information we currently care about for pragmas, but I
> would like the declarative part of the attribute to match the semantic
> expectations.
>
> +  }];
> +}
> +
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td
> +++ include/clang/Basic/DiagnosticParseKinds.td
> @@ -989,6 +989,8 @@
>     "'#pragma omp %0' cannot be an immediate substatement">;
>   def err_omp_expected_identifier_for_critical : Error<
>     "expected identifier specifying the name of the 'omp critical' directive">;
> +def err_omp_single_decl_in_declare_simd : Error<
> +  "single declaration is expected with 'declare simd' directive">;
>
>   // Pragma loop support.
>   def err_pragma_loop_missing_argument : Error<
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -7601,6 +7601,8 @@
>     "the 'copyprivate' clause must not be used with the 'nowait' clause">;
>   def note_omp_nowait_clause_here : Note<
>     "'nowait' clause is here">;
> +def err_omp_function_expected : Error<
> +  "'#pragma omp declare simd' can be applied to functions or methods only">;
>
> I've not seen any tests for ObjC, so does this really apply to methods?
>
>   } // end of OpenMP category
>
>   let CategoryName = "Related Result Type Issue" in {
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -144,6 +144,7 @@
>     class ObjCPropertyDecl;
>     class ObjCProtocolDecl;
>     class OMPThreadPrivateDecl;
> +  class OMPDeclareSimdDecl;
>     class OMPClause;
>     class OverloadCandidateSet;
>     class OverloadExpr;
> @@ -7758,6 +7759,13 @@
>                                          Stmt *AStmt, SourceLocation StartLoc,
>                                          SourceLocation EndLoc);
>
> +  /// \brief Called on well-formed '\#pragma omp declare simd' after parsing of
> +  /// the associated method/function.
> +  DeclGroupPtrTy ActOnOpenMPDeclareSimdDirective(ArrayRef<OMPClause *> Clauses,
> +                                                 Decl *ADecl,
> +                                                 SourceLocation StartLoc,
> +                                                 bool LastDirective);
> +
>     OMPClause *ActOnOpenMPSingleExprClause(OpenMPClauseKind Kind,
>                                            Expr *Expr,
>                                            SourceLocation StartLoc,
> Index: include/clang/AST/ASTMutationListener.h
> ===================================================================
> --- include/clang/AST/ASTMutationListener.h
> +++ include/clang/AST/ASTMutationListener.h
> @@ -113,6 +113,12 @@
>     /// \param D the declaration marked OpenMP threadprivate.
>     virtual void DeclarationMarkedOpenMPThreadPrivate(const Decl *D) {}
>
> +  /// \brief A declaration is marked as OpenMP declare simd which was not
> +  /// previously marked as declare simd.
> +  ///
> +  /// \param D the declaration marked OpenMP declare simd.
>
> Capitalize the first word in the documentation part of the sentence.
>
> +  virtual void DeclarationMarkedOpenMPDeclareSimd(const Decl *D) {}
> +
>     /// \brief A definition has been made visible by being redefined locally.
>     ///
>     /// \param D The definition that was previously not visible.
> Index: include/clang/Parse/Parser.h
> ===================================================================
> --- include/clang/Parse/Parser.h
> +++ include/clang/Parse/Parser.h
> @@ -1019,6 +1019,19 @@
>       CachedTokens *ExceptionSpecTokens;
>     };
>
> +  /// \brief An OpenMP declaration inside a class.
> +  struct LateParsedOpenMPDeclaration : public LateParsedDeclaration {
> +    explicit LateParsedOpenMPDeclaration(Parser *P) : Self(P) {}
> +
> +    virtual void ParseLexedMethodDeclarations();
>
> Drop the virtual, add override.
>
> +
> +    Parser *Self;
> +
> +    /// \brief The set of tokens that make up an exception-specification that
> +    /// has not yet been parsed.
> +    CachedTokens Tokens;
> +  };
> +
>     /// LateParsedMemberInitializer - An initializer for a non-static class data
>     /// member whose parsing must to be delayed until the class is completely
>     /// defined (C++11 [class.mem]p2).
> @@ -2364,7 +2377,13 @@
>     //===--------------------------------------------------------------------===//
>     // OpenMP: Directives and clauses.
>     /// \brief Parses declarative OpenMP directives.
> -  DeclGroupPtrTy ParseOpenMPDeclarativeDirective();
> +  /// \param Level Current level of declarative directive, in case if it is
>
> Drop the "if"
>
> +  /// allowed to apply multiple declarative directives to the same declaration.
> +  DeclGroupPtrTy ParseOpenMPDeclarativeDirective(bool IsInTagDecl,
> +                                                 unsigned Level = 0);
> +  /// \brief Late parse directive.
> +  void LateParseOpenMPDeclarativeDirective(OpenMPDirectiveKind DKind,
> +                                           SourceLocation Loc);
>     /// \brief Parses simple list of variables.
>     ///
>     /// \param Kind Kind of the directive.
> Index: include/clang/Serialization/ASTWriter.h
> ===================================================================
> --- include/clang/Serialization/ASTWriter.h
> +++ include/clang/Serialization/ASTWriter.h
> @@ -859,6 +859,7 @@
>                                       const ObjCCategoryDecl *ClassExt) override;
>     void DeclarationMarkedUsed(const Decl *D) override;
>     void DeclarationMarkedOpenMPThreadPrivate(const Decl *D) override;
> +  void DeclarationMarkedOpenMPDeclareSimd(const Decl *D) override;
>     void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override;
>   };
>
> Index: lib/Frontend/MultiplexConsumer.cpp
> ===================================================================
> --- lib/Frontend/MultiplexConsumer.cpp
> +++ lib/Frontend/MultiplexConsumer.cpp
> @@ -127,6 +127,7 @@
>                                       const ObjCCategoryDecl *ClassExt) override;
>     void DeclarationMarkedUsed(const Decl *D) override;
>     void DeclarationMarkedOpenMPThreadPrivate(const Decl *D) override;
> +  void DeclarationMarkedOpenMPDeclareSimd(const Decl *D) override;
>     void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override;
>
>   private:
> @@ -221,6 +222,11 @@
>     for (size_t i = 0, e = Listeners.size(); i != e; ++i)
>       Listeners[i]->DeclarationMarkedOpenMPThreadPrivate(D);
>   }
> +void MultiplexASTMutationListener::DeclarationMarkedOpenMPDeclareSimd(
> +    const Decl *D) {
> +  for (size_t i = 0, e = Listeners.size(); i != e; ++i)
> +    Listeners[i]->DeclarationMarkedOpenMPDeclareSimd(D);
> +}
>
> Can you use a range-based for loop please?
>
>   void MultiplexASTMutationListener::RedefinedHiddenDefinition(const
> NamedDecl *D,
>                                                                Module *M) {
>     for (auto *L : Listeners)
> Index: lib/Basic/OpenMPKinds.cpp
> ===================================================================
> --- lib/Basic/OpenMPKinds.cpp
> +++ lib/Basic/OpenMPKinds.cpp
> @@ -324,6 +324,8 @@
>         break;
>       }
>       break;
> +  case OMPD_declare_simd:
> +    break;
>     case OMPD_unknown:
>     case OMPD_threadprivate:
>     case OMPD_section:
> Index: lib/Sema/SemaOpenMP.cpp
> ===================================================================
> --- lib/Sema/SemaOpenMP.cpp
> +++ lib/Sema/SemaOpenMP.cpp
> @@ -1283,6 +1283,7 @@
>     case OMPD_barrier:
>     case OMPD_taskwait:
>     case OMPD_flush:
> +  case OMPD_declare_simd:
>       llvm_unreachable("OpenMP Directive is not allowed");
>     case OMPD_unknown:
>       llvm_unreachable("Unknown OpenMP directive");
> @@ -1989,6 +1990,7 @@
>                                        EndLoc);
>       break;
>     case OMPD_threadprivate:
> +  case OMPD_declare_simd:
>       llvm_unreachable("OpenMP Directive is not allowed");
>     case OMPD_unknown:
>       llvm_unreachable("Unknown OpenMP directive");
> @@ -2006,6 +2008,34 @@
>     return Res;
>   }
>
> +Sema::DeclGroupPtrTy
> +Sema::ActOnOpenMPDeclareSimdDirective(ArrayRef<OMPClause *> Clauses,
> +                                      Decl *ADecl, SourceLocation StartLoc,
> +                                      bool LastDirective) {
> +  if (auto *FTD = dyn_cast<FunctionTemplateDecl>(ADecl)) {
> +    ADecl = FTD->getTemplatedDecl();
> +  }
>
> Elide braces.
>
> +  if (!isa<FunctionDecl>(ADecl)) {
> +    Diag(ADecl->getLocation(), diag::err_omp_function_expected)
> +        << ADecl->getDeclContext()->isFileContext();
> +    return DeclGroupPtrTy();
> +  }
> +  if (auto *Attr = ADecl->getAttr<OMPDeclareSimdDeclAttr>()) {
> +    if (!Attr->getComplete()) {
> +      Attr->setNumberOfDirectives(Attr->getNumberOfDirectives() + 1);
> +      if (LastDirective)
> +        Attr->setComplete();
> +    }
> +  } else {
> +    auto *NewAttr = OMPDeclareSimdDeclAttr::CreateImplicit(
> +        Context, 1, LastDirective, SourceRange(StartLoc, StartLoc));
>
> This is not an implicit attribute because it's the direct result of
> the user typing something in source code.
>
> +    ADecl->addAttr(NewAttr);
> +    if (auto *ML = Context.getASTMutationListener())
> +      ML->DeclarationMarkedOpenMPDeclareSimd(ADecl);
> +  }
> +  return ConvertDeclToDeclGroup(ADecl);
> +}
> +
>   StmtResult Sema::ActOnOpenMPParallelDirective(ArrayRef<OMPClause *> Clauses,
>                                                 Stmt *AStmt,
>                                                 SourceLocation StartLoc,
> Index: lib/AST/DeclPrinter.cpp
> ===================================================================
> --- lib/AST/DeclPrinter.cpp
> +++ lib/AST/DeclPrinter.cpp
> @@ -36,6 +36,7 @@
>       void ProcessDeclGroup(SmallVectorImpl<Decl*>& Decls);
>
>       void Print(AccessSpecifier AS);
> +    void print(OMPDeclareSimdDeclAttr *A);
>
>       /// Print an Objective-C method type in parentheses.
>       ///
> @@ -405,7 +406,22 @@
>     }
>   }
>
> +void DeclPrinter::print(OMPDeclareSimdDeclAttr *A) {
> +  for (unsigned i = 0; i < A->getNumberOfDirectives(); ++i) {
> +    A->printPrettyPragma(Out, Policy);
> +    Indent();
> +  }
> +}
> +
>   void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
> +  if (auto *Attr = D->getAttr<OMPDeclareSimdDeclAttr>()) {
> +    if (D->getTemplatedKind() !=
> +            FunctionDecl::TK_FunctionTemplateSpecialization &&
> +        D->getTemplatedKind() != FunctionDecl::TK_FunctionTemplate &&
> +        D->getTemplatedKind() !=
> +            FunctionDecl::TK_DependentFunctionTemplateSpecialization)
> +      print(Attr);
> +  }
>     CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
>     CXXConversionDecl *ConversionDecl = dyn_cast<CXXConversionDecl>(D);
>     if (!Policy.SuppressSpecifiers) {
> @@ -912,11 +928,17 @@
>     if (PrintInstantiation) {
>       TemplateParameterList *Params = D->getTemplateParameters();
>       for (auto *I : D->specializations()) {
> +      if (auto *Attr = I->getAttr<OMPDeclareSimdDeclAttr>()) {
> +        print(Attr);
> +      }
>
> Elide braces.
>
>         PrintTemplateParameters(Params, I->getTemplateSpecializationArgs());
>         Visit(I);
>       }
>     }
>
> +  if (auto *Attr = D->getTemplatedDecl()->getAttr<OMPDeclareSimdDeclAttr>()) {
> +    print(Attr);
> +  }
>
> Elide braces.
>
>     return VisitRedeclarableTemplateDecl(D);
>   }
>
> Index: lib/Parse/ParseDeclCXX.cpp
> ===================================================================
> --- lib/Parse/ParseDeclCXX.cpp
> +++ lib/Parse/ParseDeclCXX.cpp
> @@ -2942,7 +2942,7 @@
>         }
>
>         if (Tok.is(tok::annot_pragma_openmp)) {
> -        ParseOpenMPDeclarativeDirective();
> +        ParseOpenMPDeclarativeDirective(/*IsInTagDecl=*/true);
>           continue;
>         }
>
> Index: lib/Parse/ParseOpenMP.cpp
> ===================================================================
> --- lib/Parse/ParseOpenMP.cpp
> +++ lib/Parse/ParseOpenMP.cpp
> @@ -30,18 +30,26 @@
>     // E.g.: OMPD_for OMPD_simd ===> OMPD_for_simd
>     // TODO: add other combined directives in topological order.
>     const OpenMPDirectiveKind F[][3] = {
> -    { OMPD_for, OMPD_simd, OMPD_for_simd },
> -    { OMPD_parallel, OMPD_for, OMPD_parallel_for },
> -    { OMPD_parallel_for, OMPD_simd, OMPD_parallel_for_simd },
> -    { OMPD_parallel, OMPD_sections, OMPD_parallel_sections }
> +      {OMPD_unknown /*declare*/, OMPD_simd, OMPD_declare_simd},
> +      {OMPD_for, OMPD_simd, OMPD_for_simd},
> +      {OMPD_parallel, OMPD_for, OMPD_parallel_for},
> +      {OMPD_parallel_for, OMPD_simd, OMPD_parallel_for_simd},
> +      {OMPD_parallel, OMPD_sections, OMPD_parallel_sections}
>     };
>     auto Tok = P.getCurToken();
>     auto DKind =
>         Tok.isAnnotation()
>             ? OMPD_unknown
>             : getOpenMPDirectiveKind(P.getPreprocessor().getSpelling(Tok));
> +  bool TokenMatched = false;
>     for (unsigned i = 0; i < llvm::array_lengthof(F); ++i) {
> -    if (DKind == F[i][0]) {
> +    if (!Tok.isAnnotation() && DKind == OMPD_unknown) {
> +      TokenMatched =
> +          (i == 0) && !P.getPreprocessor().getSpelling(Tok).compare("declare");
> +    } else {
> +      TokenMatched = DKind == F[i][0];
> +    }
>
> Elide braces.
>
> +    if (TokenMatched) {
>         Tok = P.getPreprocessor().LookAhead(0);
>         auto SDKind =
>             Tok.isAnnotation()
> @@ -61,13 +69,17 @@
>   ///       threadprivate-directive:
>   ///         annot_pragma_openmp 'threadprivate' simple-variable-list
>   ///
> -Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirective() {
> +Parser::DeclGroupPtrTy
> +Parser::ParseOpenMPDeclarativeDirective(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:
> @@ -85,6 +97,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();
> +    }
>
> Elide braces.
>
> +    if (IsInTagDecl) {
> +      LateParseOpenMPDeclarativeDirective(/*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 = ParseOpenMPDeclarativeDirective(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);
>
> This really weirds me out. That means this function doesn't parse just
> the pragma, it also parses any external declaration.
>
> +    }
> +    if (!Ptr || Ptr.get().isNull())
> +      return DeclGroupPtrTy();
> +    if (Ptr.get().isDeclGroup()) {
> +      Diag(Tok, diag::err_omp_single_decl_in_declare_simd);
> +      return DeclGroupPtrTy();
> +    }
> +
> +    // Append the current token at the end of the new token stream so that it
> +    // doesn't get lost.
> +    CachedPragmas.push_back(Tok);
> +    // Push back tokens for pragma.
> +    PP.EnterTokenStream(CachedPragmas.data(), CachedPragmas.size(),
> +                        /*DisableMacroExpansion=*/true,
> +                        /*OwnsTokens=*/false);
> +    // Parse pragma itself.
> +    // Consume the previously pushed token.
> +    ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
> +
> +    Actions.StartOpenMPClauses();
> +    while (Tok.isNot(tok::annot_pragma_openmp_end)) {
> +      OpenMPClauseKind CKind = Tok.isAnnotation()
> +                                   ? OMPC_unknown
> +                                   : getOpenMPClauseKind(PP.getSpelling(Tok));
> +      OMPClause *Clause =
> +          ParseOpenMPClause(DKind, CKind, !FirstClauses[CKind].getInt());
> +      FirstClauses[CKind].setInt(true);
> +      if (Clause) {
> +        FirstClauses[CKind].setPointer(Clause);
> +        Clauses.push_back(Clause);
> +      }
> +
> +      // Skip ',' if any.
> +      if (Tok.is(tok::comma))
> +        ConsumeToken();
> +    }
> +    Actions.EndOpenMPClauses();
> +    // Consume final annot_pragma_openmp_end.
> +    ConsumeToken();
> +
> +    return Actions.ActOnOpenMPDeclareSimdDirective(
> +        Clauses, Ptr.get().getSingleDecl(), Loc, Level == 0);
> +  }
>     case OMPD_unknown:
>       Diag(Tok, diag::err_omp_unknown_directive);
>       break;
> @@ -118,6 +210,69 @@
>     return DeclGroupPtrTy();
>   }
>
> +/// \brief Late parsing of declarative OpenMP directives.
> +///
> +///       threadprivate-directive:
> +///         annot_pragma_openmp 'threadprivate' simple-variable-list
>
> The comments don't look correct here.
>
> +///         annot_pragma_openmp_end
> +///
> +void Parser::LateParseOpenMPDeclarativeDirective(OpenMPDirectiveKind DKind,
> +                                                 SourceLocation Loc) {
> +  if (DKind == OMPD_declare_simd) {
> +    LateParsedOpenMPDeclaration *Decl = new LateParsedOpenMPDeclaration(this);
> +    getCurrentClass().LateParsedDeclarations.push_back(Decl);
> +
> +    Token LocalTok;
> +    LocalTok.startToken();
> +    LocalTok.setKind(tok::annot_pragma_openmp);
> +    LocalTok.setLocation(Loc);
> +    LocalTok.setAnnotationValue(
> +        reinterpret_cast<void *>(static_cast<uintptr_t>(OMPD_declare_simd)));
> +    Decl->Tokens.push_back(LocalTok);
> +
> +    do {
> +      while (Tok.isNot(tok::annot_pragma_openmp_end) && Tok.isNot(tok::eof)) {
> +        Decl->Tokens.push_back(Tok);
> +        ConsumeAnyToken();
> +      }
> +      Decl->Tokens.push_back(Tok);
> +      if (Tok.isNot(tok::eof))
> +        ConsumeAnyToken();
> +    } while (Tok.is(tok::annot_pragma_openmp));
> +
> +    LexTemplateFunctionForLateParsing(Decl->Tokens);
> +  }
> +}
> +
> +/// \brief Actual parsing of late OpenMP declaration.
> +void Parser::LateParsedOpenMPDeclaration::ParseLexedMethodDeclarations() {
> +  // Save the current token position.
> +  SourceLocation OrigLoc = Self->Tok.getLocation();
> +
> +  assert(!Tokens.empty() && "Empty body!");
>
> Is this safe to assert in a parsing method, or should this be diagnosed?
>
> +  // Append the current token at the end of the new token stream so that it
> +  // doesn't get lost.
> +  Tokens.push_back(Self->Tok);
> +  Self->PP.EnterTokenStream(Tokens.data(), Tokens.size(), true, false);
> +
> +  // Consume the previously pushed token.
> +  Self->ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
> +
> +  Self->ParseOpenMPDeclarativeDirective(/*IsInTagDecl=*/false);
> +
> +  if (Self->Tok.getLocation() != OrigLoc) {
> +    // Due to parsing error, we either went over the cached tokens or
> +    // there are still cached tokens left. If it's the latter case skip the
> +    // leftover tokens.
> +    // Since this is an uncommon situation that should be avoided, use the
> +    // expensive isBeforeInTranslationUnit call.
> +    if (Self->PP.getSourceManager().isBeforeInTranslationUnit(
> +            Self->Tok.getLocation(), OrigLoc))
> +      while (Self->Tok.getLocation() != OrigLoc && Self->Tok.isNot(tok::eof))
> +        Self->ConsumeAnyToken();
>
> Has this situation already been diagnosed? If so, then why are we
> generating the tokens in a way that require this work? And if not, are
> we missing a diagnostic here?
>
> +  }
> +}
> +
>   /// \brief Parsing of declarative or executable OpenMP directives.
>   ///
>   ///       threadprivate-directive:
> @@ -274,6 +429,11 @@
>       OMPDirectiveScope.Exit();
>       break;
>     }
> +  case OMPD_declare_simd:
> +    Diag(Tok, diag::err_omp_unexpected_directive)
> +        << getOpenMPDirectiveName(DKind);
> +    SkipUntil(tok::annot_pragma_openmp_end);
> +    break;
>     case OMPD_unknown:
>       Diag(Tok, diag::err_omp_unknown_directive);
>       SkipUntil(tok::annot_pragma_openmp_end);
> Index: lib/Parse/Parser.cpp
> ===================================================================
> --- lib/Parse/Parser.cpp
> +++ lib/Parse/Parser.cpp
> @@ -653,7 +653,7 @@
>       HandlePragmaOpenCLExtension();
>       return DeclGroupPtrTy();
>     case tok::annot_pragma_openmp:
> -    return ParseOpenMPDeclarativeDirective();
> +    return ParseOpenMPDeclarativeDirective(/*IsInTagDecl=*/false);
>     case tok::annot_pragma_ms_pointers_to_members:
>       HandlePragmaMSPointersToMembers();
>       return DeclGroupPtrTy();
> Index: lib/Serialization/ASTReaderDecl.cpp
> ===================================================================
> --- lib/Serialization/ASTReaderDecl.cpp
> +++ lib/Serialization/ASTReaderDecl.cpp
> @@ -3888,6 +3888,14 @@
>             Reader.Context, ReadSourceRange(Record, Idx)));
>         break;
>
> +    case UPD_DECL_MARKED_OPENMP_DECLARE_SIMD: {
> +      auto *Attr = OMPDeclareSimdDeclAttr::CreateImplicit(
> +          Reader.Context, Record[Idx++], Record[Idx++] != 0,
> +          ReadSourceRange(Record, Idx));
>
> Attribute is not implicit.
>
> +      D->addAttr(Attr);
> +      break;
> +    }
> +
>       case UPD_DECL_EXPORTED:
>         unsigned SubmoduleID = readSubmoduleID(Record, Idx);
>         auto *Exported = cast<NamedDecl>(D);
> Index: lib/Serialization/ASTCommon.h
> ===================================================================
> --- lib/Serialization/ASTCommon.h
> +++ lib/Serialization/ASTCommon.h
> @@ -36,6 +36,7 @@
>     UPD_MANGLING_NUMBER,
>     UPD_STATIC_LOCAL_NUMBER,
>     UPD_DECL_MARKED_OPENMP_THREADPRIVATE,
> +  UPD_DECL_MARKED_OPENMP_DECLARE_SIMD,
>     UPD_DECL_EXPORTED
>   };
>
> Index: lib/Serialization/ASTWriter.cpp
> ===================================================================
> --- lib/Serialization/ASTWriter.cpp
> +++ lib/Serialization/ASTWriter.cpp
> @@ -4615,6 +4615,14 @@
>                          Record);
>           break;
>
> +      case UPD_DECL_MARKED_OPENMP_DECLARE_SIMD: {
> +        auto *Attr = D->getAttr<OMPDeclareSimdDeclAttr>();
> +        AddSourceRange(Attr->getRange(), Record);
> +        Record.push_back(Attr->getNumberOfDirectives());
> +        Record.push_back(Attr->getComplete() ? 1 : 0);
> +        break;
> +      }
> +
>         case UPD_DECL_EXPORTED:
>           Record.push_back(getSubmoduleID(Update.getModule()));
>           break;
> @@ -5761,6 +5769,14 @@
>     DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_MARKED_OPENMP_THREADPRIVATE));
>   }
>
> +void ASTWriter::DeclarationMarkedOpenMPDeclareSimd(const Decl *D) {
> +  assert(!WritingAST && "Already writing the AST!");
> +  if (!D->isFromASTFile())
> +    return;
> +
> +  DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_MARKED_OPENMP_DECLARE_SIMD));
> +}
> +
>   void ASTWriter::RedefinedHiddenDefinition(const NamedDecl *D, Module *M) {
>     assert(!WritingAST && "Already writing the AST!");
>     assert(D->isHidden() && "expected a hidden declaration");





More information about the cfe-commits mailing list