ARM] Fix Neon vector type alignment to 64-bit

[javed.absar]

Summary

The maximum alignment for ARM NEON data types should be 64-bits as specified in ARM procedure call standard document Sec. A.2 Notes [1]. This patch fixes it from its current larger natural default values.

[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf

[olista01]

LGTM

[rengolin]

Wait, Stephen had a concern, we need his feedback on my reply before approving.

[srhines]

lib/Basic/Targets.cpp

5300

This should not be set for Android targets. Doing so would break our existing ABI.

[olista01]

Ah, I just found the email thread, looks like it never reached phabricator. Sorry for the confusion.

[rengolin]

In D33205#761141, @olista01 wrote:

Ah, I just found the email thread, looks like it never reached phabricator. Sorry for the confusion.

Np. Phab should really do something useful when it's copied in the thread... :)

[rengolin]

lib/Basic/Targets.cpp

5300

Regardless of the merits, being practical, I think we should make this condition to the environment, to make sure that at least Android keeps its original alignment.

Docs updates and future changes to the ABI should be handled separately.

[javed.absar]

Hi all:
I have made the changes (excluding Android) as recommended. Please have a look if it is now fine.
Thanks
--Javed

[rengolin]

In D33205#762066, @javed.absar wrote:

Hi all:
I have made the changes (excluding Android) as recommended. Please have a look if it is now fine.

Hi Javed, looks good, but please add a comment so people understand why Android is different.

@srhines, does that look good to you?

cheers,
--renato

[rengolin]

Approving, so that when Steve does the same, the review is free to commit.

[srhines]

lib/Basic/Targets.cpp

5300

A comment would be good here, and arm-abi-vector.c needs to have rules for checking Android now too. As it stands, there is no test for this code path, which would be bad. I think just keeping the old CHECK's as ANDROID should be fine.

[javed.absar]

Hi.
I have added the comment for Android 128-bit alignement.
Also, arm-ab-vector.c do have checks for Android (please see line line 151,157), so should be fine.
--Javed

[srhines]

Thanks for fixing this up for Android.

lib/Basic/Targets.cpp

ARMTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     }
 
     // ARM targets default to using the ARM C++ ABI.
     TheCXXABI.set(TargetCXXABI::GenericARM);
 
     // ARM has atomics up to 8 bytes
     setAtomic();
 
+    MaxVectorAlign = 64; // 64-bit alignment (AAPCS)
+
     // Do force alignment of members that follow zero length bitfields.  If
     // the alignment of the zero-length bitfield is greater than the member
     // that follows it, `bar', `bar' will be aligned as the  type of the
     // zero length bitfield.
     UseZeroLengthBitfieldAlignment = true;
 
     if (Triple.getOS() == llvm::Triple::Linux ||
         Triple.getOS() == llvm::Triple::UnknownOS)

test/CodeGen/arm-abi-vector.c

 // APCS-GNU: call double (i32, ...) @varargs_vec_5c(i32 5, <2 x i32> {{%.*}})
 // ANDROID: test_5c
 // ANDROID: call double (i32, ...) @varargs_vec_5c(i32 5, <2 x i32> {{%.*}})
   return varargs_vec_5c(5, *in);
 }
 
 double varargs_vec_9c(int fixed, ...) {
 // CHECK: varargs_vec_9c
-// CHECK: [[VAR:%.*]] = alloca <9 x i8>, align 16
+// CHECK: [[VAR:%.*]] = alloca <9 x i8>, align 8
 // CHECK: [[ALIGN:%.*]] = and i32 {{%.*}}, -8
 // CHECK: [[AP_ALIGN:%.*]] = inttoptr i32 [[ALIGN]] to i8*
 // CHECK: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP_ALIGN]], i32 16
 // CHECK: [[AP_CAST:%.*]] = bitcast i8* [[AP_ALIGN]] to <9 x i8>*
 // CHECK: [[T0:%.*]] = load <9 x i8>, <9 x i8>* [[AP_CAST]], align 8
-// CHECK: store <9 x i8> [[T0]], <9 x i8>* [[VAR]], align 16
+// CHECK: store <9 x i8> [[T0]], <9 x i8>* [[VAR]], align 8
 // APCS-GNU: varargs_vec_9c
-// APCS-GNU: [[VAR:%.*]] = alloca <9 x i8>, align 16
+// APCS-GNU: [[VAR:%.*]] = alloca <9 x i8>, align 8
 // APCS-GNU: [[AP:%.*]] = load i8*,
 // APCS-GNU: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP]], i32 16
 // APCS-GNU: [[AP_CAST:%.*]] = bitcast i8* [[AP]] to <9 x i8>*
 // APCS-GNU: [[VEC:%.*]] = load <9 x i8>, <9 x i8>* [[AP_CAST]], align 4
-// APCS-GNU: store <9 x i8> [[VEC]], <9 x i8>* [[VAR]], align 16
+// APCS-GNU: store <9 x i8> [[VEC]], <9 x i8>* [[VAR]], align 8
 // ANDROID: varargs_vec_9c
-// ANDROID: [[VAR:%.*]] = alloca <9 x i8>, align 16
+// ANDROID: [[VAR:%.*]] = alloca <9 x i8>, align 8
 // ANDROID: [[ALIGN:%.*]] = and i32 {{%.*}}, -8
 // ANDROID: [[AP_ALIGN:%.*]] = inttoptr i32 [[ALIGN]] to i8*
 // ANDROID: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP_ALIGN]], i32 16
 // ANDROID: [[AP_CAST:%.*]] = bitcast i8* [[AP_ALIGN]] to <9 x i8>*
 // ANDROID: [[T0:%.*]] = load <9 x i8>, <9 x i8>* [[AP_CAST]], align 8
-// ANDROID: store <9 x i8> [[T0]], <9 x i8>* [[VAR]], align 16
+// ANDROID: store <9 x i8> [[T0]], <9 x i8>* [[VAR]], align 8
   va_list ap;
   double sum = fixed;
   va_start(ap, fixed);
   __char9 c9 = va_arg(ap, __char9);
   sum = sum + c9.x + c9.y;
   va_end(ap);
   return sum;
 }

 // APCS-GNU: call double (i32, ...) @varargs_vec_3s(i32 3, <2 x i32> {{%.*}})
 // ANDROID: test_3s
 // ANDROID: call double (i32, ...) @varargs_vec_3s(i32 3, <3 x i16> {{%.*}})
   return varargs_vec_3s(3, *in);
 }
 
 double varargs_vec_5s(int fixed, ...) {
 // CHECK: varargs_vec_5s
-// CHECK: [[VAR_ALIGN:%.*]] = alloca <5 x i16>, align 16
+// CHECK: [[VAR_ALIGN:%.*]] = alloca <5 x i16>, align 8
 // CHECK: [[ALIGN:%.*]] = and i32 {{%.*}}, -8
 // CHECK: [[AP_ALIGN:%.*]] = inttoptr i32 [[ALIGN]] to i8*
 // CHECK: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP_ALIGN]], i32 16
 // CHECK: [[AP_CAST:%.*]] = bitcast i8* [[AP_ALIGN]] to <5 x i16>*
 // CHECK: [[VEC:%.*]] = load <5 x i16>, <5 x i16>* [[AP_CAST]], align 8
-// CHECK: store <5 x i16> [[VEC]], <5 x i16>* [[VAR_ALIGN]], align 16
+// CHECK: store <5 x i16> [[VEC]], <5 x i16>* [[VAR_ALIGN]], align 8
 // APCS-GNU: varargs_vec_5s
-// APCS-GNU: [[VAR:%.*]] = alloca <5 x i16>, align 16
+// APCS-GNU: [[VAR:%.*]] = alloca <5 x i16>, align 8
 // APCS-GNU: [[AP:%.*]] = load i8*,
 // APCS-GNU: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP]], i32 16
 // APCS-GNU: [[AP_CAST:%.*]] = bitcast i8* [[AP]] to <5 x i16>*
 // APCS-GNU: [[VEC:%.*]] = load <5 x i16>, <5 x i16>* [[AP_CAST]], align 4
 // ANDROID: varargs_vec_5s
-// ANDROID: [[VAR_ALIGN:%.*]] = alloca <5 x i16>, align 16
+// ANDROID: [[VAR_ALIGN:%.*]] = alloca <5 x i16>, align 8
 // ANDROID: [[ALIGN:%.*]] = and i32 {{%.*}}, -8
 // ANDROID: [[AP_ALIGN:%.*]] = inttoptr i32 [[ALIGN]] to i8*
 // ANDROID: [[AP_NEXT:%.*]] = getelementptr inbounds i8, i8* [[AP_ALIGN]], i32 16
 // ANDROID: [[AP_CAST:%.*]] = bitcast i8* [[AP_ALIGN]] to <5 x i16>*
 // ANDROID: [[VEC:%.*]] = load <5 x i16>, <5 x i16>* [[AP_CAST]], align 8
-// ANDROID: store <5 x i16> [[VEC]], <5 x i16>* [[VAR_ALIGN]], align 16
+// ANDROID: store <5 x i16> [[VEC]], <5 x i16>* [[VAR_ALIGN]], align 8
   va_list ap;
   double sum = fixed;
   va_start(ap, fixed);
   __short5 c5 = va_arg(ap, __short5);
   sum = sum + c5.x + c5.y;
   va_end(ap);
   return sum;
 }

test/CodeGen/arm-neon-misc.c

 // CHECK: call <2 x i64> @llvm.arm.neon.vld1.v2i64.p0i8
     q = vld1q_lane_u64(src2, q, 0);
 // CHECK: shufflevector <2 x i64>
 // CHECK: call <1 x i64> @llvm.arm.neon.vld1.v1i64.p0i8
 // CHECK: shufflevector <1 x i64>
     *dst = q;
 // CHECK: store <2 x i64>
 }
+
+// Neon types have 64-bit alignment
+int32x4_t gl_b;
+void t3(int32x4_t *src) {
+// CHECK: @t3
+  gl_b = *src;
+// CHECK: store <4 x i32> {{%.*}}, <4 x i32>* @gl_b, align 8
+}

test/CodeGen/arm-swiftcall.c

 // CHECK:   ret void
 
 typedef union {
   float f1;
   float4 fv2;
 } union_hom_fp_partial;
 TEST(union_hom_fp_partial)
 // CHECK-LABEL: define void @test_union_hom_fp_partial()
-// CHECK:   [[TMP:%.*]] = alloca [[REC:%.*]], align 16
+// CHECK:   [[TMP:%.*]] = alloca [[REC:%.*]], align 8
 // CHECK:   [[CALL:%.*]] = call [[SWIFTCC]] [[UAGG:{ float, float, float, float }]] @return_union_hom_fp_partial()
 // CHECK:   [[CAST_TMP:%.*]] = bitcast [[REC]]* [[TMP]] to [[AGG:{ float, float, float, float }]]*
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 0
 // CHECK:   [[T1:%.*]] = extractvalue [[UAGG]] [[CALL]], 0
 // CHECK:   store float [[T1]], float* [[T0]], align
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 1
 // CHECK:   [[T1:%.*]] = extractvalue [[UAGG]] [[CALL]], 1
 // CHECK:   store float [[T1]], float* [[T0]], align

 // CHECK:   ret void
 
 typedef union {
   struct { int x, y; } f1;
   float4 fv2;
 } union_het_fpv_partial;
 TEST(union_het_fpv_partial)
 // CHECK-LABEL: define void @test_union_het_fpv_partial()
-// CHECK:   [[TMP:%.*]] = alloca [[REC:%.*]], align 16
+// CHECK:   [[TMP:%.*]] = alloca [[REC:%.*]], align 8
 // CHECK:   [[CALL:%.*]] = call [[SWIFTCC]] [[UAGG:{ i32, i32, float, float }]] @return_union_het_fpv_partial()
 // CHECK:   [[CAST_TMP:%.*]] = bitcast [[REC]]* [[TMP]] to [[AGG:{ i32, i32, float, float }]]*
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 0
 // CHECK:   [[T1:%.*]] = extractvalue [[UAGG]] [[CALL]], 0
 // CHECK:   store i32 [[T1]], i32* [[T0]], align
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 1
 // CHECK:   [[T1:%.*]] = extractvalue [[UAGG]] [[CALL]], 1
 // CHECK:   store i32 [[T1]], i32* [[T0]], align

 /*****************************************************************************/
 
 TEST(int4)
 // CHECK-LABEL: define {{.*}} <4 x i32> @return_int4()
 // CHECK-LABEL: define {{.*}} @take_int4(<4 x i32>
 
 TEST(int8)
 // CHECK-LABEL: define {{.*}} @return_int8()
-// CHECK:   [[RET:%.*]] = alloca [[REC:<8 x i32>]], align 32
+// CHECK:   [[RET:%.*]] = alloca [[REC:<8 x i32>]], align 8
 // CHECK:   [[VAR:%.*]] = alloca [[REC]], align
 // CHECK:   store
 // CHECK:   load
 // CHECK:   store
 // CHECK:   [[CAST_TMP:%.*]] = bitcast [[REC]]* [[RET]] to [[AGG:{ <4 x i32>, <4 x i32> }]]*
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 0
 // CHECK:   [[FIRST:%.*]] = load <4 x i32>, <4 x i32>* [[T0]], align
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 1

 // CHECK:   [[FIRST:%.*]] = load <4 x i32>, <4 x i32>* [[T0]], align
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 1
 // CHECK:   [[SECOND:%.*]] = load <4 x i32>, <4 x i32>* [[T0]], align
 // CHECK:   call [[SWIFTCC]] void @take_int8(<4 x i32> [[FIRST]], <4 x i32> [[SECOND]])
 // CHECK:   ret void
 
 TEST(int5)
 // CHECK-LABEL: define {{.*}} @return_int5()
-// CHECK:   [[RET:%.*]] = alloca [[REC:<5 x i32>]], align 32
+// CHECK:   [[RET:%.*]] = alloca [[REC:<5 x i32>]], align 8
 // CHECK:   [[VAR:%.*]] = alloca [[REC]], align
 // CHECK:   store
 // CHECK:   load
 // CHECK:   store
 // CHECK:   [[CAST_TMP:%.*]] = bitcast [[REC]]* [[RET]] to [[AGG:{ <4 x i32>, i32 }]]*
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 0
 // CHECK:   [[FIRST:%.*]] = load <4 x i32>, <4 x i32>* [[T0]], align
 // CHECK:   [[T0:%.*]] = getelementptr inbounds [[AGG]], [[AGG]]* [[CAST_TMP]], i32 0, i32 1

test/CodeGen/armv7k-abi.c

 typedef struct {
   char arr[7];
 } OddlySizedStruct;
 
 // CHECK: define [2 x i32] @return_oddly_sized_struct()
 OddlySizedStruct return_oddly_sized_struct() {}
 
 // CHECK: define <4 x float> @test_va_arg_vec(i8* %l)
-// CHECK:   [[ALIGN_TMP:%.*]] = add i32 {{%.*}}, 15
-// CHECK:   [[ALIGNED:%.*]] = and i32 [[ALIGN_TMP]], -16
+// CHECK:   [[ALIGN_TMP:%.*]] = add i32 {{%.*}}, 7
+// CHECK:   [[ALIGNED:%.*]] = and i32 [[ALIGN_TMP]], -8
 // CHECK:   [[ALIGNED_I8:%.*]] = inttoptr i32 [[ALIGNED]] to i8*
 // CHECK:   [[ALIGNED_VEC:%.*]] = bitcast i8* [[ALIGNED_I8]] to <4 x float>
-// CHECK:   load <4 x float>, <4 x float>* [[ALIGNED_VEC]], align 16
+// CHECK:   load <4 x float>, <4 x float>* [[ALIGNED_VEC]], align 8
 float32x4_t test_va_arg_vec(__builtin_va_list l) {
   return __builtin_va_arg(l, float32x4_t);
 }