[PATCH] Add support for __declspec(thread) under -fms-extensions
Aaron Ballman
aaron at aaronballman.com
Tue Apr 29 16:42:22 PDT 2014
Just a few minor nits from me, but aside from those, LGTM!
> 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];
Looks like you forgot to hook this up to ThreadDocs. ;-)
Which may make for an interesting feature for the doc generator -- if
it finds documentation that's never used, we should probably warn you.
> + let Subjects = SubjectList<[Var]>;
> +}
> +
> def Win64 : IgnoredAttr {
> let Spellings = [Keyword<"__w64">];
> let LangOpts = [MicrosoftExt];
> Index: include/clang/Basic/AttrDocs.td
> ===================================================================
> --- include/clang/Basic/AttrDocs.td
> +++ include/clang/Basic/AttrDocs.td
> @@ -50,6 +50,23 @@
> }];
> }
>
> +def ThreadDocs : Documentation {
> + let Category = DocCatVariable;
> + let Content = [{
> +The ``__declspec(thread)`` attribute declares a variable with thread local
> +storage. It is available under the ``-fms-extensions`` flag for MSVC
> +compatiblity. Documentation for the Visual C++ attribute is available on MSDN_.
> +
> +.. _MSDN: http://msdn.microsoft.com/en-us/library/9w1sdazb.aspx
> +
> +In Clang, ``__declspec(thread)`` is generally equivalent in functionality to the
> +GNU ``__thread`` keyword. The variable must not have a destructor and must have
> +a constant initializer, if any. The attribute only applies to variables
> +declared with static storage duration, such as globals, class static data
> +members, and static locals.
> + }];
> +}
> +
> def CarriesDependencyDocs : Documentation {
> let Category = DocCatFunction;
> let Content = [{
> 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_declspec_thread_on_thread_variable : Error<
> + "'__declspec(thread)' applied to variable that already has a "
> + "thread-local storage specifier">;
> def err_attribute_dll_not_extern : Error<
> "%q0 must have external linkage when declared %q1">;
> def warn_attribute_invalid_on_definition : Warning<
> @@ -5996,6 +5999,8 @@
> "initializer for thread-local variable must be a constant expression">;
> def err_thread_nontrivial_dtor : Error<
> "type of thread-local variable has non-trivial destruction">;
> +def err_thread_nontrivial_ctor : Error<
> + "type of thread-local variable has non-trivial construction">;
> def note_use_thread_local : Note<
> "use 'thread_local' to allow this">;
>
> 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: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -1955,6 +1955,11 @@
> SectionAttr *mergeSectionAttr(Decl *D, SourceRange Range, StringRef Name,
> unsigned AttrSpellingListIndex);
>
> + /// Check if the thread storage specifier is valid, and set it if so. Return
> + /// if the thread storage specifier was set.
> + bool handleThreadStorageSpec(ThreadStorageClassSpecifier TSCS, VarDecl *VD,
> + SourceLocation SpecifierLoc);
> +
> /// \brief Describes the kind of merge to perform for availability
> /// attributes (including "deprecated", "unavailable", and "availability").
> enum AvailabilityMergeKind {
> 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/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -5041,6 +5041,35 @@
> return true;
> }
>
> +bool Sema::handleThreadStorageSpec(ThreadStorageClassSpecifier TSCS,
> + VarDecl *VD, SourceLocation SpecifierLoc) {
> + if (!Context.getTargetInfo().isTLSSupported()) {
> + Diag(SpecifierLoc, diag::err_thread_unsupported);
> + return false;
> + }
> +
> + if (!VD->hasLocalStorage()) {
> + VD->setTSCSpec(TSCS);
> + return true;
> + }
> +
> + // C++11 [dcl.stc]p4:
> + // When thread_local is applied to a variable of block scope the
> + // storage-class-specifier static is implied if it does not appear
> + // explicitly.
> + // Core issue: 'static' is not implied if the variable is declared
> + // 'extern'.
> + if (VD->getStorageClass() == SC_None && TSCS == TSCS_thread_local &&
> + VD->getDeclContext()->isFunctionOrMethod()) {
> + VD->setTSCSpec(TSCS);
> + return true;
> + }
> +
> + Diag(SpecifierLoc, diag::err_thread_non_global)
> + << DeclSpec::getSpecifierName(TSCS);
> + return false;
> +}
> +
> NamedDecl *
> Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
> TypeSourceInfo *TInfo, LookupResult &Previous,
> @@ -5333,28 +5362,9 @@
> if (IsLocalExternDecl)
> NewVD->setLocalExternDecl();
>
> - if (DeclSpec::TSCS TSCS = D.getDeclSpec().getThreadStorageClassSpec()) {
> - if (NewVD->hasLocalStorage()) {
> - // C++11 [dcl.stc]p4:
> - // When thread_local is applied to a variable of block scope the
> - // storage-class-specifier static is implied if it does not appear
> - // explicitly.
> - // Core issue: 'static' is not implied if the variable is declared
> - // 'extern'.
> - if (SCSpec == DeclSpec::SCS_unspecified &&
> - TSCS == DeclSpec::TSCS_thread_local &&
> - DC->isFunctionOrMethod())
> - NewVD->setTSCSpec(TSCS);
> - else
> - Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
> - diag::err_thread_non_global)
> - << DeclSpec::getSpecifierName(TSCS);
> - } else if (!Context.getTargetInfo().isTLSSupported())
> - Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
> - diag::err_thread_unsupported);
> - else
> - NewVD->setTSCSpec(TSCS);
> - }
> + if (DeclSpec::TSCS TSCS = D.getDeclSpec().getThreadStorageClassSpec())
> + handleThreadStorageSpec(TSCS, NewVD,
> + D.getDeclSpec().getThreadStorageClassSpecLoc());
>
> // C99 6.7.4p3
> // An inline definition of a function with external linkage shall
> @@ -8913,6 +8923,17 @@
> Diag(var->getLocation(), diag::warn_missing_variable_declarations) << var;
> }
>
> + if (var->getTSCSpec() == TSCS___declspec_thread) {
> + const CXXRecordDecl *RD = var->getType()->getAsCXXRecordDecl();
> + if (RD && RD->hasNonTrivialDefaultConstructor()) {
> + // MSVC doesn't allow thread local variables with any constructors, but we
> + // only look for the default constructor.
> + Diag(var->getLocation(), diag::err_thread_nontrivial_ctor);
Would it make sense for us to note where the declaration of that constructor is?
> + if (getLangOpts().CPlusPlus11)
> + Diag(var->getLocation(), diag::note_use_thread_local);
What about for C11?
> + }
> + }
> +
> if (var->getTLSKind() == VarDecl::TLS_Static &&
> var->getType().isDestructedType()) {
> // GNU C++98 edits for __thread, [basic.start.term]p3:
> 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_declspec_thread_on_thread_variable);
> + return;
> + }
> + if (S.handleThreadStorageSpec(TSCS___declspec_thread, VD, Attr.getLoc()))
> + D->addAttr(::new (S.Context) ThreadAttr(
> + Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
> +}
> +
> 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,42 @@
> +// RUN: %clang_cc1 -std=c++11 -fms-extensions -verify %s
> +
> +__thread __declspec(thread) int a; // expected-error {{already has a thread-local storage specifier}}
> +__declspec(thread) __thread int b; // expected-error {{already has a thread-local storage 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}} expected-note {{thread_local}}
> +
> +struct HasCtor { HasCtor(); int x; };
> +__declspec(thread) HasCtor f; // expected-error {{non-trivial construction}} expected-note {{thread_local}}
> +
> +struct HasDtor { ~HasDtor(); int x; };
> +__declspec(thread) HasDtor g; // expected-error {{non-trivial destruction}} expected-note {{thread_local}}
> +
> +struct HasDefaultedDefaultCtor {
> + HasDefaultedDefaultCtor() = default;
> + int x;
> +};
> +__declspec(thread) HasDefaultedDefaultCtor h;
> +
> +struct HasConstexprCtor {
> + constexpr HasConstexprCtor(int x) : x(x) {}
> + int x;
> +};
> +__declspec(thread) HasConstexprCtor i(42);
> +
> +int foo() {
> + __declspec(thread) int a; // expected-error {{must have global storage}}
> + static __declspec(thread) int b;
> +}
> +
> +extern __declspec(thread) int fwd_thread_var;
> +__declspec(thread) int fwd_thread_var = 5;
> +
> +extern int fwd_thread_var_mismatch; // expected-note {{previous declaration}}
> +__declspec(thread) int fwd_thread_var_mismatch = 5; // expected-error-re {{thread-local {{.*}} follows non-thread-local}}
> +
> +extern __declspec(thread) int thread_mismatch_2; // expected-note {{previous declaration}}
> +int thread_mismatch_2 = 5; // expected-error-re {{non-thread-local {{.*}} follows thread-local}}
> +
> +typedef __declspec(thread) int tls_int_t; // expected-warning {{only applies to variables}}
>
~Aaron
On Tue, Apr 29, 2014 at 6:13 PM, Reid Kleckner <rnk at google.com> wrote:
> - Add more tests
>
> http://reviews.llvm.org/D3551
>
> Files:
> include/clang/AST/Decl.h
> include/clang/Basic/Attr.td
> include/clang/Basic/AttrDocs.td
> include/clang/Basic/DiagnosticSemaKinds.td
> include/clang/Basic/Specifiers.h
> include/clang/Sema/DeclSpec.h
> include/clang/Sema/Sema.h
> lib/AST/DeclPrinter.cpp
> lib/Sema/DeclSpec.cpp
> lib/Sema/SemaDecl.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