[ARM] Fix Neon vector type alignment to 64-bit

[javed.absar]

Summary

This is restricted version of patch - https://reviews.llvm.org/D33205
that I reverted as it was leading to ABI breaks on darwin etc.
This patch restricts the fix to AAPCS only.

[srhines]

test/CodeGen/neon-aapcs-align.c

9 ↗

(On Diff #101042)

Can you make sure that an Android triple doesn't trigger this? I worry because I am not sure what getABI() will return in that case.

[javed.absar]

test/CodeGen/neon-aapcs-align.c

9 ↗

(On Diff #101042)

Could you please elaborate.There is a test already 'armv7-none-linux-androideabi'.

lib/Basic/Targets.cpp:5357:: case llvm::Triple::Android:
sets it to aapcs-linux

[srhines]

LGTM

test/CodeGen/neon-aapcs-align.c

9 ↗

(On Diff #101042)

Yes, but you are also passing -target-abi here. I wanted confirmation that this will always be passed on the cc1 line. I think the line from Targets.cpp confirms that this is going to be the case (unless someone explicitly passes a different -target-abi option to cc1. Thanks.

[javed.absar]

Thanks Stephen.
Vedant, Diana : Does this look ok to you as well?

[rovka]

It certainly seems more restrictive than the previous patch, so I think it should be ok. @rengolin probably understands the subtleties here a bit better than I do.

[javed.absar]

Hi Renato. Please could you have a look at this. I had a LGTM from @rovka and @srhines but need final one from you.
Thanks, Javed.

[rengolin]

test/CodeGen/neon-aapcs-align.c

9 ↗

(On Diff #101042)

This is odd. On Android alone, it should be 128. In anything + aapcs, it should be 64, including Android. Explicit behaviour trumps default behaviour.

Do we have tests to make sure that the right CC1 flags are passed down?

We need two types of tests here: default behaviour and overriden behaviour.

I'd also add a GNUEABI and FreeBSD lines, because they're sufficiently different from the ones above.

[javed.absar]

Hi Renato:
I have made changes as suggested (unless I got somethings wrong).

1. Now, except for android, AAPCS => 64-bit alignment
2. The fix relies only on the triple for I am not sure if 'right cc1' flag test is needed.
3. Added gnueabi and FreeBSD as suggested.

Please have a look. Thanks.
--Javed

[rengolin]

@srhines: Do you need Android to *always* be 64-bit, even if someone forces aapcs with it?

[srhines]

In D33786#774089, @rengolin wrote:

@srhines: Do you need Android to *always* be 64-bit, even if someone forces aapcs with it?

128-bit always. We don't support "multiple" ABIs. There is only one, and it makes sense to not allow people to choose something that can't work.

[rengolin]

Thanks @srhines @javed.absar, LGTM!

cfe/trunk/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();
 
+    // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
+    if (IsAAPCS && (Triple.getEnvironment() != llvm::Triple::Android))
+       MaxVectorAlign = 64;
+
     // 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)

cfe/trunk/test/CodeGen/neon-aapcs-align.c

+// RUN: %clang_cc1 -triple armv7a-none-eabi -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=AAPCS
+// RUN: %clang_cc1 -triple armv7a-none-gnueabi -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=AAPCS
+// RUN: %clang_cc1 -triple armv7a-none-freebsd -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=AAPCS
+
+// RUN: %clang_cc1 -triple armv7a-apple-ios -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=DEFAULT
+// RUN: %clang_cc1 -triple armv7a-none-android -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=DEFAULT
+// RUN: %clang_cc1 -triple armv7a-none-androideabi -target-feature +neon -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=DEFAULT
+
+#include <arm_neon.h>
+// Neon types have 64-bit alignment
+int32x4_t gl_b;
+void t3(int32x4_t *src) {
+// CHECK: @t3
+  gl_b = *src;
+// AAPCS: store <4 x i32> {{%.*}}, <4 x i32>* @gl_b, align 8
+// DEFAULT: store <4 x i32> {{%.*}}, <4 x i32>* @gl_b, align 16
+}