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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:51
+                          0, Align(8), false /* StackRealignable */),
+      RegSpillOffsets(0) {
   // Due to the SystemZ ABI, the DWARF CFA (Canonical Frame Address) is not
----------------
uweigand wrote:
> Why is the RegSpillOffsets change above needed?
I'm not sure this is needed, but since I am using this table also with CSRs that are not in it, I wanted to make sure that zero is returned in those cases.

IIUC, nullVal_ will by this be initialized to 0, which will then be the initial value of all the elements. Perhaps this is redundant with a default constructor for unsigned?


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:81
+    unsigned Reg = CS.getReg();
+    int Offset = RegSpillOffsets[Reg];
+    if (Offset) {
----------------
uweigand wrote:
> The RegSpillOffsets value is only ever valid for the non-packed case, so I think everything would be clearer if you only looked at it in that case.
OK - I suppose that if a table isn't needed, then the register numbers themselves can be used to determine the LowGPR and the offsets...?


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

https://reviews.llvm.org/D70821