[PATCH] Add support for __declspec(thread) under -fms-extensions

Aaron Ballman aaron at aaronballman.com
Tue Apr 29 13:11:02 PDT 2014


Thanks for working on this! I have a few comments below...

> Index: include/clang/AST/Decl.h
> ===================================================================
> --- include/clang/AST/Decl.h
> +++ include/clang/AST/Decl.h
> @@ -693,7 +693,7 @@
>      friend class ASTDeclReader;
>
>      unsigned SClass : 3;
> -    unsigned TSCSpec : 2;
> +    unsigned TSCSpec : 3;
>      unsigned InitStyle : 2;
>
>      /// \brief Whether this variable is the exception variable in a C++ catch
> @@ -725,7 +725,7 @@
>      /// the type of this declaration with its previous declaration.
>      unsigned PreviousDeclInSameBlockScope : 1;
>    };
> -  enum { NumVarDeclBits = 14 };
> +  enum { NumVarDeclBits = 15 };
>
>    friend class ASTDeclReader;
>    friend class StmtIteratorBase;
> @@ -754,7 +754,7 @@
>      /// Otherwise, the number of function parameter scopes enclosing
>      /// the function parameter scope in which this parameter was
>      /// declared.
> -    unsigned ScopeDepthOrObjCQuals : 7;
> +    unsigned ScopeDepthOrObjCQuals : 6;
>
>      /// The number of parameters preceding this parameter in the
>      /// function parameter scope in which it was declared.
> @@ -818,6 +818,7 @@
>        return TLS_None;
>      case TSCS___thread: // Fall through.
>      case TSCS__Thread_local:
> +    case TSCS___declspec_thread:
>        return TLS_Static;
>      case TSCS_thread_local:
>        return TLS_Dynamic;
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -1665,6 +1665,13 @@
>    let Documentation = [Undocumented];
>  }
>
> +def Thread : InheritableAttr {
> +  let Spellings = [Declspec<"thread">];
> +  let LangOpts = [MicrosoftExt];
> +  let Documentation = [Undocumented];

Please document this attribute (it's fine to punt on most of it and
simply point to MSDN).

> +  let Subjects = SubjectList<[Var]>;
> +}
> +
>  def Win64 : IgnoredAttr {
>    let Spellings = [Keyword<"__w64">];
>    let LangOpts = [MicrosoftExt];
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -2081,6 +2081,9 @@
>    "weak declaration cannot have internal linkage">;
>  def err_attribute_selectany_non_extern_data : Error<
>    "'selectany' can only be applied to data items with external linkage">;
> +def err_multiple_thread_specifiers : Error<
> +  "__declspec(thread) applied to variable declaration which already has a "
> +  "thread specifier">;
>  def err_attribute_dll_not_extern : Error<
>    "%q0 must have external linkage when declared %q1">;
>  def warn_attribute_invalid_on_definition : Warning<
> Index: include/clang/Basic/Specifiers.h
> ===================================================================
> --- include/clang/Basic/Specifiers.h
> +++ include/clang/Basic/Specifiers.h
> @@ -163,7 +163,9 @@
>      TSCS_thread_local,
>      /// C11 _Thread_local. Must be combined with either 'static' or 'extern'
>      /// if used at block scope.
> -    TSCS__Thread_local
> +    TSCS__Thread_local,
> +    /// __declspec(thread). Equivalent to GNU __thread.
> +    TSCS___declspec_thread
>    };
>
>    /// \brief Storage classes.
> Index: include/clang/Sema/DeclSpec.h
> ===================================================================
> --- include/clang/Sema/DeclSpec.h
> +++ include/clang/Sema/DeclSpec.h
> @@ -232,6 +232,7 @@
>    typedef ThreadStorageClassSpecifier TSCS;
>    static const TSCS TSCS_unspecified = clang::TSCS_unspecified;
>    static const TSCS TSCS___thread = clang::TSCS___thread;
> +  static const TSCS TSCS___declspec_thread = clang::TSCS___declspec_thread;
>    static const TSCS TSCS_thread_local = clang::TSCS_thread_local;
>    static const TSCS TSCS__Thread_local = clang::TSCS__Thread_local;
>
> Index: lib/AST/DeclPrinter.cpp
> ===================================================================
> --- lib/AST/DeclPrinter.cpp
> +++ lib/AST/DeclPrinter.cpp
> @@ -655,6 +655,8 @@
>      switch (D->getTSCSpec()) {
>      case TSCS_unspecified:
>        break;
> +    case TSCS___declspec_thread:
> +      break; // Nothing, the attribute printer will do it for us.
>      case TSCS___thread:
>        Out << "__thread ";
>        break;
> Index: lib/Sema/DeclSpec.cpp
> ===================================================================
> --- lib/Sema/DeclSpec.cpp
> +++ lib/Sema/DeclSpec.cpp
> @@ -383,6 +383,7 @@
>    switch (S) {
>    case DeclSpec::TSCS_unspecified:   return "unspecified";
>    case DeclSpec::TSCS___thread:      return "__thread";
> +  case DeclSpec::TSCS___declspec_thread: return "__declspec(thread)";
>    case DeclSpec::TSCS_thread_local:  return "thread_local";
>    case DeclSpec::TSCS__Thread_local: return "_Thread_local";
>    }
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -3697,6 +3697,18 @@
>      D->addAttr(IA);
>  }
>
> +static void handleDeclspecThreadAttr(Sema &S, Decl *D,
> +                                     const AttributeList &Attr) {
> +  VarDecl *VD = cast<VarDecl>(D);
> +  if (VD->getTSCSpec() != TSCS_unspecified) {
> +    S.Diag(Attr.getLoc(), diag::err_multiple_thread_specifiers);
> +    return;
> +  }

Looking at MSDN, don't we need to check more semantics? For instance,
the linkage:

"You can specify the thread attribute only on data items with static
storage duration. This includes global data objects (both static and
extern), local static objects, and static data members of classes. You
cannot declare automatic data objects with the thread attribute."

Also, if the type is a class, it needs to be POD (also according to MSDN).

> +  D->addAttr(::new (S.Context) ThreadAttr(
> +      Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
> +  VD->setTSCSpec(TSCS___declspec_thread);
> +}
> +
>  static void handleARMInterruptAttr(Sema &S, Decl *D,
>                                     const AttributeList &Attr) {
>    // Check the attribute arguments.
> @@ -4394,6 +4406,9 @@
>    case AttributeList::AT_SelectAny:
>      handleSimpleAttribute<SelectAnyAttr>(S, D, Attr);
>      break;
> +  case AttributeList::AT_Thread:
> +    handleDeclspecThreadAttr(S, D, Attr);
> +    break;
>
>    // Thread safety attributes:
>    case AttributeList::AT_AssertExclusiveLock:
> Index: test/SemaCXX/declspec-thread.cpp
> ===================================================================
> --- /dev/null
> +++ test/SemaCXX/declspec-thread.cpp
> @@ -0,0 +1,8 @@
> +// RUN: %clang_cc1 -fms-extensions -verify %s
> +
> +__thread __declspec(thread) int a; // expected-error {{already has a thread specifier}}
> +__declspec(thread) __thread int b; // expected-error {{already has a thread specifier}}
> +__declspec(thread) int c(); // expected-warning {{only applies to variables}}
> +__declspec(thread) int d;
> +int foo();
> +__declspec(thread) int e = foo(); // expected-error {{must be a constant expression}}
>

What about cases where redeclarations have a mismatched specifier?
(That's in addition to a test case for a non-POD test, and a
non-static storage duration.) I'd also like to see a test case where
__declspec(thread) shouldn't belong in the declaration (such as in a
typedef), and accepting parameters (just so we don't accidentally
break that in the future).

Thanks!

~Aaron

On Tue, Apr 29, 2014 at 3:59 PM, Reid Kleckner <rnk at google.com> wrote:
> Hi rsmith,
>
> I had to steal a bit from ScopeDepthOrObjCQuals.  This prevents us from
> parsing more than 63 nested function prototypes in a parameter context.
> We already can't parse more than 127, which is an arbitrary limit
> (PR19607).  If we care about supporting this, we should add an extra
> table to ASTContext for looking up large scope depth values like we do
> for parameter indices greater than 254.
>
> http://reviews.llvm.org/D3551
>
> Files:
>   include/clang/AST/Decl.h
>   include/clang/Basic/Attr.td
>   include/clang/Basic/DiagnosticSemaKinds.td
>   include/clang/Basic/Specifiers.h
>   include/clang/Sema/DeclSpec.h
>   lib/AST/DeclPrinter.cpp
>   lib/Sema/DeclSpec.cpp
>   lib/Sema/SemaDeclAttr.cpp
>   test/SemaCXX/declspec-thread.cpp
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



More information about the cfe-commits mailing list