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

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

> This changes a few test cases, since e.g. F8 <https://reviews.llvm.org/F8> is now saved on a fixed stack slot instead of a non-fixed one as was done previously.

I'm not quite sure why this should be the case: F8 <https://reviews.llvm.org/F8> should in fact still be saved in the same location (even though LLVM now considers it a fixed slot, it'll still end up at the same location, right?).

> Don't use the offset table with packed stack, instead rely on register numbers. Assert added to check that this is really ok.

Instead of relying on LLVM register numbers, it would be better to use SystemZMC::getFirstReg to map the GPR number to an integer in the range 0..15, and then just use that.  Then you don't need the assert either.



================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:478
+    }
+  if (HasStackObject || MFFrame.hasVarSizedObjects() || MFFrame.hasCalls())
     StackSize += SystemZMC::CallFrameSize;
----------------
The hasVarSizedObjects check is probably unnecessary now, since if there are any variable-sized stack objects, HasStackObject will now already be true.


================
Comment at: llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h:30
   unsigned NumLocalDynamics;
+  unsigned LowGPROffset;
 
----------------
This seems another bit of double bookkeeping.  Can't users simply look up the offset of the frame object associated with LowSavedGPR to determine this?


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

https://reviews.llvm.org/D70821