[GlobalMerge] Set the alignment on merged global structs

[dmgreen]

Summary

If no alignment is set, the abi/preferred alignment of structs will be
used which may be higher than required. This can lead to extra padding
and in the end an increase in data size.

[efriedma]

lib/CodeGen/GlobalMerge.cpp

503 ↗

(On Diff #149444)

The alignment of the output should be based on the alignment of the input globals, not the alignment of the type of the merged global.

Looking more, we reject globals with an explicit alignment higher than the ABI alignment of the type. So I guess this is technically equivalent, but that isn't obvious; please add a comment.

[dmgreen]

Added a comment. I could change it to calculate the MaxAlignment from the alignment on the globals, if you think that's better in the long run.

[efriedma]

LGTM

Long-term this code should be rewritten to do alignment computations itself, and insert padding as needed, but this is fine for now.

[dmgreen]

Thanks.

llvm/trunk/lib/CodeGen/GlobalMerge.cpp

Twine MergedName =
             ? "_MergedGlobals_" + FirstExternalName
             : "_MergedGlobals";
     auto MergedLinkage = IsMachO ? Linkage : GlobalValue::PrivateLinkage;
     auto *MergedGV = new GlobalVariable(
         M, MergedTy, isConst, MergedLinkage, MergedInit, MergedName, nullptr,
         GlobalVariable::NotThreadLocal, AddrSpace);
 
     const StructLayout *MergedLayout = DL.getStructLayout(MergedTy);
+    // Set the alignment of the merged struct as the maximum alignment of the
+    // globals to prevent over-alignment. We don't handle globals that are not
+    // default aligned, so the alignment of the MergedLayout struct is
+    // equivalent.
+    MergedGV->setAlignment(MergedLayout->getAlignment());
 
     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();
       GlobalValue::DLLStorageClassTypes DLLStorage =
           Globals[k]->getDLLStorageClass();
 
       // Copy metadata while adjusting any debug info metadata by the original

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

 ;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: .comm .L_MergedGlobals,8,4
 ;CHECK: .set m, .L_MergedGlobals
 ;CHECK: .set n, .L_MergedGlobals+4
 
-;CHECK-APPLE-IOS: .zerofill __DATA,__bss,__MergedGlobals,8,3 ; @_MergedGlobals
+;CHECK-APPLE-IOS: .zerofill __DATA,__bss,__MergedGlobals,8,2 ; @_MergedGlobals
 ;CHECK-APPLE-IOS-NOT: .set _m, l__MergedGlobals
 ;CHECK-APPLE-IOS-NOT: .set _n, l__MergedGlobals+4

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

 ;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: .comm .L_MergedGlobals,12,4
 
 ;CHECK: .globl  x
 ;CHECK: .set x, .L_MergedGlobals
 ;CHECK: .size x, 4
 ;CHECK: .globl  y
 ;CHECK: .set y, .L_MergedGlobals+4
 ;CHECK: .size y, 4
 ;CHECK: .globl  z
 ;CHECK: .set z, .L_MergedGlobals+8
 ;CHECK: .size z, 4
 
-;CHECK-APPLE-IOS: .zerofill __DATA,__common,__MergedGlobals_x,12,3
+;CHECK-APPLE-IOS: .zerofill __DATA,__common,__MergedGlobals_x,12,2
 
 ;CHECK-APPLE-IOS: .globl  _x
 ;CHECK-APPLE-IOS: .set {{.*}}, __MergedGlobals_x
 ;CHECK-APPLE-IOS: .globl  _y
 ;CHECK-APPLE-IOS: .set _y, __MergedGlobals_x+4
 ;CHECK-APPLE-IOS: .globl  _z
 ;CHECK-APPLE-IOS: .set _z, __MergedGlobals_x+8
 ;CHECK-APPLE-IOS: .subsections_via_symbols

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

;CHECK-APPLE-IOS: str w1, [x9]
   %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: .p2align  2
 ;CHECK: .L_MergedGlobals:
 ;CHECK: .size .L_MergedGlobals, 4004
 
 ;CHECK-APPLE-IOS: .zerofill __DATA,__common,_y,4000,2
 
-;CHECK-APPLE-IOS: .p2align  4
+;CHECK-APPLE-IOS: .p2align  2
 ;CHECK-APPLE-IOS:  __MergedGlobals_x:
 ;CHECK-APPLE-IOS: .long 1
 ;CHECK-APPLE-IOS: .space  4000
 
 
 ;CHECK: .set z, .L_MergedGlobals
 ;CHECK: .globl  x
 ;CHECK: .set x, .L_MergedGlobals+4
 ;CHECK: .size x, 4000
 
 ;CHECK-APPLE-IOS-NOT: .set _z, __MergedGlobals_x
 ;CHECK-APPLE-IOS:.globl _x
 ;CHECK-APPLE-IOS:.set _x, __MergedGlobals_x+4

llvm/trunk/test/CodeGen/AArch64/global-merge-4.ll

 
 ; Function Attrs: nounwind readnone ssp
 define internal i32* @returnFoo() #1 {
   ret i32* getelementptr inbounds ([5 x i32], [5 x i32]* @foo, i64 0, i64 0)
 }
 
 ;CHECK: .type .L_MergedGlobals,@object  // @_MergedGlobals
 ;CHECK: .local  .L_MergedGlobals
-;CHECK: .comm .L_MergedGlobals,60,16
+;CHECK: .comm .L_MergedGlobals,60,4
 
 attributes #0 = { nounwind ssp }
 attributes #1 = { nounwind readnone ssp }
 attributes #2 = { nounwind }

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

 ; CHECK-NEXT: str w0, [x8, _o5@PAGEOFF]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @o5, align 4
   ret void
 }
 
 ; CHECK-DAG: .zerofill __DATA,__bss,_o5,4,2
 
-; CHECK-DAG: .zerofill __DATA,__bss,[[SET1]],8,3
-; CHECK-DAG: .zerofill __DATA,__bss,[[SET2]],12,3
-; CHECK-DAG: .zerofill __DATA,__bss,[[SET3]],12,3
+; CHECK-DAG: .zerofill __DATA,__bss,[[SET1]],8,2
+; CHECK-DAG: .zerofill __DATA,__bss,[[SET2]],12,2
+; CHECK-DAG: .zerofill __DATA,__bss,[[SET3]],12,2
 
 attributes #0 = { nounwind }

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

 ; CHECK-NEXT: str w0, [x8, [[SET]]@PAGEOFF+8]
 ; CHECK-NEXT: str w1, [x9, _n4@PAGEOFF]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @m3, align 4
   store i32 %a2, i32* @n4, align 4
   ret void
 }
 
-; CHECK-DAG: .zerofill __DATA,__bss,[[SET]],16,3
+; CHECK-DAG: .zerofill __DATA,__bss,[[SET]],16,2
 ; CHECK-DAG: .zerofill __DATA,__bss,_m2,4,2
 ; CHECK-DAG: .zerofill __DATA,__bss,_n2,4,2
 ; CHECK-DAG: .zerofill __DATA,__bss,_n4,4,2

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

 define void @f4(i32 %a1) #0 {
 ; CHECK-NEXT: adrp x8, _o2@PAGE
 ; CHECK-NEXT: str w0, [x8, _o2@PAGEOFF]
 ; CHECK-NEXT: ret
   store i32 %a1, i32* @o2, align 4
   ret void
 }
 
-; CHECK-DAG: .zerofill __DATA,__bss,[[SET]],20,4
+; CHECK-DAG: .zerofill __DATA,__bss,[[SET]],20,2
 ; CHECK-DAG: .zerofill __DATA,__bss,_o2,4,2
 
 attributes #0 = { nounwind }

llvm/trunk/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,__MergedGlobals,60,4
+; MERGE: .zerofill __DATA,__bss,__MergedGlobals,60,2
 ; 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,__MergedGlobals,60,4
+; NO-MERGE-NOT: .zerofill __DATA,__bss,__MergedGlobals,60,2
 ; 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,__MergedGlobals,60,4
+; NO-MERGE-NOT: .zerofill __DATA,__bss,__MergedGlobals,60,2
 
 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
 

llvm/trunk/test/CodeGen/ARM/global-merge-alignment.ll

+; RUN: llc < %s -arm-global-merge -data-sections | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7em-arm-none-eabi"
+
+@f = local_unnamed_addr global [4 x i32*] zeroinitializer, align 4
+@d = local_unnamed_addr global i64 0, align 8
+
+;CHECK: .section  .bss..L_MergedGlobals,"aw",%nobits
+;CHECK-NEXT: .p2align 3
+;CHECK-NEXT: .L_MergedGlobals:
+;CHECK-NEXT: .zero  24
+;CHECK-NEXT: .size  .L_MergedGlobals, 24
+
+
+define i32 @func_1() {
+  %1 = load i64, i64* @d, align 8
+  %2 = load i32*, i32** getelementptr inbounds ([4 x i32*], [4 x i32*]* @f, i32 0, i32 0), align 4
+  %3 = load i32, i32* %2, align 4
+  %4 = trunc i64 %1 to i32
+  %5 = add i32 %3, %4
+  ret i32 %5
+}
+

llvm/trunk/test/Transforms/GlobalMerge/debug-info.ll

 ; RUN: opt -global-merge -global-merge-max-offset=100 -S -o - %s | FileCheck %s
 
 source_filename = "test/Transforms/GlobalMerge/debug-info.ll"
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
-; CHECK: @_MergedGlobals = private global { i32, i32 } { i32 1, i32 2 }, !dbg [[A:![0-9]+]], !dbg [[B:![0-9]+]]
+; CHECK: @_MergedGlobals = private global { i32, i32 } { i32 1, i32 2 }, align 4, !dbg [[A:![0-9]+]], !dbg [[B:![0-9]+]]
 
 @a = internal global i32 1, !dbg !0
 @b = internal global i32 2, !dbg !2
 
 define void @use1() {
   %x = load i32, i32* @a
   %y = load i32, i32* @b
   ret void