CodeGen: Fix linkage of reference temporaries

[majnemer]

Summary

A reference temporary should inherit the linkage of the variable it
initializes. Otherwise, we may hit cases where a reference temporary
wouldn't have the same value in all translation units.

[majnemer]

Handle static data members.

[majnemer]

Give proper linkage to things with GVA_TemplateInstantiation.

[rsmith]

This basically looks good to me, but we really need to get the Itanium ABI to cover this.

lib/CodeGen/CGDecl.cpp

133–134

Do you have tests for this change? It seems to imply that dllimport, dllexport, and selectany now work on static local variables and didn't previously.

138–140

Hmm, I wonder why we don't do this for static locals too. Emitting it both eagerly and with a discardable linkage makes very little sense to me...

lib/CodeGen/CodeGenModule.cpp

2873–2883

What should the linkage of the temporary be if the linkage of the variable is strong? Seems we have two choices here:

1. Always the same as that of the variable. This creates more symbols than are strictly necessary, but allows us to emit the variable as available_externally in some circumstances.
2. If the variable is strong, the temporary is private.

The difference only seems to matter if it's possible to provide an available_externally definition (that is, for a static data member with an initializer specified within the class). Since our mangling doesn't collide with anyone else's, this will only matter if/when we start emitting the available_externally definitions.

[majnemer]

Give reference temporaries private linkage if their variable has strong, external linkage.

[majnemer]

lib/CodeGen/CGDecl.cpp

133–134

It is a semantic error for any of those to appear on static local variables. We get this wrong but I believe that's a different bug from this one.

lib/CodeGen/CodeGenModule.cpp

2873–2883

I've updated this differential to implement option #2. This should cause us to use available_externally for the reference temporary if the variable was available_externally while using private if the variable was external.

[rsmith]

LGTM

I expect we should do the same thing for visibility of such temporaries.

[majnemer]

Committed in rL207451

lib/CodeGen/CGDecl.cpp

void CodeGenFunction::EmitDecl(const Decl &D) {
   }
 }
 
 /// EmitVarDecl - This method handles emission of any variable declaration
 /// inside a function, including static vars etc.
 void CodeGenFunction::EmitVarDecl(const VarDecl &D) {
   if (D.isStaticLocal()) {
     llvm::GlobalValue::LinkageTypes Linkage =
-      llvm::GlobalValue::InternalLinkage;
-
-    // If the variable is externally visible, it must have weak linkage so it
-    // can be uniqued.
-    if (D.isExternallyVisible()) {
-      Linkage = llvm::GlobalValue::LinkOnceODRLinkage;
+        CGM.GetLLVMLinkageVarDefinition(&D, /*isConstant=*/false);
 
-      // FIXME: We need to force the emission/use of a guard variable for
-      // some variables even if we can constant-evaluate them because
-      // we can't guarantee every translation unit will constant-evaluate them.
-    }
 
     return EmitStaticVarDecl(D, Linkage);
   }
 
   if (D.hasExternalStorage())
     // Don't emit it now, allow it to be emitted lazily on its first use.
     return;
 
   if (D.getStorageClass() == SC_OpenCLWorkGroupLocal)
     return CGM.getOpenCLRuntime().EmitWorkGroupLocalVarDecl(*this, D);
 
   assert(D.hasLocalStorage());
   return EmitAutoVarDecl(D);
 }
 

lib/CodeGen/CGDeclCXX.cpp

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 (Addr->getLinkage() == llvm::GlobalValue::WeakODRLinkage ||
-      Addr->getLinkage() == llvm::GlobalValue::WeakAnyLinkage) {
+  if (llvm::GlobalVariable::isWeakLinkage(Addr->getLinkage()) ||
+      llvm::GlobalVariable::isLinkOnceLinkage(Addr->getLinkage())) {
     EmitCXXGuardedInit(*D, Addr, PerformInit);
   } else {
     EmitCXXGlobalVarDeclInit(*D, Addr, PerformInit);
   }
 
   FinishFunction();
 }
 

lib/CodeGen/CodeGenModule.cpp

else if (D->hasAttr<SelectAnyAttr>()) {
     // 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 (isConstant)
       return llvm::GlobalVariable::WeakODRLinkage;
     else
       return llvm::GlobalVariable::WeakAnyLinkage;
-  } else if (Linkage == GVA_TemplateInstantiation || Linkage == GVA_StrongODR)
+  } else if (Linkage == GVA_StrongODR)
     return llvm::GlobalVariable::WeakODRLinkage;
+  else if (Linkage == GVA_TemplateInstantiation)
+    return llvm::Function::LinkOnceODRLinkage;
   else if (D->getTLSKind() == VarDecl::TLS_Dynamic &&
            getTarget().getTriple().isMacOSX())
     // On Darwin, the backing variable for a C++11 thread_local variable always
     // has internal linkage; all accesses should just be calls to the
     // Itanium-specified entry point, which has the normal linkage of the
     // variable.
     return llvm::GlobalValue::InternalLinkage;
   else if (getCXXABI().isInlineInitializedStaticDataMemberLinkOnce() &&
            isVarDeclInlineInitializedStaticDataMember(D))
     // If required by the ABI, give definitions of static data members with inline
     // initializers linkonce_odr linkage.
     return llvm::GlobalVariable::LinkOnceODRLinkage;
-  // C++ doesn't have tentative definitions and thus cannot have common linkage.
-  else if (!getLangOpts().CPlusPlus &&
+  else if (D->isStaticLocal()) {
+    // If the variable is externally visible, it must have weak linkage so it
+    // can be uniqued.
+    if (D->isExternallyVisible())
+      return llvm::GlobalValue::LinkOnceODRLinkage;
+    else
+      return llvm::GlobalValue::InternalLinkage;
+  } else if (!getLangOpts().CPlusPlus &&
              !isVarDeclStrongDefinition(D, CodeGenOpts.NoCommon))
+    // C++ doesn't have tentative definitions and thus cannot have common
+    // linkage.
     return llvm::GlobalVariable::CommonLinkage;
 
   return llvm::GlobalVariable::ExternalLinkage;
 }
 
 /// Replace the uses of a function that was declared with a non-proto type.
 /// We want to silently drop extra arguments from call sites
 static void replaceUsesOfNonProtoConstant(llvm::Constant *old,
if (Value) {
     Type = InitialValue->getType();
   } else {
     // No initializer, the initialization will be provided when we
     // initialize the declaration which performed lifetime extension.
     Type = getTypes().ConvertTypeForMem(MaterializedType);
   }
 
   // Create a global variable for this lifetime-extended temporary.
-  llvm::GlobalVariable *GV =
-    new llvm::GlobalVariable(getModule(), Type, Constant,
-                             llvm::GlobalValue::PrivateLinkage,
-                             InitialValue, Name.c_str());
+  llvm::GlobalValue::LinkageTypes Linkage =
+      GetLLVMLinkageVarDefinition(VD, Constant);
+  if (Linkage == llvm::GlobalVariable::ExternalLinkage)
+    Linkage = llvm::GlobalVariable::PrivateLinkage;
+  unsigned AddrSpace = GetGlobalVarAddressSpace(
+      VD, getContext().getTargetAddressSpace(MaterializedType));
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+      getModule(), Type, Constant, Linkage, InitialValue, Name.c_str(),
+      /*InsertBefore=*/nullptr, llvm::GlobalVariable::NotThreadLocal,
+      AddrSpace);
   GV->setAlignment(
       getContext().getTypeAlignInChars(MaterializedType).getQuantity());
   if (VD->getTLSKind())
     setTLSMode(GV, *VD);
   Slot = GV;
   return GV;
 }
 
 /// EmitObjCPropertyImplementations - Emit information for synthesized

test/CXX/temp/temp.spec/temp.explicit/p1-emit.cpp

 // CHECK: @_ZN1XIiE7member1E = weak_odr global i32 0
 template int X<int>::member1;
 
 // CHECK: @_ZN1XIiE7member2E = weak_odr global i32 17
 template int X<int>::member2;
 
 // For implicit instantiation of 
 long& get(bool Cond1, bool Cond2) {
-  // CHECK: @_ZN1XIlE7member1E = weak_odr global i64 0
-  // CHECK: @_ZN1XIlE7member2E = weak_odr global i64 17
+  // CHECK: @_ZN1XIlE7member1E = linkonce_odr global i64 0
+  // CHECK: @_ZN1XIlE7member2E = linkonce_odr global i64 17
   // CHECK: @_ZN1XIlE7member3E = external global i64
   return Cond1? X<long>::member1 
        : Cond2? X<long>::member2
               : X<long>::member3;
 }

test/CodeGenCXX/const-init-cxx11.cpp

namespace UnemittedTemporaryDecl {
   // CHECK: @_ZN22UnemittedTemporaryDecl3refE
 
   // CHECK: @_ZN22UnemittedTemporaryDecl4ref2E = constant i32* @_ZGRN22UnemittedTemporaryDecl3refE
 }
 
 // CHECK: @_ZZN12LocalVarInit3aggEvE1a = internal constant {{.*}} i32 101
 // CHECK: @_ZZN12LocalVarInit4ctorEvE1a = internal constant {{.*}} i32 102
 // CHECK: @_ZZN12LocalVarInit8mutable_EvE1a = private unnamed_addr constant {{.*}} i32 103
+// CHECK: @_ZGRN33ClassTemplateWithStaticDataMember1SIvE1aE = linkonce_odr constant i32 5
+// CHECK: @_ZN33ClassTemplateWithStaticDataMember3useE = constant i32* @_ZGRN33ClassTemplateWithStaticDataMember1SIvE1aE
+// CHECK: @_ZGRZN20InlineStaticConstRef3funEvE1i = linkonce_odr constant i32 10
 
 // Constant initialization tests go before this point,
 // dynamic initialization tests go after.
 
 // We must emit a constant initializer for NonLiteralConstexpr::ntd, but also
 // emit an initializer to register its destructor.
 // CHECK: define {{.*}}cxx_global_var_init{{.*}}
 // CHECK-NOT: NonLiteralConstexpr

 namespace Null {
   decltype(nullptr) null();
   // CHECK: call {{.*}} @_ZN4Null4nullEv(
   int *p = null();
   struct S {};
   // CHECK: call {{.*}} @_ZN4Null4nullEv(
   int S::*q = null();
 }
+
+namespace InlineStaticConstRef {
+  inline const int &fun() {
+    static const int &i = 10;
+    return i;
+    // CHECK: ret i32* @_ZGRZN20InlineStaticConstRef3funEvE1i
+  }
+  const int &use = fun();
+}
+
+namespace ClassTemplateWithStaticDataMember {
+  template <typename T>
+  struct S {
+    static const int &a;
+  };
+  template <typename T>
+  const int &S<T>::a = 5;
+  const int &use = S<void>::a;
+}

test/CodeGenCXX/const-init-cxx1y.cpp

namespace ModifyStaticTemporary {
   // Can't emit all of 'c' as a constant here, so emit the initial value of
   // 'c.temporary', not the value as modified by the partial evaluation within
   // the initialization of 'c.x'.
   A c = { 10, (++c.temporary, b.x) };
   // CHECK: @_ZGRN21ModifyStaticTemporary1cE = private global i32 10
   // CHECK: @_ZN21ModifyStaticTemporary1cE = global {{.*}} zeroinitializer
 }
 
+// CHECK: @_ZGRN28VariableTemplateWithConstRef1iIvEE = linkonce_odr constant i32 5, align 4
+// CHECK: @_ZN28VariableTemplateWithConstRef3useE = constant i32* @_ZGRN28VariableTemplateWithConstRef1iIvEE
+namespace VariableTemplateWithConstRef {
+  template <typename T>
+  const int &i = 5;
+  const int &use = i<void>;
+}
+
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE = linkonce_odr constant i32 1
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE2 = linkonce_odr global {{.*}} { i32* @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE }
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE3 = linkonce_odr constant i32 2
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE4 = linkonce_odr global {{.*}} { i32* @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE3 }
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE5 = linkonce_odr constant i32 3
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE6 = linkonce_odr global {{.*}} { i32* @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE5 }
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE7 = linkonce_odr constant i32 4
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE8 = linkonce_odr global {{.*}} { i32* @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE7 }
+// CHECK: @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE9 = linkonce_odr global {{.*}} { {{.*}} @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE2, {{.*}} @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE4, {{.*}} @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE6, {{.*}} @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE8 }
+// CHECK: @_ZN24VariableTemplateWithPack1pE = global {{.*}} @_ZGRN24VariableTemplateWithPack1sIJLi1ELi2ELi3ELi4EEEE9
+namespace VariableTemplateWithPack {
+  struct A {
+    const int &r;
+  };
+  struct S {
+    A &&a, &&b, &&c, &&d;
+  };
+  template <int... N>
+  S &&s = {A{N}...};
+  S *p = &s<1, 2, 3, 4>;
+}
+
 // CHECK: __cxa_atexit({{.*}} @_ZN1BD1Ev {{.*}} @b
 
 // CHECK: define
 // CHECK-NOT: @_ZGRN21ModifyStaticTemporary1cE
 // CHECK: store {{.*}} @_ZGRN21ModifyStaticTemporary1cE, {{.*}} @_ZN21ModifyStaticTemporary1cE
 // CHECK: add
 // CHECK: store
 // CHECK: load {{.*}} @_ZN21ModifyStaticTemporary1bE
 // CHECK: store {{.*}} @_ZN21ModifyStaticTemporary1cE

test/CodeGenCXX/cxx11-thread-local.cpp

 thread_local int U::m = f();
 
 template<typename T> struct V { static thread_local int m; };
 template<typename T> thread_local int V<T>::m = g();
 
 // CHECK: @e = global i32 0
 int e = V<int>::m;
 
-// CHECK: @_ZN1VIiE1mE = weak_odr thread_local global i32 0
+// CHECK: @_ZN1VIiE1mE = linkonce_odr thread_local global i32 0
 
 // CHECK: @_ZZ1fvE1n = internal thread_local global i32 0
 
 // CHECK: @_ZGVZ1fvE1n = internal thread_local global i8 0
 
 // CHECK: @_ZZ8tls_dtorvE1s = internal thread_local global
 // CHECK: @_ZGVZ8tls_dtorvE1s = internal thread_local global i8 0
 
 // CHECK: @_ZZ8tls_dtorvE1t = internal thread_local global
 // CHECK: @_ZGVZ8tls_dtorvE1t = internal thread_local global i8 0
 
 // CHECK: @_ZZ8tls_dtorvE1u = internal thread_local global
 // CHECK: @_ZGVZ8tls_dtorvE1u = internal thread_local global i8 0
-// CHECK: @_ZGRZ8tls_dtorvE1u = private thread_local global
+// CHECK: @_ZGRZ8tls_dtorvE1u = internal thread_local global
 
-// CHECK: @_ZGVN1VIiE1mE = weak_odr thread_local global i64 0
+// CHECK: @_ZGVN1VIiE1mE = linkonce_odr thread_local global i64 0
 
 // CHECK: @__tls_guard = internal thread_local global i8 0
 
 // CHECK: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
 // CHECK: @_ZTH1a = alias void ()* @__tls_init
 // CHECK: @_ZTHL1d = alias internal void ()* @__tls_init
 // CHECK: @_ZTHN1U1mE = alias void ()* @__tls_init
-// CHECK: @_ZTHN1VIiE1mE = alias weak_odr void ()* @__tls_init
+// CHECK: @_ZTHN1VIiE1mE = alias linkonce_odr void ()* @__tls_init
 
 
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
 // CHECK: call i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 

test/CodeGenCXX/cxx1y-variable-template.cpp

template<typename U> struct Inner {
     template<typename V> static int arr[];
   };
 };
 Outer<char[100]> outer_int;
 int init_arr();
 template<typename T> template<typename U> template<typename V> int Outer<T>::Inner<U>::arr[sizeof(T) + sizeof(U) + sizeof(V)] = { init_arr() };
 int *p = Outer<char[100]>::Inner<char[20]>::arr<char[3]>;
 
-// CHECK: @_ZN5OuterIA100_cE5InnerIA20_cE3arrIA3_cEE = weak_odr global [123 x i32] zeroinitializer
-// CHECK: @_ZGVN5OuterIA100_cE5InnerIA20_cE3arrIA3_cEE = weak_odr global
+// CHECK: @_ZN5OuterIA100_cE5InnerIA20_cE3arrIA3_cEE = linkonce_odr global [123 x i32] zeroinitializer
+// CHECK: @_ZGVN5OuterIA100_cE5InnerIA20_cE3arrIA3_cEE = linkonce_odr global
 
 // CHECK: call {{.*}}@_Z8init_arrv

test/CodeGenCXX/specialized-static-data-mem-init.cpp

 // RUN: %clang_cc1 %s -emit-llvm -triple %itanium_abi_triple -o - | FileCheck %s
 // rdar: // 8562966
 // pr8409
 
-// CHECK: @_ZN1CIiE11needs_guardE = weak_odr global
-// CHECK: @_ZGVN1CIiE11needs_guardE = weak_odr global
+// CHECK: @_ZN1CIiE11needs_guardE = linkonce_odr global
+// CHECK: @_ZGVN1CIiE11needs_guardE = linkonce_odr global
 
 struct K
 {
   K();
   K(const K &);
   ~K();
   void PrintNumK();
 };

test/CodeGenCXX/static-init-3.cpp

struct X1
     static void use(T const &) {}
      static T & get() {
         static X0<T> t;
         use(instance);
         return static_cast<T &>(t);
     }
 };
 
-// CHECK: @_ZN2X1I2X2I1BEE8instanceE = weak_odr global %struct.X2* null, align 8
-// CHECJ: @_ZN2X1I2X2I1AEE8instanceE = weak_odr global %struct.X2* null, align 8
+// CHECK: @_ZN2X1I2X2I1BEE8instanceE = linkonce_odr global %struct.X2* null, align 8
+// CHECJ: @_ZN2X1I2X2I1AEE8instanceE = linkonce_odr global %struct.X2* null, align 8
 template<class T> T & X1<T>::instance = X1<T>::get();
 
 class A { };
 class B : public A { };
 
 template<typename T> struct X2 {};
 X2< B > bg = X1< X2< B > >::get(); 
 X2< A > ag = X1< X2< A > >::get();

test/CodeGenCXX/template-instantiation.cpp

 // RUN: %clang_cc1 %s -O1 -disable-llvm-optzns -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
 
 // CHECK: @_ZN7PR100011xE = global
 // CHECK-NOT: @_ZN7PR100014kBarE = external global i32
 //
 // CHECK-NOT: @_ZTVN5test118stdio_sync_filebufIwEE = constant
 // CHECK-NOT: _ZTVN5test315basic_fstreamXXIcEE
 // CHECK-NOT: @_ZTVN5test018stdio_sync_filebufIA1_iEE
 // CHECK-NOT: @_ZTVN5test018stdio_sync_filebufIA2_iEE
 // CHECK:     @_ZTVN5test018stdio_sync_filebufIA3_iEE = weak_odr unnamed_addr constant
 
-// CHECK: @_ZN7PR100011SIiE3arrE = weak_odr global [3 x i32]
-// CHECK-NOT: @_ZN7PR100011SIiE3arr2E = weak_odr global [3 x i32]A
+// CHECK: @_ZN7PR100011SIiE3arrE = linkonce_odr global [3 x i32]
+// CHECK-NOT: @_ZN7PR100011SIiE3arr2E = linkonce_odr global [3 x i32]A
 
 // CHECK:     @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant
 
 // CHECK-NOT: _ZTVN5test31SIiEE
 // CHECK-NOT: _ZTSN5test31SIiEE
 
 // CHECK-LABEL: define linkonce_odr void @_ZN5test21CIiEC1Ev(%"class.test2::C"* %this) unnamed_addr
 // CHECK-LABEL: define linkonce_odr void @_ZN5test21CIiE6foobarIdEEvT_(

test/CodeGenCXX/vla.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s
 
 template<typename T>
 struct S {
   static int n;
 };
 template<typename T> int S<T>::n = 5;
 
 int f() {
   // Make sure that the reference here is enough to trigger the instantiation of
   // the static data member.
-  // CHECK: @_ZN1SIiE1nE = weak_odr global i32 5
+  // CHECK: @_ZN1SIiE1nE = linkonce_odr global i32 5
   int a[S<int>::n];
   return sizeof a;
 }
 
 // rdar://problem/9506377
 void test0(void *array, int n) {
   // CHECK-LABEL: define void @_Z5test0Pvi(
   // CHECK:      [[ARRAY:%.*]] = alloca i8*, align 8