[ARM] Align global variables passed to memory intrinsics

[john.brawn]

Summary

Fill in the TODO in CodeGenPrepare::OptimizeCallInst so that global variables that are passed to memory intrinsics are aligned in the same way that allocas are.

[hfinkel]

lib/CodeGen/CodeGenPrepare.cpp

1249 ↗

(On Diff #22198)

If you add a continue here, then you might end up skipping some logic further down that would otherwise apply, no?

1257 ↗

(On Diff #22198)

Why? Do the alignments need to match?

1260 ↗

(On Diff #22198)

Do you want this or hasUniqueInitializer() or hasDefinitiveInitializer()?

[john.brawn]

lib/CodeGen/CodeGenPrepare.cpp

1249 ↗

(On Diff #22198)

That's what we want: this is the 'offset must be a multiple of the alignment' check, and if it's not true we don't want to align for both allocas and global variables.

1257 ↗

(On Diff #22198)

http://llvm.org/docs/LangRef.html#global-variables says:
Targets and optimizers are not allowed to over-align the global if the global has an assigned section. In this case, the extra alignment could be observable: for example, code could assume that the globals are densely packed in their section and try to iterate over them as an array, alignment padding would break this iteration.

1260 ↗

(On Diff #22198)

I think that hasUniqueInitializer is what we want. We want to make sure that increasing the alignment of this GlobalVariable will definitely increase the alignment in the final executable, i.e. we won't get a definition of this variable from another object that doesn't have this increased alignment.

[john.brawn]

Change to use hasUniqueInitializer instead of hasInitializer.

[rengolin]

lib/CodeGen/CodeGenPrepare.cpp

1249 ↗

(On Diff #22198)

I agree, this is a continue on the local block, only, and it has the same semantics as previously.

1261 ↗

(On Diff #22198)

Why is this necessary?

[john.brawn]

lib/CodeGen/CodeGenPrepare.cpp

1261 ↗

(On Diff #22198)

As I said to Hal previously, http://llvm.org/docs/LangRef.html#global-variables says:
Targets and optimizers are not allowed to over-align the global if the global has an assigned section. In this case, the extra alignment could be observable: for example, code could assume that the globals are densely packed in their section and try to iterate over them as an array, alignment padding would break this iteration.

[rengolin]

LGTM. But please, let Hal reply to your questions before commit.

lib/CodeGen/CodeGenPrepare.cpp

1261 ↗

(On Diff #22198)

Sorry, I missed the "section" part when I read that. :)

[john.brawn]

Hal, any comments on my comments on your comments? If not I'll check this in.

[john.brawn]

Hal: pinging you to check if this is OK to commit.

[hfinkel]

In D8421#153211, @john.brawn wrote:

Hal: pinging you to check if this is OK to commit.

LGTM too, thanks!

llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

for (auto &Arg : CI->arg_operands()) {
       // sense for GEPs if the offset is a multiple of the desired alignment and
       // if size - offset meets the size threshold.
       if (!Arg->getType()->isPointerTy())
         continue;
       APInt Offset(TD->getPointerSizeInBits(
                      cast<PointerType>(Arg->getType())->getAddressSpace()), 0);
       Value *Val = Arg->stripAndAccumulateInBoundsConstantOffsets(*TD, Offset);
       uint64_t Offset2 = Offset.getLimitedValue();
+      if ((Offset2 & (PrefAlign-1)) != 0)
+        continue;
       AllocaInst *AI;
-      if ((Offset2 & (PrefAlign-1)) == 0 &&
-          (AI = dyn_cast<AllocaInst>(Val)) &&
+      if ((AI = dyn_cast<AllocaInst>(Val)) &&
           AI->getAlignment() < PrefAlign &&
           TD->getTypeAllocSize(AI->getAllocatedType()) >= MinSize + Offset2)
         AI->setAlignment(PrefAlign);
-      // TODO: Also align GlobalVariables
+      // Global variables can only be aligned if they are defined in this
+      // object (i.e. they are uniquely initialized in this object), and
+      // over-aligning global variables that have an explicit section is
+      // forbidden.
+      GlobalVariable *GV;
+      if ((GV = dyn_cast<GlobalVariable>(Val)) &&
+          GV->hasUniqueInitializer() &&
+          !GV->hasSection() &&
+          GV->getAlignment() < PrefAlign &&
+          TD->getTypeAllocSize(
+            GV->getType()->getElementType()) >= MinSize + Offset2)
+        GV->setAlignment(PrefAlign);
     }
     // If this is a memcpy (or similar) then we may be able to improve the
     // alignment
     if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(CI)) {
       unsigned Align = getKnownAlignment(MI->getDest(), *TD);
       if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI))
         Align = std::min(Align, getKnownAlignment(MTI->getSource(), *TD));
       if (Align > MI->getAlignment())

llvm/trunk/test/CodeGen/ARM/memcpy-inline.ll

 ; CHECK-T1: strh [[TREG2]]
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* getelementptr inbounds (%struct.x, %struct.x* @dst, i32 0, i32 0), i8* getelementptr inbounds (%struct.x, %struct.x* @src, i32 0, i32 0), i32 11, i32 8, i1 false)
   ret i32 0
 }
 
 define void @t1(i8* nocapture %C) nounwind {
 entry:
 ; CHECK-LABEL: t1:
-; CHECK: vld1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]
+; CHECK: vld1.64 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]
 ; CHECK: vst1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r0]
 ; CHECK: adds r0, #15
 ; CHECK: adds r1, #15
 ; CHECK: vld1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]
 ; CHECK: vst1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r0]
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %C, i8* getelementptr inbounds ([31 x i8], [31 x i8]* @.str1, i64 0, i64 0), i64 31, i32 1, i1 false)
   ret void
 }
 
 define void @t2(i8* nocapture %C) nounwind {
 entry:
 ; CHECK-LABEL: t2:
 ; CHECK: movw [[REG2:r[0-9]+]], #16716
 ; CHECK: movt [[REG2:r[0-9]+]], #72
 ; CHECK: str [[REG2]], [r0, #32]
 ; CHECK: vld1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]!
 ; CHECK: vst1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r0]!
-; CHECK: vld1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]
+; CHECK: vld1.64 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]
 ; CHECK: vst1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r0]
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %C, i8* getelementptr inbounds ([36 x i8], [36 x i8]* @.str2, i64 0, i64 0), i64 36, i32 1, i1 false)
   ret void
 }
 
 define void @t3(i8* nocapture %C) nounwind {
 entry:
 ; CHECK-LABEL: t3:
 ; CHECK: vld1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r1]!
 ; CHECK: vst1.8 {d{{[0-9]+}}, d{{[0-9]+}}}, [r0]!
-; CHECK: vld1.8 {d{{[0-9]+}}}, [r1]
+; CHECK: vldr d{{[0-9]+}}, [r1]
 ; CHECK: vst1.8 {d{{[0-9]+}}}, [r0]
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %C, i8* getelementptr inbounds ([24 x i8], [24 x i8]* @.str3, i64 0, i64 0), i64 24, i32 1, i1 false)
   ret void
 }
 
 define void @t4(i8* nocapture %C) nounwind {
 entry:
 ; CHECK-LABEL: t4:
-; CHECK: vld1.8 {[[REG3:d[0-9]+]], [[REG4:d[0-9]+]]}, [r1]
+; CHECK: vld1.64 {[[REG3:d[0-9]+]], [[REG4:d[0-9]+]]}, [r1]
 ; CHECK: vst1.8 {[[REG3]], [[REG4]]}, [r0]!
 ; CHECK: strh [[REG5:r[0-9]+]], [r0]
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %C, i8* getelementptr inbounds ([18 x i8], [18 x i8]* @.str4, i64 0, i64 0), i64 18, i32 1, i1 false)
   ret void
 }
 
 define void @t5(i8* nocapture %C) nounwind {
 entry:

 ; CHECK-T1: strb [[TREG4]],
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %C, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str5, i64 0, i64 0), i64 7, i32 1, i1 false)
   ret void
 }
 
 define void @t6() nounwind {
 entry:
 ; CHECK-LABEL: t6:
-; CHECK: vld1.8 {[[REG9:d[0-9]+]]}, [r0]
+; CHECK: vldr [[REG9:d[0-9]+]], [r0]
 ; CHECK: vstr [[REG9]], [r1]
 ; CHECK: adds r1, #6
 ; CHECK: adds r0, #6
-; CHECK: vld1.8
+; CHECK: vld1.16
 ; CHECK: vst1.16
 ; CHECK-T1-LABEL: t6:
 ; CHECK-T1: movs [[TREG5:r[0-9]]],
 ; CHECK-T1: strh [[TREG5]],
 ; CHECK-T1: ldr [[TREG6:r[0-9]]],
 ; CHECK-T1: str [[TREG6]]
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds ([512 x i8], [512 x i8]* @spool.splbuf, i64 0, i64 0), i8* getelementptr inbounds ([14 x i8], [14 x i8]* @.str6, i64 0, i64 0), i64 14, i32 1, i1 false)
   ret void

llvm/trunk/test/CodeGen/ARM/memfunc.ll

 ; RUN: llc < %s -mtriple=armv7-apple-ios -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-IOS --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=thumbv7m-none-macho -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-DARWIN --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-eabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-eabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
 
-@from = common global [500 x i32] zeroinitializer, align 4
-@to = common global [500 x i32] zeroinitializer, align 4
-
-define void @f1() {
+define void @f1(i8* %dest, i8* %src) {
 entry:
   ; CHECK-LABEL: f1
 
   ; CHECK-IOS: memmove
   ; CHECK-DARWIN: memmove
   ; CHECK-EABI: __aeabi_memmove
-  call void @llvm.memmove.p0i8.p0i8.i32(i8* bitcast ([500 x i32]* @from to i8*), i8* bitcast ([500 x i32]* @to to i8*), i32 500, i32 0, i1 false)
+  call void @llvm.memmove.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 500, i32 0, i1 false)
 
   ; CHECK-IOS: memcpy
   ; CHECK-DARWIN: memcpy
   ; CHECK-EABI: __aeabi_memcpy
-  call void @llvm.memcpy.p0i8.p0i8.i32(i8* bitcast ([500 x i32]* @from to i8*), i8* bitcast ([500 x i32]* @to to i8*), i32 500, i32 0, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 500, i32 0, i1 false)
 
   ; EABI memset swaps arguments
   ; CHECK-IOS: mov r1, #0
   ; CHECK-IOS: memset
   ; CHECK-DARWIN: movs r1, #0
   ; CHECK-DARWIN: memset
   ; CHECK-EABI: mov r2, #0
   ; CHECK-EABI: __aeabi_memset
-  call void @llvm.memset.p0i8.i32(i8* bitcast ([500 x i32]* @from to i8*), i8 0, i32 500, i32 0, i1 false)
+  call void @llvm.memset.p0i8.i32(i8* %dest, i8 0, i32 500, i32 0, i1 false)
   unreachable
 }
 
 ; Check that alloca arguments to memory intrinsics are automatically aligned if at least 8 bytes in size
 define void @f2(i8* %dest, i32 %n) {
 entry:
   ; CHECK-LABEL: f2
 
entry:
   ; CHECK-EABI: __aeabi_memset
   %arr2 = alloca [13 x i8], align 1
   %2 = getelementptr inbounds [13 x i8], [13 x i8]* %arr2, i32 0, i32 16
   call void @llvm.memset.p0i8.i32(i8* %2, i8 0, i32 %n, i32 0, i1 false)
 
   unreachable
 }
 
+; Check that global variables are aligned if they are large enough, but only if
+; they are defined in this object and don't have an explicit section.
+@arr1 = global [7 x i8] c"\01\02\03\04\05\06\07", align 1
+@arr2 = global [8 x i8] c"\01\02\03\04\05\06\07\08", align 1
+@arr3 = global [7 x i8] c"\01\02\03\04\05\06\07", section "foo,bar", align 1
+@arr4 = global [8 x i8] c"\01\02\03\04\05\06\07\08", section "foo,bar", align 1
+@arr5 = weak global [7 x i8] c"\01\02\03\04\05\06\07", align 1
+@arr6 = weak_odr global [7 x i8] c"\01\02\03\04\05\06\07", align 1
+@arr7 = external global [7 x i8], align 1
+define void @f9(i8* %dest, i32 %n) {
+entry:
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @arr1, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([8 x i8], [8 x i8]* @arr2, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @arr3, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([8 x i8], [8 x i8]* @arr4, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @arr5, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @arr6, i32 0, i32 0), i32 %n, i32 1, i1 false)
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @arr7, i32 0, i32 0), i32 %n, i32 1, i1 false)
+
+  unreachable
+}
+
+; CHECK: {{\.data|\.section.+data}}
+; CHECK-NOT: .align
+; CHECK: arr1:
+; CHECK-IOS: .align 3
+; CHECK-DARWIN: .align 2
+; CHECK-EABI: .align 2
+; CHECK: arr2:
+; CHECK: {{\.section.+foo,bar}}
+; CHECK-NOT: .align
+; CHECK: arr3:
+; CHECK-NOT: .align
+; CHECK: arr4:
+; CHECK: {{\.data|\.section.+data}}
+; CHECK-NOT: .align
+; CHECK: arr5:
+; CHECK-NOT: .align
+; CHECK: arr6:
+; CHECK-NOT: arr7:
+
 declare void @llvm.memmove.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
 declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind