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

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

I don't think this approach will really do everything -mpacked-stack is supposed to do.  Fundamentally, with the packed stack layout, only some N (< 160) bytes of the incoming register save area are reserved for register save slots, instead of all 160 bytes.  Everything **below** "(incoming SP) + 160 - N" is supposed to be freely usable for **any** use, including FPR/VR save slots and outgoing function args / register save area, but also just local stack variables and reload spill slots in general.

I don't see where in this patch you actually make this area available for this type of general allocation.  I believe the fundamental problem is that you still set the TargetFrameLowering's "LocalAreaOffset" to -160, which still makes all those bytes reserved.  You apparently try to still force in some of the special areas (like FPR spill slots or the outgoing register save area), but this doesn't address the general case.  (Conversely, if the general case **were** addressed, you actually wouldn't have to implement anything special for those special cases.

So ideally, you'd want to set LocalAreaOffset to -N (where N depends on the number of GPRs to be saved) instead of -160 for the packed stack layout.  Unfortunately, I believe the LocalAreaOffset mechanism doesn't support a variable value for this field.   One option would be to find (or create) some way to instantiate a per-function FrameLowering.  Another way might be to just always set the LocalAreaOffset to 0 and then force common code to still leave the register save area fully (non-packed layout) or partially (packed layout) empty by placing fixed frame objects there (cf. the logic in PEI::calculateFrameObjectOffsets).


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

https://reviews.llvm.org/D70821