[PATCH] Implement __declspec(selectany) under -fms-extensions

[Erik Schwiebert]

My understanding of declspec(selectany) is that only one item needs to be kept in the final linked object; all others may be discarded.  The behavior is DEFINITLY different from "weak" linkage where all are kept and the loader is responsible for selecting the one that wins.

MacBU is very happy to see selectany support being added!

Schwieb

-----Original Message-----
From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Rafael Espíndola
Sent: Friday, May 17, 2013 10:38 AM
To: reviews+D814+public+6d3fefb9eec8de81 at llvm-reviews.chandlerc.com
Cc: llvm cfe
Subject: Re: [PATCH] Implement __declspec(selectany) under -fms-extensions

Can they be discarded? If so, please add a comment saying that they
can. If not, you need no use weak_odr.

On 17 May 2013 13:22, Reid Kleckner <rnk at google.com> wrote:
> selectany only applies to externally visible global variables.  It has
> the effect of making the data linkonce.  MSDN doesn't seem to require
> that all definitions be the same, which means clang shouldn't use
> linkonceodr.
>
> http://llvm-reviews.chandlerc.com/D814
>
> Files:
>   include/clang/Basic/Attr.td
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/CodeGen/CodeGenModule.cpp
>   lib/Sema/SemaDecl.cpp
>   lib/Sema/SemaDeclAttr.cpp
>   test/CodeGen/ms-declspecs.c
>   test/SemaCXX/attr-selectany.cpp
>
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -948,6 +948,10 @@
>    let Spellings = [Keyword<"__forceinline">];
>  }
>
> +def SelectAny : InheritableAttr {
> +  let Spellings = [Declspec<"selectany">];
> +}
> +
>  def Win64 : InheritableAttr {
>    let Spellings = [Keyword<"__w64">];
>  }
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1950,6 +1950,8 @@
>    "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>;
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -1937,7 +1937,12 @@
>      return llvm::Function::DLLImportLinkage;
>    else if (D->hasAttr<DLLExportAttr>())
>      return llvm::Function::DLLExportLinkage;
> -  else if (D->hasAttr<WeakAttr>()) {
> +  else if (D->hasAttr<SelectAnyAttr>()) {
> +    // MSDN does not say that selectany globals all have to have the same
> +    // definition, so we use linkonce instead of linkonceodr here to be
> +    // conservative.
> +    return llvm::GlobalVariable::LinkOnceAnyLinkage;
> +  } else if (D->hasAttr<WeakAttr>()) {
>      if (GV->isConstant())
>        return llvm::GlobalVariable::WeakODRLinkage;
>      else
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -4636,6 +4636,15 @@
>        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,
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -4690,6 +4690,16 @@
>                               Attr.getAttributeSpellingListIndex()));
>  }
>
> +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
>  //===----------------------------------------------------------------------===//
> @@ -4916,6 +4926,9 @@
>    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:
> Index: test/CodeGen/ms-declspecs.c
> ===================================================================
> --- test/CodeGen/ms-declspecs.c
> +++ test/CodeGen/ms-declspecs.c
> @@ -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 = linkonce global i32 1, align 4
> +// CHECK: @x2 = linkonce constant i32 2, align 4
> +
>  struct __declspec(align(16)) S {
>    char x;
>  };
> Index: test/SemaCXX/attr-selectany.cpp
> ===================================================================
> --- /dev/null
> +++ test/SemaCXX/attr-selectany.cpp
> @@ -0,0 +1,26 @@
> +// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
> +// MSVC produces similar diagnostics.
> +
> +__declspec(selectany) void foo() { } // expected-error{{'selectany' can only be applied to data items with external linkage}}
> +
> +__declspec(selectany) int x1 = 1;
> +
> +const __declspec(selectany) int x2 = 2; // expected-error{{'selectany' can only be applied to data items with external linkage}}
> +
> +extern const __declspec(selectany) int x3 = 3;
> +
> +extern const int x4;
> +const __declspec(selectany) int x4 = 4;
> +
> +// MSDN says this is incorrect, but MSVC doesn't diagnose it.
> +extern __declspec(selectany) int x5;
> +
> +static __declspec(selectany) int x6 = 2; // expected-error{{'selectany' can only be applied to data items with external linkage}}
> +
> +class X {
> + public:
> +  X(int i) { i++; };
> +  int i;
> +};
> +
> +__declspec(selectany) X x(1);
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits