<div dir="ltr">In the absence of the code owner's approval, I had Doug approve it. The person responsible for the patch said that it was low risk. And we haven't yet gone into phase II of testing. The only problem here was that dependent patches weren't listed as being needed.<div>
<br></div><div>-bw</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 21, 2013 at 11:44 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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?<div class="HOEnZb">
<div class="h5"><br>
<br><div class="gmail_quote">On Tue, May 21, 2013 at 11:09 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>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.<br>
</div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 21, 2013 at 1:39 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Why was this change merged to the branch at all?<br><br><div class="gmail_quote"><div>On Tue, May 21, 2013 at 5:00 AM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>Bill and Reid, it depends on r181677 and r182096. release_33 has been broken.<br>
<br>
SemaDeclAttr.cpp: In function ‘void handleSelectAnyAttr(clang::Sema&,<br>
clang::Decl*, const clang::AttributeList&)’:<br>
SemaDeclAttr.cpp:4687:33: error: ‘checkMicrosoftExt’ was not declared<br>
in this scope<br>
<br>
SemaDecl.cpp: In function ‘void<br>
checkAttributesAfterMerging(clang::Sema&, clang::NamedDecl&)’:<br>
SemaDecl.cpp:4645:38: error: ‘class clang::NamedDecl’ has no member<br>
named ‘isExternallyVisible’<br>
<br>
...Takumi<br>
<br>
<br>
2013/5/21 Bill Wendling <<a href="mailto:isanbard@gmail.com" target="_blank">isanbard@gmail.com</a>>:<br>
> Author: void<br>
> Date: Mon May 20 19:06:11 2013<br>
> New Revision: 182337<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=182337&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=182337&view=rev</a><br>
> Log:<br>
> Merging r182266:<br>
> ------------------------------------------------------------------------<br>
> r182266 | rnk | 2013-05-20 07:02:37 -0700 (Mon, 20 May 2013) | 13 lines<br>
><br>
> Implement __declspec(selectany) under -fms-extensions<br>
><br>
> selectany only applies to externally visible global variables. It has<br>
> the effect of making the data weak_odr.<br>
><br>
> The MSDN docs suggest that unused definitions can only be dropped at<br>
> linktime, so Clang uses weak instead of linkonce. MSVC optimizes away<br>
> references to constant selectany data, so it must assume that there is<br>
> only one definition, hence weak_odr.<br>
><br>
> Reviewers: espindola<br>
><br>
> Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D814" target="_blank">http://llvm-reviews.chandlerc.com/D814</a><br>
> ------------------------------------------------------------------------<br>
><br>
> Added:<br>
> cfe/branches/release_33/test/SemaCXX/attr-selectany.cpp<br>
> - copied unchanged from r182266, cfe/trunk/test/SemaCXX/attr-selectany.cpp<br>
> Modified:<br>
> cfe/branches/release_33/ (props changed)<br>
> cfe/branches/release_33/include/clang/Basic/Attr.td<br>
> cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td<br>
> cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp<br>
> cfe/branches/release_33/lib/Sema/SemaDecl.cpp<br>
> cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp<br>
> cfe/branches/release_33/test/CodeGen/ms-declspecs.c<br>
><br>
> Propchange: cfe/branches/release_33/<br>
> ------------------------------------------------------------------------------<br>
> --- svn:mergeinfo (original)<br>
> +++ svn:mergeinfo Mon May 20 19:06:11 2013<br>
> @@ -1,4 +1,4 @@<br>
> /cfe/branches/type-system-rewrite:134693-134817<br>
> -/cfe/trunk:181286,181299,181368,181465,181728,181750,181909,182072<br>
> +/cfe/trunk:181286,181299,181368,181465,181728,181750,181909,182072,182266<br>
> /cfe/trunk/test:170344<br>
> /cfe/trunk/test/SemaTemplate:126920<br>
><br>
> Modified: cfe/branches/release_33/include/clang/Basic/Attr.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/Attr.td?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/Attr.td?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/include/clang/Basic/Attr.td (original)<br>
> +++ cfe/branches/release_33/include/clang/Basic/Attr.td Mon May 20 19:06:11 2013<br>
> @@ -948,6 +948,10 @@ def ForceInline : InheritableAttr {<br>
> let Spellings = [Keyword<"__forceinline">];<br>
> }<br>
><br>
> +def SelectAny : InheritableAttr {<br>
> + let Spellings = [Declspec<"selectany">];<br>
> +}<br>
> +<br>
> def Win64 : InheritableAttr {<br>
> let Spellings = [Keyword<"__w64">];<br>
> }<br>
><br>
> Modified: cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/branches/release_33/include/clang/Basic/DiagnosticSemaKinds.td Mon May 20 19:06:11 2013<br>
> @@ -1948,6 +1948,8 @@ def warn_weak_identifier_undeclared : Wa<br>
> "weak identifier %0 never declared">;<br>
> def err_attribute_weak_static : Error<<br>
> "weak declaration cannot have internal linkage">;<br>
> +def err_attribute_selectany_non_extern_data : Error<<br>
> + "'selectany' can only be applied to data items with external linkage">;<br>
> def warn_attribute_weak_import_invalid_on_definition : Warning<<br>
> "'weak_import' attribute cannot be specified on a definition">,<br>
> InGroup<IgnoredAttributes>;<br>
><br>
> Modified: cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp (original)<br>
> +++ cfe/branches/release_33/lib/CodeGen/CodeGenModule.cpp Mon May 20 19:06:11 2013<br>
> @@ -1900,7 +1900,13 @@ CodeGenModule::GetLLVMLinkageVarDefiniti<br>
> return llvm::Function::DLLImportLinkage;<br>
> else if (D->hasAttr<DLLExportAttr>())<br>
> return llvm::Function::DLLExportLinkage;<br>
> - else if (D->hasAttr<WeakAttr>()) {<br>
> + else if (D->hasAttr<SelectAnyAttr>()) {<br>
> + // selectany symbols are externally visible, so use weak instead of<br>
> + // linkonce. MSVC optimizes away references to const selectany globals, so<br>
> + // all definitions should be the same and ODR linkage should be used.<br>
> + // <a href="http://msdn.microsoft.com/en-us/library/5tkz6s71.aspx" target="_blank">http://msdn.microsoft.com/en-us/library/5tkz6s71.aspx</a><br>
> + return llvm::GlobalVariable::WeakODRLinkage;<br>
> + } else if (D->hasAttr<WeakAttr>()) {<br>
> if (GV->isConstant())<br>
> return llvm::GlobalVariable::WeakODRLinkage;<br>
> else<br>
><br>
> Modified: cfe/branches/release_33/lib/Sema/SemaDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDecl.cpp?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDecl.cpp?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/lib/Sema/SemaDecl.cpp (original)<br>
> +++ cfe/branches/release_33/lib/Sema/SemaDecl.cpp Mon May 20 19:06:11 2013<br>
> @@ -4638,6 +4638,15 @@ static void checkAttributesAfterMerging(<br>
> ND.dropAttr<WeakRefAttr>();<br>
> }<br>
> }<br>
> +<br>
> + // 'selectany' only applies to externally visible varable declarations.<br>
> + // It does not apply to functions.<br>
> + if (SelectAnyAttr *Attr = ND.getAttr<SelectAnyAttr>()) {<br>
> + if (isa<FunctionDecl>(ND) || !ND.isExternallyVisible()) {<br>
> + S.Diag(Attr->getLocation(), diag::err_attribute_selectany_non_extern_data);<br>
> + ND.dropAttr<SelectAnyAttr>();<br>
> + }<br>
> + }<br>
> }<br>
><br>
> /// Given that we are within the definition of the given function,<br>
><br>
> Modified: cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp (original)<br>
> +++ cfe/branches/release_33/lib/Sema/SemaDeclAttr.cpp Mon May 20 19:06:11 2013<br>
> @@ -4683,6 +4683,16 @@ static void handleForceInlineAttr(Sema &<br>
> S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();<br>
> }<br>
><br>
> +static void handleSelectAnyAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
> + if (!checkMicrosoftExt(S, Attr))<br>
> + return;<br>
> + // Check linkage after possibly merging declaratinos. See<br>
> + // checkAttributesAfterMerging().<br>
> + D->addAttr(::new (S.Context)<br>
> + SelectAnyAttr(Attr.getRange(), S.Context,<br>
> + Attr.getAttributeSpellingListIndex()));<br>
> +}<br>
> +<br>
> //===----------------------------------------------------------------------===//<br>
> // Top Level Sema Entry Points<br>
> //===----------------------------------------------------------------------===//<br>
> @@ -4909,6 +4919,9 @@ static void ProcessInheritableDeclAttr(S<br>
> case AttributeList::AT_ForceInline:<br>
> handleForceInlineAttr(S, D, Attr);<br>
> break;<br>
> + case AttributeList::AT_SelectAny:<br>
> + handleSelectAnyAttr(S, D, Attr);<br>
> + break;<br>
><br>
> // Thread safety attributes:<br>
> case AttributeList::AT_GuardedVar:<br>
><br>
> Modified: cfe/branches/release_33/test/CodeGen/ms-declspecs.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/test/CodeGen/ms-declspecs.c?rev=182337&r1=182336&r2=182337&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/branches/release_33/test/CodeGen/ms-declspecs.c?rev=182337&r1=182336&r2=182337&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/branches/release_33/test/CodeGen/ms-declspecs.c (original)<br>
> +++ cfe/branches/release_33/test/CodeGen/ms-declspecs.c Mon May 20 19:06:11 2013<br>
> @@ -1,5 +1,10 @@<br>
> // RUN: %clang_cc1 -triple i386-pc-win32 %s -emit-llvm -fms-compatibility -o - | FileCheck %s<br>
><br>
> +__declspec(selectany) int x1 = 1;<br>
> +const __declspec(selectany) int x2 = 2;<br>
> +// CHECK: @x1 = weak_odr global i32 1, align 4<br>
> +// CHECK: @x2 = weak_odr constant i32 2, align 4<br>
> +<br>
> struct __declspec(align(16)) S {<br>
> char x;<br>
> };<br>
><br>
><br>
> _______________________________________________<br>
> llvm-branch-commits mailing list<br>
> <a href="mailto:llvm-branch-commits@cs.uiuc.edu" target="_blank">llvm-branch-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-branch-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-branch-commits</a><br>
<br></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br>
</div></div></blockquote></div><br></div>