[Sparc] Don't overlap variable-sized allocas with other stack variables.
authorJames Y Knight <jyknight@google.com>
Tue, 25 Oct 2016 22:13:28 +0000 (22:13 +0000)
committerJames Y Knight <jyknight@google.com>
Tue, 25 Oct 2016 22:13:28 +0000 (22:13 +0000)
On SparcV8, it was previously the case that a variable-sized alloca
might overlap by 4-bytes the last fixed stack variable, effectively
because 92 (the number of bytes reserved for the register spill area) !=
96 (the offset added to SP for where to start a DYNAMIC_STACKALLOC).

It's not as simple as changing 96 to 92, because variables that should
be 8-byte aligned would then be misaligned.

For now, simply increase the allocation size by 8 bytes for each dynamic
allocation -- wastes space, but at least doesn't overlap. As the large
comment says, doing this more efficiently will require larger changes in
llvm.

Also adds some test cases showing that we continue to not support
dynamic stack allocation and over-alignment in the same function.

llvm-svn: 285131

llvm/lib/Target/Sparc/SparcISelLowering.cpp
llvm/test/CodeGen/SPARC/2013-05-17-CallFrame.ll
llvm/test/CodeGen/SPARC/fail-alloca-align.ll [new file with mode: 0644]
llvm/test/CodeGen/SPARC/stack-align.ll

index d6abb55..95cd201 100644 (file)
@@ -2568,17 +2568,57 @@ static SDValue LowerDYNAMIC_STACKALLOC(SDValue Op, SelectionDAG &DAG,
                                        const SparcSubtarget *Subtarget) {
   SDValue Chain = Op.getOperand(0);  // Legalize the chain.
   SDValue Size  = Op.getOperand(1);  // Legalize the size.
+  unsigned Align = cast<ConstantSDNode>(Op.getOperand(2))->getZExtValue();
+  unsigned StackAlign = Subtarget->getFrameLowering()->getStackAlignment();
   EVT VT = Size->getValueType(0);
   SDLoc dl(Op);
 
+  // TODO: implement over-aligned alloca. (Note: also implies
+  // supporting support for overaligned function frames + dynamic
+  // allocations, at all, which currently isn't supported)
+  if (Align > StackAlign) {
+    const MachineFunction &MF = DAG.getMachineFunction();
+    report_fatal_error("Function \"" + Twine(MF.getName()) + "\": "
+                       "over-aligned dynamic alloca not supported.");
+  }
+
+  // The resultant pointer needs to be above the register spill area
+  // at the bottom of the stack.
+  unsigned regSpillArea;
+  if (Subtarget->is64Bit()) {
+    regSpillArea = 128;
+  } else {
+    // On Sparc32, the size of the spill area is 92. Unfortunately,
+    // that's only 4-byte aligned, not 8-byte aligned (the stack
+    // pointer is 8-byte aligned). So, if the user asked for an 8-byte
+    // aligned dynamic allocation, we actually need to add 96 to the
+    // bottom of the stack, instead of 92, to ensure 8-byte alignment.
+
+    // That also means adding 4 to the size of the allocation --
+    // before applying the 8-byte rounding. Unfortunately, we the
+    // value we get here has already had rounding applied. So, we need
+    // to add 8, instead, wasting a bit more memory.
+
+    // Further, this only actually needs to be done if the required
+    // alignment is > 4, but, we've lost that info by this point, too,
+    // so we always apply it.
+
+    // (An alternative approach would be to always reserve 96 bytes
+    // instead of the required 92, but then we'd waste 4 extra bytes
+    // in every frame, not just those with dynamic stack allocations)
+
+    // TODO: modify code in SelectionDAGBuilder to make this less sad.
+
+    Size = DAG.getNode(ISD::ADD, dl, VT, Size,
+                       DAG.getConstant(8, dl, VT));
+    regSpillArea = 96;
+  }
+
   unsigned SPReg = SP::O6;
   SDValue SP = DAG.getCopyFromReg(Chain, dl, SPReg, VT);
   SDValue NewSP = DAG.getNode(ISD::SUB, dl, VT, SP, Size); // Value
   Chain = DAG.getCopyToReg(SP.getValue(1), dl, SPReg, NewSP);    // Output chain
 
-  // The resultant pointer is actually 16 words from the bottom of the stack,
-  // to provide a register spill area.
-  unsigned regSpillArea = Subtarget->is64Bit() ? 128 : 96;
   regSpillArea += Subtarget->getStackPointerBias();
 
   SDValue NewVal = DAG.getNode(ISD::ADD, dl, VT, NewSP,
index 81f586f..ad92984 100644 (file)
@@ -3,7 +3,13 @@
 
 ; V8-LABEL: variable_alloca_with_adj_call_stack
 ; V8:       save %sp, -96, %sp
-; V8:       add {{.+}}, 96, %o0
+; (this should ideally be doing "add 4+7; and -8", instead of
+;  "add 7; and -8; add 8"; see comments in LowerDYNAMIC_STACKALLOC)
+; V8:       add %i0, 7, %i0
+; V8-NEXT:  and %i0, -8, %i0
+; V8-NEXT:  add %i0, 8, %i0
+; V8-NEXT:  sub %sp, %i0, %i0
+; V8-NEXT:  add %i0, 96, %o0
 ; V8:       add %sp, -16, %sp
 ; V8:       call foo
 ; V8:       add %sp, 16, %sp
@@ -22,5 +28,4 @@ entry:
   ret void
 }
 
-
-declare void @foo(i8* , i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*);
+declare void @foo(i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*);
diff --git a/llvm/test/CodeGen/SPARC/fail-alloca-align.ll b/llvm/test/CodeGen/SPARC/fail-alloca-align.ll
new file mode 100644 (file)
index 0000000..b8d84a9
--- /dev/null
@@ -0,0 +1,23 @@
+;; Sparc backend can't currently handle variable allocas with
+;; alignment greater than the stack alignment.  This code ought to
+;; compile, but doesn't currently.
+
+;; RUN: not llc -march=sparc < %s 2>&1 | FileCheck %s
+;; RUN: not llc -march=sparcv9 < %s 2>&1 | FileCheck %s
+;; CHECK: ERROR: Function {{.*}} required stack re-alignment
+
+define void @variable_alloca_with_overalignment(i32 %num) {
+  %aligned = alloca i32, align 64
+  %var_size = alloca i8, i32 %num, align 4
+  call void @foo(i32* %aligned, i8* %var_size)
+  ret void
+}
+
+;; Same but with the alloca itself overaligned
+define void @variable_alloca_with_overalignment_2(i32 %num) {
+  %var_size = alloca i8, i32 %num, align 64
+  call void @foo(i32* null, i8* %var_size)
+  ret void
+}
+
+declare void @foo(i32*, i8*);
index 2554ee8..b152e6a 100644 (file)
@@ -1,12 +1,10 @@
 ; RUN: llc -march=sparc < %s | FileCheck %s
 declare void @stack_realign_helper(i32 %a, i32* %b)
 
-@foo = global i32 1
-
 ;; This is a function where we have a local variable of 64-byte
 ;; alignment.  We want to see that the stack is aligned (the initial
-;; andn), that the local var is accessed via stack pointer (to %o0), and that
-;; the argument is accessed via frame pointer not stack pointer (to %o1).
+;; andn), that the local var is accessed via stack pointer (to %o1), and that
+;; the argument is accessed via frame pointer not stack pointer (to %o0).
 
 ;; CHECK-LABEL: stack_realign:
 ;; CHECK:      andn %sp, 63, %sp