[PATCH] D70821: [SystemZ] Implement the packed stack layout

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 231832.
jonpa added a comment.

This version of the patch is the experiment of setting the Local Area Offset to 0, and then creating fixed stack objects to represent the incoming register save area. This means that we are pretending that incoming SP and CFA are the same, and debug dumps show [SP - ...] where SP really is CFA. In order to make this work, the offset between the incoming SP and the CFA now has to be added in SystemZFrameLowering::getFrameIndexReference() instead. I am not sure if this is simpler or not compared to the previous version...

One question I have is about the decision as to when to create of the outgoing Reg Save Area. It seems that on trunk, this is to be done if any stack space is needed outside the incoming register save area. I am a little unsure about the reasoning here, as the only reason I can think of to allocate it in this case is in the case of an exception in which case that routine would need the incoming reg save area. But if we are already using our incoming reg save area, then those registers would be overwritten... I so far just mimic this current behavior and allocate the outgoing reg save area only when CSRs stack space usage includes registers that would not go into the incoming reg save area.

- In SystemZFrameLowering::determineCalleeSaves() the incoming use of the full reg save area in is always added for the default stack layout.

- Implement SystemZFrameLowering::assignCalleeSavedSpillSlots() (instead of getCalleeSavedSpillSlots()):
  - Create fixed stack objects for those registers that do not go into the Register Save Area, in the same way as currently is the result of the common code (CSR saves topmost).
  - Remember the RegSaveAreaUsage() with packed stack. This is used later when deciding if to allocate a new Reg save area.

- processFunctionBeforeFrameFinalized() should now be correct but it is not giving the exact same results on SPEC. In one case it looked like trunk was adding too much, while being conservatively correct:

  # Machine code for function scan_one_insn: NoPHIs, TracksLiveness, NoVRegs
  Frame Objects:
    fi#-3: size=8, align=8, fixed, at location [SP+120]
    fi#-2: size=8, align=8, fixed, at location [SP+112]
    fi#-1: size=8, align=8, fixed, at location [SP+104]
    fi#0: size=240, align=8, at location [SP+160]
    fi#1: size=120, align=4, at location [SP+160]
    fi#2: size=240, align=8, at location [SP+160]
    fi#3: size=3120, align=4, at location [SP+160]
  
  (gdb) p StackSize
  $519 = 3936
  (gdb) p MaxArgOffset
  $520 = 160
  (gdb) p MaxReach
  $521 = 4096

StackSize + CallFrameSize = 3776 + 160 = 3936
MFFrame.estimateStackSize(MF) adds 56 to the stack size based on the -56 offset for %R13 <https://reviews.llvm.org/source/pstl/>. This shouldn't be done, since CallFrameSize is added later...

The patch seems to be NFC except for a few cases were spill emergency slots are no longer allocated, per above.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70821/new/

https://reviews.llvm.org/D70821

Files:
  llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.h
  llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
  llvm/lib/Target/SystemZ/SystemZFrameLowering.h
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h
  llvm/test/CodeGen/SystemZ/frame-22.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70821.231832.patch
Type: text/x-patch
Size: 19086 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191203/0dae93dd/attachment.bin>