LLVM targeting aarch64 doesn't correctly produce aligned accesses for non-aligned data at -O0/fast-isel (-mno-unaligned-access)

[labrinea]

Summary

test.c228 BDownload

Clang doesn't correctly generate loads in the presence of the -mno-unaligned-access flag. At -O0 it produces:
$ clang --target=aarch64-arm-none-eabi -mno-unaligned-access -O0 -S test.c -o-

foo:
	stp	x29, x30, [sp, #-16]!
	mov	 x29, sp
	sub	sp, sp, #32             // =32
	adrp	x8, a1
	add	x8, x8, :lo12:a1
	movz	x2, #0x5
	sub	x9, x29, #8             // =8
	add	x10, sp, #16            // =16
	str	x0, [sp, #16]
	mov	 x0, x9
	mov	 x1, x10
	str	x8, [sp, #8]            // 8-byte Folded Spill
	bl	memcpy
	ldur	w11, [x29, #-7]
	ldr	x8, [sp, #8]            // 8-byte Folded Reload
	ldur	w12, [x8, #1]
	add	 w11, w12, w11
	stur	w11, [x8, #1]
	mov	 w0, w11
	mov	 sp, x29
	ldp	x29, x30, [sp], #16
	ret

At -O1 and above, it produces correct code, using ldrb's to access to non-aligned data:
clang --target=aarch64-arm-none-eabi -mno-unaligned-access -O1 -S test.c -o-

foo:
        adrp	x8, a1
	add	x8, x8, :lo12:a1
	ldrb	w9, [x8, #1]!
	ldrb	w10, [x8, #3]
	ldrb	w11, [x8, #2]
	ldrb	w12, [x8, #1]
	bfi	w11, w10, #8, #8
	lsr	x10, x0, #8
	bfi	w9, w12, #8, #8
	bfi	w9, w11, #16, #16
	add	 w0, w9, w10
	strb	 w0, [x8]
	lsr	w9, w0, #24
	lsr	w10, w0, #16
	lsr	w11, w0, #8
	strb	w9, [x8, #3]
	strb	w10, [x8, #2]
	strb	w11, [x8, #1]
	ret

The root cause seems to be in fast-isel not producing unaligned access correctly for -mno-unaligned-access:
clang --target=aarch64-arm-none-eabi -mno-unaligned-access -O1 -mllvm -fast-isel -S test.c -o-

foo:
        adrp	x8, a1
	add	x8, x8, :lo12:a1
	ldur	w9, [x8, #1]
	lsr	x10, x0, #8
	add	 w0, w9, w10
	stur	w0, [x8, #1]
	ret

[mcrosier]

Adding Juergen as he might want to fix this in a less conservative way.

[mcrosier]

test/CodeGen/AArch64/arm64-strict-align.ll

3 ↗

(On Diff #27443)

Adding -fast-isel to the run line means we're no longer testing the ISelDAG path. Please create a new RUN line for fast-isel.

[labrinea]

Created a new RUN line for fast-isel.

[mcrosier]

Please address the comments about the test case and then commit.

test/CodeGen/AArch64/arm64-strict-align.ll

4 ↗

(On Diff #27459)

Please use the "CHECK-STRICT" prefix; the output is exactly the same for fast-isel and non-fast-isel. No need for the redundancy.

[labrinea]

Removed redundant label from regression test.

[mcrosier]

LGTM. Feel free to commit.

llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp

 
 unsigned AArch64FastISel::emitAnd_ri(MVT RetVT, unsigned LHSReg, bool LHSIsKill,
                                      uint64_t Imm) {
   return emitLogicalOp_ri(ISD::AND, RetVT, LHSReg, LHSIsKill, Imm);
 }
 
 unsigned AArch64FastISel::emitLoad(MVT VT, MVT RetVT, Address Addr,
                                    bool WantZExt, MachineMemOperand *MMO) {
+  if(!TLI.allowsMisalignedMemoryAccesses(VT))
+    return 0;
+
   // Simplify this down to something we can handle.
   if (!simplifyAddress(Addr, VT))
     return 0;
 
   unsigned ScaleFactor = getImplicitScaleFactor(VT);
   if (!ScaleFactor)
     llvm_unreachable("Unexpected value type.");
 
bool AArch64FastISel::selectLoad(const Instruction *I) {
   }
 
   updateValueMap(I, ResultReg);
   return true;
 }
 
 bool AArch64FastISel::emitStore(MVT VT, unsigned SrcReg, Address Addr,
                                 MachineMemOperand *MMO) {
+  if(!TLI.allowsMisalignedMemoryAccesses(VT))
+    return false;
+
   // Simplify this down to something we can handle.
   if (!simplifyAddress(Addr, VT))
     return false;
 
   unsigned ScaleFactor = getImplicitScaleFactor(VT);
   if (!ScaleFactor)
     llvm_unreachable("Unexpected value type.");
 

llvm/trunk/test/CodeGen/AArch64/arm64-strict-align.ll

 ; RUN: llc < %s -mtriple=arm64-apple-darwin | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-apple-darwin -aarch64-no-strict-align | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-apple-darwin -aarch64-strict-align | FileCheck %s --check-prefix=CHECK-STRICT
+; RUN: llc < %s -mtriple=arm64-apple-darwin -aarch64-strict-align -fast-isel | FileCheck %s --check-prefix=CHECK-STRICT
 
 define i32 @f0(i32* nocapture %p) nounwind {
 ; CHECK-STRICT: ldrh [[HIGH:w[0-9]+]], [x0, #2]
 ; CHECK-STRICT: ldrh [[LOW:w[0-9]+]], [x0]
 ; CHECK-STRICT: bfi [[LOW]], [[HIGH]], #16, #16
 ; CHECK-STRICT: ret
 
 ; CHECK: ldr w0, [x0]