[GlobalMerge] Revert "Use private linkage for MergedGlobals variables" on Darwin.

[aprantl]

Summary

This is a partial revert of r244615 (http://reviews.llvm.org/D11942), which caused a major regression in debug info quality.

Turning the artificial __MergedGlobal symbols into private symbols (l__MergedGlobal) means that the linker will not include them in the symbol table of the final executable. Without a symbol table entry dsymutil is not be able to process the debug info for any of the merged globals and thus drops the debug info for all of them.

This patch is enabling the old behavior for all MachO targets while leaving all other targets unaffected.

rdar://problem/29160481

[ab]

LGTM.

lib/CodeGen/GlobalMerge.cpp

463–475

Nit: compute the name/linkage upfront, based on IsMachO/HasExternal, and only new GlobalVariable once?

[aprantl]

Thanks!
r286607 + r286610.

lib/CodeGen/GlobalMerge.cpp

bool GlobalMerge::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
 
   ssize_t i = GlobalSet.find_first();
   while (i != -1) {
     ssize_t j = 0;
     uint64_t MergedSize = 0;
     std::vector<Type*> Tys;
     std::vector<Constant*> Inits;
 
+    bool HasExternal = false;
+    GlobalVariable *TheFirstExternal = nullptr;
     for (j = i; j != -1; j = GlobalSet.find_next(j)) {
       Type *Ty = Globals[j]->getValueType();
       MergedSize += DL.getTypeAllocSize(Ty);
       if (MergedSize > MaxOffset) {
         break;
       }
       Tys.push_back(Ty);
       Inits.push_back(Globals[j]->getInitializer());
+
+      if (Globals[j]->hasExternalLinkage() && !HasExternal) {
+        HasExternal = true;
+        TheFirstExternal = Globals[j];
+      }
     }
 
+    // If merged variables doesn't have external linkage, we needn't to expose
+    // the symbol after merging.
+    GlobalValue::LinkageTypes Linkage = HasExternal
+                                            ? GlobalValue::ExternalLinkage
+                                            : GlobalValue::InternalLinkage;
     StructType *MergedTy = StructType::get(M.getContext(), Tys);
     Constant *MergedInit = ConstantStruct::get(MergedTy, Inits);
 
-    GlobalVariable *MergedGV = new GlobalVariable(
+    // On Darwin external linkage needs to be preserved, otherwise dsymutil
+    // cannot preserve the debug info for the merged globals.
+    GlobalVariable *MergedGV;
+    if (IsMachO)
+      // If merged variables have external linkage, we use the symbol name of
+      // the first variable merged as the suffix of global symbol name. This
+      // avoids a link-time naming conflict for _MergedGlobals symbols.
+      MergedGV = new GlobalVariable(
+          M, MergedTy, isConst, Linkage, MergedInit,
+          HasExternal ? "_MergedGlobals_" + TheFirstExternal->getName()
+                      : "_MergedGlobals",
+          nullptr, GlobalVariable::NotThreadLocal, AddrSpace);
+    else
+      MergedGV = new GlobalVariable(
           M, MergedTy, isConst, GlobalValue::PrivateLinkage, MergedInit,
           "_MergedGlobals", nullptr, GlobalVariable::NotThreadLocal, AddrSpace);
 
     const StructLayout *MergedLayout = DL.getStructLayout(MergedTy);
 
     for (ssize_t k = i, idx = 0; k != j; k = GlobalSet.find_next(k), ++idx) {
       GlobalValue::LinkageTypes Linkage = Globals[k]->getLinkage();
       std::string Name = Globals[k]->getName();
 
       // Copy metadata while adjusting any debug info metadata by the original
       // global's offset within the merged global.
       MergedGV->copyMetadata(Globals[k], MergedLayout->getElementOffset(idx));
 
       Constant *Idx[2] = {
         ConstantInt::get(Int32Ty, 0),
         ConstantInt::get(Int32Ty, idx),
       };
       Constant *GEP =
           ConstantExpr::getInBoundsGetElementPtr(MergedTy, MergedGV, Idx);
       Globals[k]->replaceAllUsesWith(GEP);
-      Globals[k]->eraseFromParent();
+      Globals[k]->eraseFromParent(); 
 
       // When the linkage is not internal we must emit an alias for the original
       // variable name as it may be accessed from another object. On non-Mach-O
       // we can also emit an alias for internal linkage as it's safe to do so.
       // It's not safe on Mach-O as the alias (and thus the portion of the
       // MergedGlobals variable) may be dead stripped at link time.
       if (Linkage != GlobalValue::InternalLinkage || !IsMachO) {
         GlobalAlias::create(Tys[idx], AddrSpace, Linkage, Name, GEP, &M);

test/CodeGen/AArch64/global-merge-1.ll

 ; RUN: llc %s -mtriple=aarch64-none-linux-gnu -aarch64-enable-global-merge -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-none-linux-gnu -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s
 
 ; RUN: llc %s -mtriple=aarch64-linux-gnuabi -aarch64-enable-global-merge -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-linux-gnuabi -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s
 
 ; RUN: llc %s -mtriple=aarch64-apple-ios -aarch64-enable-global-merge -o - | FileCheck %s --check-prefix=CHECK-APPLE-IOS
 ; RUN: llc %s -mtriple=aarch64-apple-ios -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s --check-prefix=CHECK-APPLE-IOS
 
 @m = internal global i32 0, align 4
 @n = internal global i32 0, align 4
 
 define void @f1(i32 %a1, i32 %a2) {
 ;CHECK-APPLE-IOS-NOT: adrp
-;CHECK-APPLE-IOS: adrp  x8, l__MergedGlobals@PAGE
+;CHECK-APPLE-IOS: adrp  x8, __MergedGlobals@PAGE
 ;CHECK-APPLE-IOS-NOT: adrp
-;CHECK-APPLE-IOS: add x8, x8, l__MergedGlobals@PAGEOFF
+;CHECK-APPLE-IOS: add x8, x8, __MergedGlobals@PAGEOFF
   store i32 %a1, i32* @m, align 4
   store i32 %a2, i32* @n, align 4
   ret void
 }
 
 ;CHECK: .type .L_MergedGlobals,@object  // @_MergedGlobals
 ;CHECK: .local  .L_MergedGlobals
 ;CHECK: .comm .L_MergedGlobals,8,8
 ;CHECK: m = .L_MergedGlobals
 ;CHECK: n = .L_MergedGlobals+4
 
-;CHECK-APPLE-IOS: .zerofill __DATA,__bss,l__MergedGlobals,8,3 ; @_MergedGlobals
+;CHECK-APPLE-IOS: .zerofill __DATA,__bss,__MergedGlobals,8,3 ; @_MergedGlobals
 ;CHECK-APPLE-IOS-NOT: _m = l__MergedGlobals
 ;CHECK-APPLE-IOS-NOT: _n = l__MergedGlobals+4

test/CodeGen/AArch64/global-merge-2.ll

 ; RUN: llc %s -mtriple=aarch64-none-linux-gnu -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-linux-gnuabi -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-apple-ios -aarch64-enable-global-merge -global-merge-on-external -o - | FileCheck %s --check-prefix=CHECK-APPLE-IOS
 
 @x = global i32 0, align 4
 @y = global i32 0, align 4
 @z = global i32 0, align 4
 
 define void @f1(i32 %a1, i32 %a2) {
 ;CHECK-APPLE-IOS-LABEL: _f1:
 ;CHECK-APPLE-IOS-NOT: adrp
-;CHECK-APPLE-IOS: adrp  x8, l__MergedGlobals@PAGE
-;CHECK-APPLE-IOS: add x8, x8, l__MergedGlobals@PAGEOFF
+;CHECK-APPLE-IOS: adrp  x8, __MergedGlobals_x@PAGE
+;CHECK-APPLE-IOS: add x8, x8, __MergedGlobals_x@PAGEOFF
 ;CHECK-APPLE-IOS-NOT: adrp
   store i32 %a1, i32* @x, align 4
   store i32 %a2, i32* @y, align 4
   ret void
 }
 
 define void @g1(i32 %a1, i32 %a2) {
 ;CHECK-APPLE-IOS-LABEL: _g1:
-;CHECK-APPLE-IOS: adrp  x8, l__MergedGlobals@PAGE
-;CHECK-APPLE-IOS: add x8, x8, l__MergedGlobals@PAGEOFF
+;CHECK-APPLE-IOS: adrp  x8, __MergedGlobals_x@PAGE
+;CHECK-APPLE-IOS: add x8, x8, __MergedGlobals_x@PAGEOFF
 ;CHECK-APPLE-IOS-NOT: adrp
   store i32 %a1, i32* @y, align 4
   store i32 %a2, i32* @z, align 4
   ret void
 }
 
 ;CHECK: .type .L_MergedGlobals,@object // @_MergedGlobals
 ;CHECK: .local  .L_MergedGlobals
 ;CHECK: .comm .L_MergedGlobals,12,8
 
 ;CHECK: .globl  x
 ;CHECK: x = .L_MergedGlobals
 ;CHECK: .size x, 4
 ;CHECK: .globl  y
 ;CHECK: y = .L_MergedGlobals+4
 ;CHECK: .size y, 4
 ;CHECK: .globl  z
 ;CHECK: z = .L_MergedGlobals+8
 ;CHECK: .size z, 4
 
-;CHECK-APPLE-IOS: .zerofill __DATA,__bss,l__MergedGlobals,12,3
+;CHECK-APPLE-IOS: .zerofill __DATA,__common,__MergedGlobals_x,12,3
 
 ;CHECK-APPLE-IOS: .globl  _x
-;CHECK-APPLE-IOS:  = l__MergedGlobals
+;CHECK-APPLE-IOS:  = __MergedGlobals_x
 ;CHECK-APPLE-IOS: .globl  _y
-;CHECK-APPLE-IOS: _y = l__MergedGlobals+4
+;CHECK-APPLE-IOS: _y = __MergedGlobals_x+4
 ;CHECK-APPLE-IOS: .globl  _z
-;CHECK-APPLE-IOS: _z = l__MergedGlobals+8
+;CHECK-APPLE-IOS: _z = __MergedGlobals_x+8
 ;CHECK-APPLE-IOS: .subsections_via_symbols

test/CodeGen/AArch64/global-merge-3.ll

 ; RUN: llc %s -mtriple=aarch64-none-linux-gnu -aarch64-enable-global-merge -global-merge-on-external -disable-post-ra -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-linux-gnuabi -aarch64-enable-global-merge -global-merge-on-external -disable-post-ra -o - | FileCheck %s
 ; RUN: llc %s -mtriple=aarch64-apple-ios -aarch64-enable-global-merge -global-merge-on-external -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-APPLE-IOS
 
 @x = global [1000 x i32] zeroinitializer, align 1
 @y = global [1000 x i32] zeroinitializer, align 1
 @z = internal global i32 1, align 4
 
 define void @f1(i32 %a1, i32 %a2, i32 %a3) {
-;CHECK-APPLE-IOS: adrp  x8, l__MergedGlobals@PAGE
+;CHECK-APPLE-IOS: adrp  x8, __MergedGlobals_x@PAGE
 ;CHECK-APPLE-IOS-NOT: adrp
-;CHECK-APPLE-IOS: add x8, x8, l__MergedGlobals@PAGEOFF
-;CHECK-APPLE-IOS: adrp  x9, l__MergedGlobals.1@PAGE
-;CHECK-APPLE-IOS: add x9, x9, l__MergedGlobals.1@PAGEOFF
+;CHECK-APPLE-IOS: add x8, x8, __MergedGlobals_x@PAGEOFF
+;CHECK-APPLE-IOS: adrp  x9, __MergedGlobals_y@PAGE
+;CHECK-APPLE-IOS: add x9, x9, __MergedGlobals_y@PAGEOFF
   %x3 = getelementptr inbounds [1000 x i32], [1000 x i32]* @x, i32 0, i64 3
   %y3 = getelementptr inbounds [1000 x i32], [1000 x i32]* @y, i32 0, i64 3
   store i32 %a1, i32* %x3, align 4
   store i32 %a2, i32* %y3, align 4
   store i32 %a3, i32* @z, align 4
   ret void
 }
 
 ;CHECK: .type .L_MergedGlobals,@object // @_MergedGlobals
 ;CHECK: .p2align  4
 ;CHECK: .L_MergedGlobals:
 ;CHECK: .size .L_MergedGlobals, 4004
 
 ;CHECK: .type .L_MergedGlobals.1,@object // @_MergedGlobals.1
 ;CHECK: .local  .L_MergedGlobals.1
 ;CHECK: .comm .L_MergedGlobals.1,4000,16
 
 ;CHECK-APPLE-IOS: .p2align  4
-;CHECK-APPLE-IOS:  l__MergedGlobals:
+;CHECK-APPLE-IOS:  __MergedGlobals_x:
 ;CHECK-APPLE-IOS: .long 1
 ;CHECK-APPLE-IOS: .space  4000
 
-;CHECK-APPLE-IOS: .zerofill __DATA,__bss,l__MergedGlobals.1,4000,4
+;CHECK-APPLE-IOS: .zerofill __DATA,__common,__MergedGlobals_y,4000,4
 
 ;CHECK: z = .L_MergedGlobals
 ;CHECK: .globl  x
 ;CHECK: x = .L_MergedGlobals+4
 ;CHECK: .size x, 4000
 ;CHECK: .globl  y
 ;CHECK: y = .L_MergedGlobals.1
 ;CHECK: .size y, 4000
 
-;CHECK-APPLE-IOS-NOT: _z = l__MergedGlobals
+;CHECK-APPLE-IOS-NOT: _z = __MergedGlobals_x
 ;CHECK-APPLE-IOS:.globl _x
-;CHECK-APPLE-IOS: _x = l__MergedGlobals+4
+;CHECK-APPLE-IOS: _x = __MergedGlobals_x+4
 ;CHECK-APPLE-IOS:.globl _y
-;CHECK-APPLE-IOS: _y = l__MergedGlobals.1
+;CHECK-APPLE-IOS: _y = __MergedGlobals_y

test/CodeGen/AArch64/global-merge-group-by-use.ll

 ; RUN: llc -mtriple=aarch64-apple-ios -asm-verbose=false \
 ; RUN:   -aarch64-enable-collect-loh=false -aarch64-enable-global-merge \
 ; RUN:   -global-merge-group-by-use -global-merge-ignore-single-use=false %s \
 ; RUN:   -o - | FileCheck %s
 
 ; We assume that globals of the same size aren't reordered inside a set.
 
 ; Check that we create two MergedGlobal instances for two functions using
 ; disjoint sets of globals
 
 @m1 = internal global i32 0, align 4
 @n1 = internal global i32 0, align 4
 
 ; CHECK-LABEL: f1:
 define void @f1(i32 %a1, i32 %a2) #0 {
-; CHECK-NEXT: adrp x8, [[SET1:l__MergedGlobals.[0-9]*]]@PAGE
+; CHECK-NEXT: adrp x8, [[SET1:__MergedGlobals.[0-9]*]]@PAGE
 ; CHECK-NEXT: add x8, x8, [[SET1]]@PAGEOFF
 ; CHECK-NEXT: stp w0, w1, [x8]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m1, align 4
   store i32 %a2, i32* @n1, align 4
   ret void
 }
 
 @m2 = internal global i32 0, align 4
 @n2 = internal global i32 0, align 4
 @o2 = internal global i32 0, align 4
 
 ; CHECK-LABEL: f2:
 define void @f2(i32 %a1, i32 %a2, i32 %a3) #0 {
-; CHECK-NEXT: adrp x8, [[SET2:l__MergedGlobals.[0-9]*]]@PAGE
+; CHECK-NEXT: adrp x8, [[SET2:__MergedGlobals.[0-9]*]]@PAGE
 ; CHECK-NEXT: add x8, x8, [[SET2]]@PAGEOFF
 ; CHECK-NEXT: stp w0, w1, [x8]
 ; CHECK-NEXT: str w2, [x8, #8]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m2, align 4
   store i32 %a2, i32* @n2, align 4
   store i32 %a3, i32* @o2, align 4
   ret void
 }
 
 ; Sanity-check (don't worry about cost models) that we pick the biggest subset
 ; of all global used "together" directly or indirectly.  Here, that means
 ; merging n3, m4, and n4 together, but ignoring m3.
 
 @m3 = internal global i32 0, align 4
 @n3 = internal global i32 0, align 4
 
 ; CHECK-LABEL: f3:
 define void @f3(i32 %a1, i32 %a2) #0 {
 ; CHECK-NEXT: adrp x8, _m3@PAGE
-; CHECK-NEXT: adrp x9, [[SET3:l__MergedGlobals[0-9]*]]@PAGE
+; CHECK-NEXT: adrp x9, [[SET3:__MergedGlobals[0-9]*]]@PAGE
 ; CHECK-NEXT: str w0, [x8, _m3@PAGEOFF]
 ; CHECK-NEXT: str w1, [x9, [[SET3]]@PAGEOFF]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m3, align 4
   store i32 %a2, i32* @n3, align 4
   ret void
 }
 

test/CodeGen/AArch64/global-merge-ignore-single-use-minsize.ll

 ; RUN: llc -mtriple=aarch64-apple-ios -asm-verbose=false \
 ; RUN:   -aarch64-enable-collect-loh=false -O1 -global-merge-group-by-use \
 ; RUN:   -global-merge-ignore-single-use %s -o - | FileCheck %s
 
 ; Check that, at -O1, we only merge globals used in minsize functions.
 ; We assume that globals of the same size aren't reordered inside a set.
 ; We use -global-merge-ignore-single-use, and thus only expect one merged set.
 
 @m1 = internal global i32 0, align 4
 @n1 = internal global i32 0, align 4
 
 ; CHECK-LABEL: f1:
 define void @f1(i32 %a1, i32 %a2) minsize nounwind {
-; CHECK-NEXT: adrp x8, [[SET:l__MergedGlobals]]@PAGE
+; CHECK-NEXT: adrp x8, [[SET:__MergedGlobals]]@PAGE
 ; CHECK-NEXT: add x8, x8, [[SET]]@PAGEOFF
 ; CHECK-NEXT: stp w0, w1, [x8]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m1, align 4
   store i32 %a2, i32* @n1, align 4
   ret void
 }
 

test/CodeGen/AArch64/global-merge-ignore-single-use.ll

 ; RUN: llc -mtriple=aarch64-apple-ios -asm-verbose=false \
 ; RUN:   -aarch64-enable-collect-loh=false -aarch64-enable-global-merge \
 ; RUN:   -global-merge-group-by-use -global-merge-ignore-single-use %s -o - \
 ; RUN:   | FileCheck %s
 
 ; We assume that globals of the same size aren't reordered inside a set.
 
 @m1 = internal global i32 0, align 4
 @n1 = internal global i32 0, align 4
 @o1 = internal global i32 0, align 4
 
 ; CHECK-LABEL: f1:
 define void @f1(i32 %a1, i32 %a2) #0 {
-; CHECK-NEXT: adrp x8, [[SET:l__MergedGlobals]]@PAGE
+; CHECK-NEXT: adrp x8, [[SET:__MergedGlobals]]@PAGE
 ; CHECK-NEXT: add x8, x8, [[SET]]@PAGEOFF
 ; CHECK-NEXT: stp w0, w1, [x8]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m1, align 4
   store i32 %a2, i32* @n1, align 4
   ret void
 }
 

test/CodeGen/ARM/2011-06-29-MergeGlobalsAlign.ll

 ; RUN: llc < %s -mtriple=thumbv7-apple-darwin10 -arm-global-merge -global-merge-group-by-use=false | FileCheck %s
-; CHECK: .zerofill __DATA,__bss,l__MergedGlobals,16,2
+; CHECK: .zerofill __DATA,__bss,__MergedGlobals,16,2
 
 @prev = external global [0 x i16]
 @max_lazy_match = internal unnamed_addr global i32 0, align 4
 @read_buf = external global i32 (i8*, i32)*
 @window = external global [0 x i8]
 @lookahead = internal unnamed_addr global i32 0, align 4
 @eofile.b = internal unnamed_addr global i32 0
 @ins_h = internal unnamed_addr global i32 0, align 4

test/CodeGen/ARM/global-merge-1.ll

 ; RUN: llc %s -O0 -o - | FileCheck -check-prefix=NO-MERGE %s
 ; RUN: llc %s -O0 -o - -arm-global-merge=false | FileCheck -check-prefix=NO-MERGE %s
 ; RUN: llc %s -O0 -o - -arm-global-merge=true | FileCheck -check-prefix=MERGE %s
 ; RUN: llc %s -O1 -o - | FileCheck -check-prefix=NO-MERGE %s
 ; RUN: llc %s -O1 -o - -arm-global-merge=false | FileCheck -check-prefix=NO-MERGE %s
 ; RUN: llc %s -O1 -o - -arm-global-merge=true | FileCheck -check-prefix=MERGE %s
 ; RUN: llc %s -O3 -o - | FileCheck -check-prefix=MERGE %s
 ; RUN: llc %s -O3 -o - -arm-global-merge=false | FileCheck -check-prefix=NO-MERGE %s
 ; RUN: llc %s -O3 -o - -arm-global-merge=true | FileCheck -check-prefix=MERGE %s
 
 ; MERGE-NOT: .zerofill __DATA,__bss,_bar,20,2
 ; MERGE-NOT: .zerofill __DATA,__bss,_baz,20,2
 ; MERGE-NOT: .zerofill __DATA,__bss,_foo,20,2
-; MERGE: .zerofill __DATA,__bss,l__MergedGlobals,60,4
+; MERGE: .zerofill __DATA,__bss,__MergedGlobals,60,4
 ; MERGE-NOT: .zerofill __DATA,__bss,_bar,20,2
 ; MERGE-NOT: .zerofill __DATA,__bss,_baz,20,2
 ; MERGE-NOT: .zerofill __DATA,__bss,_foo,20,2
 
-; NO-MERGE-NOT: .zerofill __DATA,__bss,l__MergedGlobals,60,4
+; NO-MERGE-NOT: .zerofill __DATA,__bss,__MergedGlobals,60,4
 ; NO-MERGE: .zerofill __DATA,__bss,_bar,20,2
 ; NO-MERGE: .zerofill __DATA,__bss,_baz,20,2
 ; NO-MERGE: .zerofill __DATA,__bss,_foo,20,2
-; NO-MERGE-NOT: .zerofill __DATA,__bss,l__MergedGlobals,60,4
+; NO-MERGE-NOT: .zerofill __DATA,__bss,__MergedGlobals,60,4
 
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32-S32"
 target triple = "thumbv7-apple-ios3.0.0"
 
 @bar = internal global [5 x i32] zeroinitializer, align 4
 @baz = internal global [5 x i32] zeroinitializer, align 4
 @foo = internal global [5 x i32] zeroinitializer, align 4