[llvm-branch-commits] [cfe-branch] r182337 - Merging r182266:

Richard Smith richard at metafoo.co.uk
Tue May 21 11:44:34 PDT 2013


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 <rnk at google.com> 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 <richard at metafoo.co.uk>wrote:
>
>> Why was this change merged to the branch at all?
>>
>> On Tue, May 21, 2013 at 5:00 AM, NAKAMURA Takumi <geek4civic at gmail.com>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 <isanbard at gmail.com>:
>>> > 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
>>> > llvm-branch-commits at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-branch-commits
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-branch-commits/attachments/20130521/25901ebb/attachment.html>


More information about the llvm-branch-commits mailing list