Well, this is at least a breach of process, since the request went neither to the code owner nor to the mailing list. I don't particularly see why we should take a new feature into the branch at this stage. This should be easy to work around in client code with a portability macro which expands to either __declspec(selectany) or __attribute__((weak)), right?
On Tue, May 21, 2013 at 11:09 AM, Reid Kleckner <[email protected]> wrote: > Erik Schwiebert requested it, since apparently it makes it significantly > easier for him to port code over to Clang from MSVC. I figured it would be > OK since before this change using selectany would lead to link errors, so > this should strictly fix code. I forgot to think about the code drift > between 3.3 and TOT. > > > On Tue, May 21, 2013 at 1:39 PM, Richard Smith <[email protected]>wrote: > >> Why was this change merged to the branch at all? >> >> On Tue, May 21, 2013 at 5:00 AM, NAKAMURA Takumi <[email protected]>wrote: >> >>> Bill and Reid, it depends on r181677 and r182096. release_33 has been >>> broken. >>> >>> SemaDeclAttr.cpp: In function ‘void handleSelectAnyAttr(clang::Sema&, >>> clang::Decl*, const clang::AttributeList&)’: >>> SemaDeclAttr.cpp:4687:33: error: ‘checkMicrosoftExt’ was not declared >>> in this scope >>> >>> SemaDecl.cpp: In function ‘void >>> checkAttributesAfterMerging(clang::Sema&, clang::NamedDecl&)’: >>> SemaDecl.cpp:4645:38: error: ‘class clang::NamedDecl’ has no member >>> named ‘isExternallyVisible’ >>> >>> ...Takumi >>> >>> >>> 2013/5/21 Bill Wendling <[email protected]>: >>> > Author: void >>> > Date: Mon May 20 19:06:11 2013 >>> > New Revision: 182337 >>> > >>> > URL: http://llvm.org/viewvc/llvm-project?rev=182337&view=rev >>> > Log: >>> > Merging r182266: >>> > >>> ------------------------------------------------------------------------ >>> > r182266 | rnk | 2013-05-20 07:02:37 -0700 (Mon, 20 May 2013) | 13 lines >>> > >>> > Implement __declspec(selectany) under -fms-extensions >>> > >>> > selectany only applies to externally visible global variables. It has >>> > the effect of making the data weak_odr. >>> > >>> > The MSDN docs suggest that unused definitions can only be dropped at >>> > linktime, so Clang uses weak instead of linkonce. MSVC optimizes away >>> > references to constant selectany data, so it must assume that there is >>> > only one definition, hence weak_odr. >>> > >>> > Reviewers: espindola >>> > >>> > Differential Revision: http://llvm-reviews.chandlerc.com/D814 >>> > >>> ------------------------------------------------------------------------ >>> > >>> > Added: >>> > cfe/branches/release_33/test/SemaCXX/attr-selectany.cpp >>> > - copied unchanged from r182266, >>> cfe/trunk/test/SemaCXX/attr-selectany.cpp >>> > Modified: >>> > cfe/branches/release_33/ (props changed) >>> > cfe/branches/release_33/include/clang/Basic/Attr.td >>> > cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td >>> > cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp >>> > cfe/branches/release_33/lib/Sema/SemaDecl.cpp >>> > cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp >>> > cfe/branches/release_33/test/CodeGen/ms-declspecs.c >>> > >>> > Propchange: cfe/branches/release_33/ >>> > >>> ------------------------------------------------------------------------------ >>> > --- svn:mergeinfo (original) >>> > +++ svn:mergeinfo Mon May 20 19:06:11 2013 >>> > @@ -1,4 +1,4 @@ >>> > /cfe/branches/type-system-rewrite:134693-134817 >>> > -/cfe/trunk:181286,181299,181368,181465,181728,181750,181909,182072 >>> > >>> +/cfe/trunk:181286,181299,181368,181465,181728,181750,181909,182072,182266 >>> > /cfe/trunk/test:170344 >>> > /cfe/trunk/test/SemaTemplate:126920 >>> > >>> > Modified: cfe/branches/release_33/include/clang/Basic/Attr.td >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/Attr.td?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/include/clang/Basic/Attr.td (original) >>> > +++ cfe/branches/release_33/include/clang/Basic/Attr.td Mon May 20 >>> 19:06:11 2013 >>> > @@ -948,6 +948,10 @@ def ForceInline : InheritableAttr { >>> > let Spellings = [Keyword<"__forceinline">]; >>> > } >>> > >>> > +def SelectAny : InheritableAttr { >>> > + let Spellings = [Declspec<"selectany">]; >>> > +} >>> > + >>> > def Win64 : InheritableAttr { >>> > let Spellings = [Keyword<"__w64">]; >>> > } >>> > >>> > Modified: >>> cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td >>> (original) >>> > +++ cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td >>> Mon May 20 19:06:11 2013 >>> > @@ -1948,6 +1948,8 @@ def warn_weak_identifier_undeclared : Wa >>> > "weak identifier %0 never declared">; >>> > def err_attribute_weak_static : Error< >>> > "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 warn_attribute_weak_import_invalid_on_definition : Warning< >>> > "'weak_import' attribute cannot be specified on a definition">, >>> > InGroup<IgnoredAttributes>; >>> > >>> > Modified: cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp (original) >>> > +++ cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp Mon May 20 >>> 19:06:11 2013 >>> > @@ -1900,7 +1900,13 @@ CodeGenModule::GetLLVMLinkageVarDefiniti >>> > return llvm::Function::DLLImportLinkage; >>> > else if (D->hasAttr<DLLExportAttr>()) >>> > return llvm::Function::DLLExportLinkage; >>> > - else if (D->hasAttr<WeakAttr>()) { >>> > + else if (D->hasAttr<SelectAnyAttr>()) { >>> > + // selectany symbols are externally visible, so use weak instead >>> of >>> > + // linkonce. MSVC optimizes away references to const selectany >>> globals, so >>> > + // all definitions should be the same and ODR linkage should be >>> used. >>> > + // http://msdn.microsoft.com/en-us/library/5tkz6s71.aspx >>> > + return llvm::GlobalVariable::WeakODRLinkage; >>> > + } else if (D->hasAttr<WeakAttr>()) { >>> > if (GV->isConstant()) >>> > return llvm::GlobalVariable::WeakODRLinkage; >>> > else >>> > >>> > Modified: cfe/branches/release_33/lib/Sema/SemaDecl.cpp >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDecl.cpp?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/lib/Sema/SemaDecl.cpp (original) >>> > +++ cfe/branches/release_33/lib/Sema/SemaDecl.cpp Mon May 20 19:06:11 >>> 2013 >>> > @@ -4638,6 +4638,15 @@ static void checkAttributesAfterMerging( >>> > ND.dropAttr<WeakRefAttr>(); >>> > } >>> > } >>> > + >>> > + // 'selectany' only applies to externally visible varable >>> declarations. >>> > + // It does not apply to functions. >>> > + if (SelectAnyAttr *Attr = ND.getAttr<SelectAnyAttr>()) { >>> > + if (isa<FunctionDecl>(ND) || !ND.isExternallyVisible()) { >>> > + S.Diag(Attr->getLocation(), >>> diag::err_attribute_selectany_non_extern_data); >>> > + ND.dropAttr<SelectAnyAttr>(); >>> > + } >>> > + } >>> > } >>> > >>> > /// Given that we are within the definition of the given function, >>> > >>> > Modified: cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp (original) >>> > +++ cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp Mon May 20 >>> 19:06:11 2013 >>> > @@ -4683,6 +4683,16 @@ static void handleForceInlineAttr(Sema & >>> > S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << >>> Attr.getName(); >>> > } >>> > >>> > +static void handleSelectAnyAttr(Sema &S, Decl *D, const AttributeList >>> &Attr) { >>> > + if (!checkMicrosoftExt(S, Attr)) >>> > + return; >>> > + // Check linkage after possibly merging declaratinos. See >>> > + // checkAttributesAfterMerging(). >>> > + D->addAttr(::new (S.Context) >>> > + SelectAnyAttr(Attr.getRange(), S.Context, >>> > + Attr.getAttributeSpellingListIndex())); >>> > +} >>> > + >>> > >>> >>> //===----------------------------------------------------------------------===// >>> > // Top Level Sema Entry Points >>> > >>> >>> //===----------------------------------------------------------------------===// >>> > @@ -4909,6 +4919,9 @@ static void ProcessInheritableDeclAttr(S >>> > case AttributeList::AT_ForceInline: >>> > handleForceInlineAttr(S, D, Attr); >>> > break; >>> > + case AttributeList::AT_SelectAny: >>> > + handleSelectAnyAttr(S, D, Attr); >>> > + break; >>> > >>> > // Thread safety attributes: >>> > case AttributeList::AT_GuardedVar: >>> > >>> > Modified: cfe/branches/release_33/test/CodeGen/ms-declspecs.c >>> > URL: >>> http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/test/CodeGen/ms-declspecs.c?rev=182337&r1=182336&r2=182337&view=diff >>> > >>> ============================================================================== >>> > --- cfe/branches/release_33/test/CodeGen/ms-declspecs.c (original) >>> > +++ cfe/branches/release_33/test/CodeGen/ms-declspecs.c Mon May 20 >>> 19:06:11 2013 >>> > @@ -1,5 +1,10 @@ >>> > // RUN: %clang_cc1 -triple i386-pc-win32 %s -emit-llvm >>> -fms-compatibility -o - | FileCheck %s >>> > >>> > +__declspec(selectany) int x1 = 1; >>> > +const __declspec(selectany) int x2 = 2; >>> > +// CHECK: @x1 = weak_odr global i32 1, align 4 >>> > +// CHECK: @x2 = weak_odr constant i32 2, align 4 >>> > + >>> > struct __declspec(align(16)) S { >>> > char x; >>> > }; >>> > >>> > >>> > _______________________________________________ >>> > llvm-branch-commits mailing list >>> > [email protected] >>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-branch-commits >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
