[PATCH] D11470: [ARM] Define subtarget feature "strict-align"

[JF Bastien]

jfb added inline comments.

================
Comment at: test/CodeGen/ARM/2011-10-26-memset-inline.ll:2
@@ -1,3 +1,3 @@
 ; Make sure short memsets on ARM lower to stores, even when optimizing for size.
-; RUN: llc -march=arm < %s | FileCheck %s -check-prefix=CHECK-GENERIC
+; RUN: llc -march=arm -mattr=+strict-align < %s | FileCheck %s -check-prefix=CHECK-GENERIC
 ; RUN: llc -march=arm -mcpu=cortex-a8 < %s | FileCheck %s -check-prefix=CHECK-UNALIGNED
----------------
ahatanak wrote:
> jfb wrote:
> > Doesn't this test's triple put it in the `hasV6Ops() && isTargetMachO()` category, which supports unaligned memory accesses?
> I'm not sure why it is this way, but it looks like thumbv7 triple doesn't result in HasV7Ops being set to true (see function ARM_MC::ParseARMTriple).
So this was a bug in the old code?

Does you clang change "fix" this bug?

================
Comment at: test/CodeGen/ARM/build-attributes.ll:62
@@ -61,2 +61,3 @@
 ; RUN: llc < %s -mtriple=armv7-linux-gnueabi -mcpu=cortex-a17 -enable-sign-dependent-rounding-fp-math | FileCheck %s --check-prefix=DYN-ROUNDING
 ; RUN: llc < %s -mtriple=thumbv6m-linux-gnueabi -mcpu=cortex-m0 | FileCheck %s --check-prefix=CORTEX-M0
+; RUN: llc < %s -mtriple=thumbv6m-linux-gnueabi -mcpu=cortex-m0 -mattr=+strict-align  -enable-unsafe-fp-math -disable-fp-elim -enable-no-infs-fp-math -enable-no-nans-fp-math -fp-contract=fast | FileCheck %s --check-prefix=CORTEX-M0-FAST
----------------
ahatanak wrote:
> jfb wrote:
> > This v6m needs strict-align?
> Don't we need strict-align since no v6M cores support unaligned memory access?
IIUC you should add `-mattr=+strict-align` here. After your fix clang will do it automatically, but llc won't so you need the flag.

================
Comment at: test/CodeGen/ARM/build-attributes.ll:129
@@ -130,1 +128,3 @@
+; RUN: llc < %s -mtriple=armv8.1a-none-linux-gnueabi | FileCheck %s --check-prefix=NO-STRICT-ALIGN
+; RUN: llc < %s -mtriple=armv8.1a-none-linux-gnueabi -mattr=+strict-align | FileCheck %s --check-prefix=STRICT-ALIGN
 ; RUN: llc < %s -mtriple=armv8.1a-none-linux-gnueabi | FileCheck %s --check-prefix=NO-STRICT-ALIGN
----------------
ahatanak wrote:
> jfb wrote:
> > The current code didn't affect v8, so this seems wrong. I think it should fix v8 to use the same flags, though.
> For the first test, I removed -arm-no-strict-align because v8 does allow unaligned access, and for the second test, I replaced -arm-strict-align with -mattr=+strict-align.
The llc flag `-arm-no-strict-align` and `-arm-strict-align` don't do anything for v8.
With your current code, `-mattr=+strict-align` also doesn't do anything for v8.

v8 does have `-aarch64-strict-align` and `-aarch64-no-strict-align`, which I suggested you change to be consistent with the ARM change.

This line went from being misguided to being differently misguided. I'd rather fix v8 to act the same as ARM :-)


http://reviews.llvm.org/D11470