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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa marked an inline comment as done.
jonpa added a comment.

In D70821#1774071 <https://reviews.llvm.org/D70821#1774071>, @uweigand wrote:

> > 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?).


That's because now that they are fixed they don't get considered for HasStackObject, and even though the offset for F8 <https://reviews.llvm.org/F8> is the same from CFA, since the outgoing reg save area is in these cases not allocated, the final SP offset will be 160 less.

> 
> 
>> 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.

Ah, seems like a good idea...



================
Comment at: llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h:30
   unsigned NumLocalDynamics;
+  unsigned LowGPROffset;
 
----------------
uweigand wrote:
> 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?
I am not sure there is an easy way to do this... It seems to me then we would have to loop over the frame objects and find the reg. And since it is needed in two different functions that should be a function, so I figured it might be simpler to just save the value like this. Or would it be better to write the loop in both functions?



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

https://reviews.llvm.org/D70821