Use comdats to avoid double initialization of weak data

[rnk]

Summary

Initializers of global data that can appear multiple TUs (static data
members of class templates or __declspec(selectany) data) are now in a
comdat group keyed on the global variable being initialized. On
non-Windows platforms, this is a code size and startup time
optimization. On Windows, this is necessary for ABI compatibility with
MSVC.

Fixes PR16959.

[rsmith]

LGTM

[rnk]

Closed by commit rL209555 (authored by @rnk).

lib/CodeGen/CGDeclCXX.cpp

SmallString<256> FnName;
     llvm::raw_svector_ostream Out(FnName);
     getCXXABI().getMangleContext().mangleDynamicInitializer(D, Out);
   }
 
   // Create a variable initialization function.
   llvm::Function *Fn =
       CreateGlobalInitOrDestructFunction(*this, FTy, FnName.str());
 
-  CodeGenFunction(*this).GenerateCXXGlobalVarDeclInitFunc(Fn, D, Addr,
-                                                          PerformInit);
+  CodeGenFunction(*this)
+      .GenerateCXXGlobalVarDeclInitFunc(Fn, D, Addr, PerformInit);
 
   if (D->getTLSKind()) {
     // FIXME: Should we support init_priority for thread_local?
     // FIXME: Ideally, initialization of instantiated thread_local static data
     // members of class templates should not trigger initialization of other
     // entities in the TU.
     // FIXME: We only need to register one __cxa_thread_atexit function for the
     // entire TU.
     CXXThreadLocalInits.push_back(Fn);
   } else if (const InitPriorityAttr *IPA = D->getAttr<InitPriorityAttr>()) {
     OrderGlobalInits Key(IPA->getPriority(), PrioritizedCXXGlobalInits.size());
     PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
     DelayedCXXInitPosition.erase(D);
   } else if (D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization &&
              D->getTemplateSpecializationKind() != TSK_Undeclared) {
     // C++ [basic.start.init]p2:
     //   Definitions of explicitly specialized class template static data
     //   members have ordered initialization. Other class template static data
     //   members (i.e., implicitly or explicitly instantiated specializations)
     //   have unordered initialization.
     //
     // As a consequence, we can put them into their own llvm.global_ctors entry.
-    // This should allow GlobalOpt to fire more often, and allow us to implement
-    // the Microsoft C++ ABI, which uses COMDAT elimination to avoid double
-    // initializaiton.
-    AddGlobalCtor(Fn);
+    //
+    // In addition, put the initializer into a COMDAT group with the global
+    // being initialized.  On most platforms, this is a minor startup time
+    // optimization.  In the MS C++ ABI, there are no guard variables, so this
+    // COMDAT key is required for correctness.
+    AddGlobalCtor(Fn, 65535, Addr);
     DelayedCXXInitPosition.erase(D);
   } else {
     llvm::DenseMap<const Decl *, unsigned>::iterator I =
       DelayedCXXInitPosition.find(D);
     if (I == DelayedCXXInitPosition.end()) {
       CXXGlobalInits.push_back(Fn);
     } else {
       assert(CXXGlobalInits[I->second] == 0);
void CodeGenFunction::GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
   StartFunction(GlobalDecl(D), getContext().VoidTy, Fn,
                 getTypes().arrangeNullaryFunction(),
                 FunctionArgList(), D->getLocation(),
                 D->getInit()->getExprLoc());
 
   // Use guarded initialization if the global variable is weak. This
   // occurs for, e.g., instantiated static data members and
   // definitions explicitly marked weak.
-  if (llvm::GlobalVariable::isWeakLinkage(Addr->getLinkage()) ||
-      llvm::GlobalVariable::isLinkOnceLinkage(Addr->getLinkage())) {
+  if (Addr->hasWeakLinkage() || Addr->hasLinkOnceLinkage()) {
     EmitCXXGuardedInit(*D, Addr, PerformInit);
   } else {
     EmitCXXGlobalVarDeclInit(*D, Addr, PerformInit);
   }
 
   FinishFunction();
 }
 

lib/CodeGen/CodeGenModule.h

 };
 
 /// This class organizes the cross-function state that is used while generating
 /// LLVM code.
 class CodeGenModule : public CodeGenTypeCache {
   CodeGenModule(const CodeGenModule &) LLVM_DELETED_FUNCTION;
   void operator=(const CodeGenModule &) LLVM_DELETED_FUNCTION;
 
-  typedef std::vector<std::pair<llvm::Constant*, int> > CtorList;
+  struct Structor {
+    Structor() : Priority(0), Initializer(nullptr), AssociatedData(nullptr) {}
+    Structor(int Priority, llvm::Constant *Initializer,
+             llvm::Constant *AssociatedData)
+        : Priority(Priority), Initializer(Initializer),
+          AssociatedData(AssociatedData) {}
+    int Priority;
+    llvm::Constant *Initializer;
+    llvm::Constant *AssociatedData;
+  };
+
+  typedef std::vector<Structor> CtorList;
 
   ASTContext &Context;
   const LangOptions &LangOpts;
   const CodeGenOptions &CodeGenOpts;
   llvm::Module &TheModule;
   DiagnosticsEngine &Diags;
   const llvm::DataLayout &TheDataLayout;
   const TargetInfo &Target;
private:
 
   /// Emit the function that initializes the specified global (if PerformInit is
   /// true) and registers its destructor.
   void EmitCXXGlobalVarDeclInitFunc(const VarDecl *D,
                                     llvm::GlobalVariable *Addr,
                                     bool PerformInit);
 
   // FIXME: Hardcoding priority here is gross.
-  void AddGlobalCtor(llvm::Function *Ctor, int Priority=65535);
-  void AddGlobalDtor(llvm::Function *Dtor, int Priority=65535);
+  void AddGlobalCtor(llvm::Function *Ctor, int Priority = 65535,
+                     llvm::Constant *AssociatedData = 0);
+  void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535);
 
   /// Generates a global array of functions and priorities using the given list
   /// and name. This array will have appending linkage and is suitable for use
   /// as a LLVM constructor or destructor array.
   void EmitCtorList(const CtorList &Fns, const char *GlobalName);
 
   /// Emit the RTTI descriptors for the given type.
   void EmitFundamentalRTTIDescriptor(QualType Type);

lib/CodeGen/CodeGenModule.cpp

 }
 
 llvm::GlobalValue *CodeGenModule::GetGlobalValue(StringRef Name) {
   return getModule().getNamedValue(Name);
 }
 
 /// AddGlobalCtor - Add a function to the list that will be called before
 /// main() runs.
-void CodeGenModule::AddGlobalCtor(llvm::Function * Ctor, int Priority) {
+void CodeGenModule::AddGlobalCtor(llvm::Function *Ctor, int Priority,
+                                  llvm::Constant *AssociatedData) {
   // FIXME: Type coercion of void()* types.
-  GlobalCtors.push_back(std::make_pair(Ctor, Priority));
+  GlobalCtors.push_back(Structor(Priority, Ctor, AssociatedData));
 }
 
 /// AddGlobalDtor - Add a function to the list that will be called
 /// when the module is unloaded.
-void CodeGenModule::AddGlobalDtor(llvm::Function * Dtor, int Priority) {
+void CodeGenModule::AddGlobalDtor(llvm::Function *Dtor, int Priority) {
   // FIXME: Type coercion of void()* types.
-  GlobalDtors.push_back(std::make_pair(Dtor, Priority));
+  GlobalDtors.push_back(Structor(Priority, Dtor, 0));
 }
 
 void CodeGenModule::EmitCtorList(const CtorList &Fns, const char *GlobalName) {
   // Ctor function type is void()*.
   llvm::FunctionType* CtorFTy = llvm::FunctionType::get(VoidTy, false);
   llvm::Type *CtorPFTy = llvm::PointerType::getUnqual(CtorFTy);
 
-  // Get the type of a ctor entry, { i32, void ()* }.
-  llvm::StructType *CtorStructTy =
-    llvm::StructType::get(Int32Ty, llvm::PointerType::getUnqual(CtorFTy), NULL);
+  // Get the type of a ctor entry, { i32, void ()*, i8* }.
+  llvm::StructType *CtorStructTy = llvm::StructType::get(
+      Int32Ty, llvm::PointerType::getUnqual(CtorFTy), VoidPtrTy, NULL);
 
   // Construct the constructor and destructor arrays.
   SmallVector<llvm::Constant*, 8> Ctors;
   for (CtorList::const_iterator I = Fns.begin(), E = Fns.end(); I != E; ++I) {
     llvm::Constant *S[] = {
-      llvm::ConstantInt::get(Int32Ty, I->second, false),
-      llvm::ConstantExpr::getBitCast(I->first, CtorPFTy)
+      llvm::ConstantInt::get(Int32Ty, I->Priority, false),
+      llvm::ConstantExpr::getBitCast(I->Initializer, CtorPFTy),
+      (I->AssociatedData
+           ? llvm::ConstantExpr::getBitCast(I->AssociatedData, VoidPtrTy)
+           : llvm::Constant::getNullValue(VoidPtrTy))
     };
     Ctors.push_back(llvm::ConstantStruct::get(CtorStructTy, S));
   }
 
   if (!Ctors.empty()) {
     llvm::ArrayType *AT = llvm::ArrayType::get(CtorStructTy, Ctors.size());
     new llvm::GlobalVariable(TheModule, AT, false,
                              llvm::GlobalValue::AppendingLinkage,

lib/CodeGen/MicrosoftCXXABI.cpp

llvm::Value* MicrosoftCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
   // over the cookie completely.
   return CGF.Builder.CreateConstInBoundsGEP1_64(newPtr,
                                                 cookieSize.getQuantity());
 }
 
 void MicrosoftCXXABI::EmitGuardedInit(CodeGenFunction &CGF, const VarDecl &D,
                                       llvm::GlobalVariable *GV,
                                       bool PerformInit) {
+  // MSVC only uses guards for static locals.
+  if (!D.isStaticLocal()) {
+    assert(GV->hasWeakLinkage() || GV->hasLinkOnceLinkage());
+    // GlobalOpt is allowed to discard the initializer, so use linkonce_odr.
+    CGF.CurFn->setLinkage(llvm::GlobalValue::LinkOnceODRLinkage);
+    CGF.EmitCXXGlobalVarDeclInit(D, GV, PerformInit);
+    return;
+  }
+
   // MSVC always uses an i32 bitfield to guard initialization, which is *not*
   // threadsafe.  Since the user may be linking in inline functions compiled by
   // cl.exe, there's no reason to provide a false sense of security by using
   // critical sections here.
 
   if (D.getTLSKind())
     CGM.ErrorUnsupported(&D, "dynamic TLS initialization");
 
   CGBuilderTy &Builder = CGF.Builder;
   llvm::IntegerType *GuardTy = CGF.Int32Ty;
   llvm::ConstantInt *Zero = llvm::ConstantInt::get(GuardTy, 0);
 
   // Get the guard variable for this function if we have one already.
-  GuardInfo EmptyGuardInfo;
-  GuardInfo *GI = &EmptyGuardInfo;
-  if (isa<FunctionDecl>(D.getDeclContext())) {
-    GI = &GuardVariableMap[D.getDeclContext()];
-  }
+  GuardInfo *GI = &GuardVariableMap[D.getDeclContext()];
 
   unsigned BitIndex;
   if (D.isStaticLocal() && D.isExternallyVisible()) {
     // Externally visible variables have to be numbered in Sema to properly
     // handle unreachable VarDecls.
     BitIndex = getContext().getStaticLocalNumber(&D);
     assert(BitIndex > 0);
     BitIndex--;

test/CodeGenCXX/apple-kext.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fno-use-cxa-atexit -fapple-kext -emit-llvm -o - %s | FileCheck %s
 
 // CHECK: @_ZN5test01aE = global [[A:%.*]] zeroinitializer
-// CHECK: @llvm.global_ctors = appending global {{.*}} { i32 65535, void ()* [[CTOR0:@.*]] }
-// CHECK: @llvm.global_dtors = appending global {{.*}} { i32 65535, void ()* [[DTOR0:@.*]] }
+// CHECK: @llvm.global_ctors = appending global {{.*}} { i32 65535, void ()* [[CTOR0:@.*]], i8* null }
+// CHECK: @llvm.global_dtors = appending global {{.*}} { i32 65535, void ()* [[DTOR0:@.*]], i8* null }
 
 // rdar://11241230
 namespace test0 {
   struct A { A(); ~A(); };
   A a;
 }
 // CHECK:    define internal void [[CTOR0_:@.*]]()
 // CHECK:      call void @_ZN5test01AC1Ev([[A]]* @_ZN5test01aE)

test/CodeGenCXX/init-priority-attr.cpp

 public:
   static A a;
   C() { foo(4); }
 };
 
 
 A C::a = A();
 
-// CHECK: @llvm.global_ctors = appending global [3 x { i32, void ()* }] [{ i32, void ()* } { i32 200, void ()* @_GLOBAL__I_000200 }, { i32, void ()* } { i32 300, void ()* @_GLOBAL__I_000300 }, { i32, void ()* } { i32 65535, void ()* @_GLOBAL__sub_I_init_priority_attr.cpp }]
+// CHECK: @llvm.global_ctors = appending global [3 x { i32, void ()*, i8* }]
+// CHECK: [{ i32, void ()*, i8* } { i32 200, void ()* @_GLOBAL__I_000200, i8* null },
+// CHECK:  { i32, void ()*, i8* } { i32 300, void ()* @_GLOBAL__I_000300, i8* null },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_init_priority_attr.cpp, i8* null }]
 
 // CHECK: _GLOBAL__I_000200()
 // CHECK: _Z3fooi(i32 3)
 // CHECK-NEXT: ret void
 
 // CHECK: _GLOBAL__I_000300()
 // CHECK: _Z3fooi(i32 2)
 // CHECK-NEXT: _Z3fooi(i32 1)

test/CodeGenCXX/microsoft-abi-static-initializers.cpp

 // RUN: %clang_cc1 -fms-extensions -emit-llvm %s -o - -mconstructor-aliases -triple=i386-pc-win32 | FileCheck %s
 
-// CHECK: @llvm.global_ctors = appending global [2 x { i32, void ()* }]
-// CHECK: [{ i32, void ()* } { i32 65535, void ()* @"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ"
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @_GLOBAL__sub_I_microsoft_abi_static_initializers.cpp }]
+// CHECK: @llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }]
+// CHECK: [{ i32, void ()*, i8* } { i32 65535, void ()* @"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ",
+// CHECK:       i8* bitcast (%class.A* @"\01?foo@?$B@H@@2VA@@A" to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_microsoft_abi_static_initializers.cpp, i8* null }]
 
 struct S {
   S();
   ~S();
 };
 
 S s;
 
-// CHECK: define internal void @"\01??__Es@@YAXXZ"() [[NUW:#[0-9]+]]
-// CHECK: %{{[.0-9A-Z_a-z]+}} = call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
+// CHECK: define internal void @"\01??__Es@@YAXXZ"()
+// CHECK: call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
 // CHECK: call i32 @atexit(void ()* @"\01??__Fs@@YAXXZ")
 // CHECK: ret void
 
-// CHECK: define internal void @"\01??__Fs@@YAXXZ"() [[NUW]] {
+// CHECK: define internal void @"\01??__Fs@@YAXXZ"()
 // CHECK: call x86_thiscallcc void @"\01??1S@@QAE@XZ"
 // CHECK: ret void
 
 // These globals should use distinct guard variables, and not different bits of
 // the same global.
 __declspec(selectany) S selectany1;
 __declspec(selectany) S selectany2;
-// CHECK: define internal void @"\01??__Eselectany1@@YAXXZ"() [[NUW:#[0-9]+]]
-// CHECK: load i32* @"\01??_Bselectany1@@3US@@A@5"
-// CHECK: ret void
-// CHECK: define internal void @"\01??__Eselectany2@@YAXXZ"() [[NUW:#[0-9]+]]
-// CHECK: load i32* @"\01??_Bselectany2@@3US@@A@5"
+// CHECK: define linkonce_odr void @"\01??__Eselectany1@@YAXXZ"()
+// CHECK-NOT: @"\01??_Bselectany1
+// CHECK: call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
+// CHECK: ret void
+// CHECK: define linkonce_odr void @"\01??__Eselectany2@@YAXXZ"()
+// CHECK-NOT: @"\01??_Bselectany2
+// CHECK: call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
 // CHECK: ret void
 
 void StaticLocal() {
   static S TheS;
 }
 // CHECK-LABEL: define void @"\01?StaticLocal@@YAXXZ"()
 // CHECK: load i32* @"\01?$S1@?0??StaticLocal@@YAXXZ@4IA"
 // CHECK: store i32 {{.*}}, i32* @"\01?$S1@?0??StaticLocal@@YAXXZ@4IA"

 // CHECK: and i32 {{.*}}, 4
 // CHECK: ret
 
 // Force WeakODRLinkage by using templates
 class A {
  public:
   A() {}
   ~A() {}
+  int a;
 };
 
 template<typename T>
 class B {
  public:
   static A foo;
 };
 

 // CHECK: ret %struct.S* @"\01?TheS@?1??getS@@YAAAUS@@XZ@4U2@A"
 
 void force_usage() {
   UnreachableStatic();
   getS();
   (void)B<int>::foo;  // (void) - force usage
 }
 
-// CHECK: define internal void @"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ"() [[NUW]]
-// CHECK: load i32* @"\01??_Bfoo@?$B@H@@2VA@@A@5"
-// CHECK: store i32 {{.*}}, i32* @"\01??_Bfoo@?$B@H@@2VA@@A@5"
-// CHECK: %{{[.0-9A-Z_a-z]+}} = call x86_thiscallcc %class.A* @"\01??0A@@QAE@XZ"
+// CHECK: define linkonce_odr void @"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ"()
+// CHECK-NOT: and
+// CHECK-NOT: ?_Bfoo@
+// CHECK: call x86_thiscallcc %class.A* @"\01??0A@@QAE@XZ"
 // CHECK: call i32 @atexit(void ()* @"\01??__Ffoo@?$B@H@@2VA@@A@YAXXZ")
 // CHECK: ret void
 
 // CHECK: define linkonce_odr x86_thiscallcc %class.A* @"\01??0A@@QAE@XZ"
 
 // CHECK: define linkonce_odr x86_thiscallcc void @"\01??1A@@QAE@XZ"
 
 // CHECK: define internal void @"\01??__Ffoo@?$B@H@@2VA@@A@YAXXZ"
 // CHECK: call x86_thiscallcc void @"\01??1A@@QAE@XZ"{{.*}}foo
 // CHECK: ret void
 
-// CHECK: define internal void @_GLOBAL__sub_I_microsoft_abi_static_initializers.cpp() [[NUW]] {
+// CHECK: define internal void @_GLOBAL__sub_I_microsoft_abi_static_initializers.cpp()
 // CHECK: call void @"\01??__Es@@YAXXZ"()
 // CHECK: ret void
-
-// CHECK: attributes [[NUW]] = { nounwind }

test/CodeGenCXX/static-member-variable-explicit-specialization.cpp

 // RUN: %clang_cc1 %s -std=c++1y -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
 // CHECK: ; ModuleID
 
 extern "C" int foo();
 
 template<typename T> struct A { static int a; };
 template<typename T> int A<T>::a = foo();
 
 // CHECK-NOT: @_ZN1AIcE1aE
 template<> int A<char>::a;
 
 // CHECK: @_ZN1AIbE1aE = global i32 10
 template<> int A<bool>::a = 10;
 
-// CHECK: @llvm.global_ctors = appending global [7 x { i32, void ()* }]
-// CHECK: [{ i32, void ()* } { i32 65535, void ()* @[[unordered1:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @[[unordered2:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @[[unordered3:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @[[unordered4:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @[[unordered5:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @[[unordered6:[^ ]*]] },
-// CHECK:  { i32, void ()* } { i32 65535, void ()* @_GLOBAL__sub_I_static_member_variable_explicit_specialization.cpp }]
+// CHECK: @llvm.global_ctors = appending global [7 x { i32, void ()*, i8* }]
+// CHECK: [{ i32, void ()*, i8* } { i32 65535, void ()* @[[unordered1:[^,]*]], i8* bitcast (i32* @_ZN1AIsE1aE to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered2:[^,]*]], i8* bitcast (i16* @_Z1xIsE to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered3:[^,]*]], i8* bitcast (i32* @_ZN2ns1aIiE1iE to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered4:[^,]*]], i8* bitcast (i32* @_ZN2ns1b1iIiEE to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered5:[^,]*]], i8* bitcast (i32* @_ZN1AIvE1aE to i8*) },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered6:[^,]*]], i8* @_Z1xIcE },
+// CHECK:  { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_static_member_variable_explicit_specialization.cpp, i8* null }]
 
 template int A<short>::a;  // Unordered
 int b = foo();
 int c = foo();
 int d = A<void>::a; // Unordered
 
 // An explicit specialization is ordered, and goes in __GLOBAL_sub_I_static_member_variable_explicit_specialization.cpp.
 template<> struct A<int> { static int a; };