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

Aaron Ballman aaron.ballman at gmail.com
Tue Jul 7 05:18:04 PDT 2015


On Tue, Jul 7, 2015 at 7:40 AM, Bataev, Alexey <a.bataev at hotmail.com> wrote:
>> 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:

Eww, but fair enough. :-)

>
> #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.

Fair, but we don't need that tested multiple times.

>
>> 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).

That means -ast-print is broken and should be fixed.

>
>> 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.

Can we update the name of the function to suggest this?

>
>> 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.

Good deal, thank you!

>
>> 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.

Thank you!

~Aaron

>
>> 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