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

Bill Wendling isanbard at gmail.com
Tue May 21 13:29:51 PDT 2013


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.

-bw



On Tue, May 21, 2013 at 11:44 AM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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/cfe-commits/attachments/20130521/5db2cac5/attachment.html>


More information about the cfe-commits mailing list