[SPARC] Fix stupid oversight in stack realignment support.

[jyknight]

Summary

If you're going to realign %sp to get object alignment properly (which
the code does), and stack offsets and alignments are calculated going
down from %fp (which they are), then the total stack size had better
be a multiple of the alignment. LLVM did indeed ensure that.

And then, after aligning, the sparc frame code added 96 (for sparcv8)
to the frame size, making any requested alignment of 64-bytes or
higher *guaranteed* to be misaligned. The test case added with r245668
even tests this exact scenario, and asserted the incorrect behavior,
which I somehow failed to notice. D'oh.

This change fixes the frame lowering code to align the stack size
*after* adding the spill area, instead.

[chandlerc]

Looks good with a comment update below.

lib/Target/Sparc/SparcFrameLowering.cpp

121–123

I wouldn't reference the hook here. Instead, I would explain the why. Clearly (according to the spec of the API) to do this here your target has to set the hook correctly. But why does SPARC handle alignment this way.

lib/Target/Sparc/SparcFrameLowering.h

public:
 
   bool hasReservedCallFrame(const MachineFunction &MF) const override;
   bool hasFP(const MachineFunction &MF) const override;
   void determineCalleeSaves(MachineFunction &MF, BitVector &SavedRegs,
                             RegScavenger *RS = nullptr) const override;
 
   int getFrameIndexReference(const MachineFunction &MF, int FI,
                              unsigned &FrameReg) const override;
+
+  /// targetHandlesStackFrameRounding - Returns true if the target is
+  /// responsible for rounding up the stack frame (probably at emitPrologue
+  /// time).
+  bool targetHandlesStackFrameRounding() const override { return true; }
+
 private:
   // Remap input registers to output registers for leaf procedure.
   void remapRegsForLeafProc(MachineFunction &MF) const;
 
   // Returns true if MF is a leaf procedure.
   bool isLeafProc(MachineFunction &MF) const;
 
 

lib/Target/Sparc/SparcFrameLowering.cpp

void SparcFrameLowering::emitPrologue(MachineFunction &MF,
   unsigned SAVErr = SP::SAVErr;
   if (FuncInfo->isLeafProc()) {
     if (NumBytes == 0)
       return;
     SAVEri = SP::ADDri;
     SAVErr = SP::ADDrr;
   }
 
+  // Add the extra call frame stack size, if needed (this would've
+  // been done by PrologEpilogInserter.cpp, except that we have
+  // targetHandlesStackFrameRounding returning true.)
+  if (MFI->adjustsStack() && hasReservedCallFrame(MF))
+    NumBytes += MFI->getMaxCallFrameSize();
+
+  // Adds the subtarget-specific spill area to the stack size. Also
+  // ensures target-required alignment.
   NumBytes = MF.getSubtarget<SparcSubtarget>().getAdjustedFrameSize(NumBytes);
-  MFI->setStackSize(NumBytes); // Update stack size with corrected value.
+  // Ensure that the size is also sufficiently aligned for the data on the stack.
+  if (MFI->getMaxAlignment() > 0) {
+    NumBytes = RoundUpToAlignment(NumBytes, MFI->getMaxAlignment());
+  }
+  // Update stack size with corrected value.
+  MFI->setStackSize(NumBytes);
 
   emitSPAdjustment(MF, MBB, MBBI, -NumBytes, SAVErr, SAVEri);
 
   MachineModuleInfo &MMI = MF.getMMI();
   unsigned regFP = RegInfo.getDwarfRegNum(SP::I6, true);
 
   // Emit ".cfi_def_cfa_register 30".
   unsigned CFIIndex =

lib/Target/Sparc/SparcSubtarget.cpp

 int SparcSubtarget::getAdjustedFrameSize(int frameSize) const {
 
   if (is64Bit()) {
     // All 64-bit stack frames must be 16-byte aligned, and must reserve space
     // for spilling the 16 window registers at %sp+BIAS..%sp+BIAS+128.
     frameSize += 128;
     // Frames with calls must also reserve space for 6 outgoing arguments
     // whether they are used or not. LowerCall_64 takes care of that.
-    assert(frameSize % 16 == 0 && "Stack size not 16-byte aligned");
+    frameSize = RoundUpToAlignment(frameSize, 16);
   } else {
     // Emit the correct save instruction based on the number of bytes in
     // the frame. Minimum stack frame size according to V8 ABI is:
     //   16 words for register window spill
     //    1 word for address of returned aggregate-value
     // +  6 words for passing parameters on the stack
     // ----------
     //   23 words * 4 bytes per word = 92 bytes
     frameSize += 92;
 
     // Round up to next doubleword boundary -- a double-word boundary
     // is required by the ABI.
     frameSize = RoundUpToAlignment(frameSize, 8);
   }
   return frameSize;
 }

test/CodeGen/SPARC/stack-align.ll

 ; 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).
 
 ;; CHECK-LABEL: stack_realign:
 ;; CHECK:      andn %sp, 63, %sp
 ;; CHECK-NEXT: ld [%fp+92], %o0
 ;; CHECK-NEXT: call stack_realign_helper
-;; CHECK-NEXT: add %sp, 96, %o1
+;; CHECK-NEXT: add %sp, 128, %o1
 
 define void @stack_realign(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g) {
 entry:
   %aligned = alloca i32, align 64
   call void @stack_realign_helper(i32 %g, i32* %aligned)
   ret void
 }