[PATCH] D45770: [AArch64] Disable spill slot scavenging when stack realignment required.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm created this revision.
paulwalker-arm added a reviewer: gberry.
Herald added subscribers: llvm-commits, kristof.beyls, javed.absar, qcolombet, rengolin.

Prevent corruption of the callee-save region by spills whose offset calculation assumes FP-SP is a compile time constant.


Repository:
  rL LLVM

https://reviews.llvm.org/D45770

Files:
  lib/Target/AArch64/AArch64FrameLowering.cpp
  test/CodeGen/AArch64/spill-stack-realignment.mir


Index: test/CodeGen/AArch64/spill-stack-realignment.mir
===================================================================
--- /dev/null
+++ test/CodeGen/AArch64/spill-stack-realignment.mir
@@ -0,0 +1,53 @@
+# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass=prologepilog %s -o - | FileCheck %s
+
+# Ensure we don't perform stack slot scavenging when the stack pointer must be
+# aligned to something larger than required by the target. Allowing scavenging
+# can result in the callee-save region being corrupted when FP is used to build
+# the callee-save area and SP used to spill into it.  This is becuase the value
+# of FP-SP becomes runtime variable after the alignment.
+--- |
+
+  define void @test(float* %p) nounwind {
+  entry:
+    %large_aligned_var = alloca float, align 64
+    %small_aligned_var = alloca float, align 4
+    %s0 = load float, float* %p, align 4
+    store volatile float %s0, float* %large_aligned_var, align 4
+    store volatile float %s0, float* %small_aligned_var, align 4
+    call void asm sideeffect "; inlineasm", "~{x28}"()
+    ret void
+  }
+
+...
+---
+name:            test
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    64
+# CHECK:      stack:
+# CHECK:        name: large_aligned_var, type: default, offset: -64, size: 4,
+# CHECK-NEXT:     alignment: 64
+# CHECK-NEXT:     local-offset: -64
+# CHECK:        name: small_aligned_var, type: default, offset: -68, size: 4,
+# CHECK-NEXT:     alignment: 4
+# CHECK-NEXT:     local-offset: -68
+stack:
+  - { id: 0, name: large_aligned_var, size: 4, alignment: 64, local-offset: -64 }
+  - { id: 1, name: small_aligned_var, size: 4, alignment: 4, local-offset: -68 }
+
+# CHECK: body:
+# CHECK:   $sp = ANDXri killed ${{x[0-9]+}}, 7865
+# CHECK:   STRSui $s0, $sp, 16
+# CHECK:   STRSui killed $s0, $sp, 15
+body:             |
+  bb.0.entry:
+    liveins: $x0
+
+    $s0 = LDRSui killed $x0, 0 :: (load 4 from %ir.p)
+    STRSui $s0, %stack.0.large_aligned_var, 0 :: (volatile store 4 into %ir.large_aligned_var, align 64)
+    STRSui killed $s0, %stack.1.small_aligned_var, 0 :: (volatile store 4 into %ir.small_aligned_var)
+    ; Force preserve a CSR to create a hole in the CSR stack region.
+    INLINEASM &"; inlineasm", 1, 12, implicit-def dead early-clobber $x28
+    RET_ReallyLR
+
+...
Index: lib/Target/AArch64/AArch64FrameLowering.cpp
===================================================================
--- lib/Target/AArch64/AArch64FrameLowering.cpp
+++ lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -1499,8 +1499,21 @@
   AFI->setCalleeSavedStackSize(alignTo(8 * NumRegsSpilled, 16));
 }
 
+// When stack realignment is required the size of the stack frame becomes
+// runtime variable. This manifests itself as a runtime variable gap between the
+// local and callee-save regions. If FP is used to build the callee-save region
+// then FP must be used to access any locals placed within it. This does not
+// happen today causing corruption to callee saves by SP relative stores whose
+// offset is calculated assuming FP-SP is a compile time constant.
+//
+// An option is to force all callee-save region accesses to be relative to the
+// same base (SP or FP) but given the alignment is likely to reduce the benefit
+// of slot scavenging it's simpler to disable the optimisation.
 bool AArch64FrameLowering::enableStackSlotScavenging(
     const MachineFunction &MF) const {
   const AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
-  return AFI->hasCalleeSaveStackFreeSpace();
+  const AArch64RegisterInfo *RegInfo = static_cast<const AArch64RegisterInfo *>(
+      MF.getSubtarget().getRegisterInfo());
+  return AFI->hasCalleeSaveStackFreeSpace() &&
+         !RegInfo->needsStackRealignment(MF);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45770.142928.patch
Type: text/x-patch
Size: 3745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180418/234e794f/attachment.bin>