[PATCH] D70427: [SystemZ] The Local Area Offset in SystemZFrameLowering.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 230216.
jonpa retitled this revision from "[SystemZ]  Stop using the Local Area Offset in SystemZFrameLowering." to "[SystemZ]  The Local Area Offset in SystemZFrameLowering.".
jonpa added a comment.

Thank you very much for the explanation - it does make more sense now :-)

I think it would be nice to have this mentioned in a comment, so I tried to reuse part of your answer here in the SystemZFrameLowering() constructor.

The one part that stands out from this then to me, is the fact that the SpillOffsetTable is relative to the incoming SP, which is why the offsets are incorrect in the created stack objects. To have a fully correct IR, I guess we should return offsets relative to the CFA, in other words subtract 160 from each entry. But there is no real point in this since those offsets are never used (we compute the offset for the STMG instead), so I instead figured that at least having a comment mentioning that might be helpful.

Unfortunately I was still left with a slight confusion, as I am not quite sure how this works for the FPRs part of that table: Normally those FPRs are not callee-saved, so it wouldn't matter. But how about the CSR_SystemZ_AllRegs calling convention? If F0D would be callee-saved, would it end up in the right place?


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

https://reviews.llvm.org/D70427

Files:
  llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp


Index: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
===================================================================
--- llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -49,6 +49,11 @@
     : TargetFrameLowering(TargetFrameLowering::StackGrowsDown, Align(8),
                           -SystemZMC::CallFrameSize, Align(8),
                           false /* StackRealignable */) {
+  // Due to the SystemZ ABI, the DWARF CFA (Canonical Frame Address) is not
+  // equal to the incoming stack pointer, but to incoming stack pointer plus
+  // 160.  The getOffsetOfLocalArea() returned value is interpreted as "the
+  // offset of the local area from the CFA".
+
   // Create a mapping from register number to save slot offset.
   RegSpillOffsets.grow(SystemZ::NUM_TARGET_REGS);
   for (unsigned I = 0, E = array_lengthof(SpillOffsetTable); I != E; ++I)
@@ -57,6 +62,11 @@
 
 const TargetFrameLowering::SpillSlot *
 SystemZFrameLowering::getCalleeSavedSpillSlots(unsigned &NumEntries) const {
+  // The offsets in SpillOffsetTable are relative to incoming stack pointer
+  // and are therefore incorrect here since we have defined the Local Area
+  // Offset as -SystemZMC::CallFrameSize. This however does not matter since
+  // the created frame object offsets are not used as we build the STMG/LMG
+  // instructions.
   NumEntries = array_lengthof(SpillOffsetTable);
   return SpillOffsetTable;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70427.230216.patch
Type: text/x-patch
Size: 1465 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191120/f860e5f9/attachment.bin>