[PATCH] D114457: [z/OS] Implement prologue and epilogue generation for z/OS target.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:884
     Spills.push_back(CalleeSavedInfo(RegSP));
+    CSI.push_back(CalleeSavedInfo(RegSP));
+  }
----------------
As discussed when this function was added initially, this routine should not modify it's CSI argument.  If we want to add the SP to the general set of spilled and restored registers, this should be done in determineCalleeSaves instead.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:1178
+  const SystemZSubtarget &Subtarget = MF.getSubtarget<SystemZSubtarget>();
+  const TargetRegisterInfo *RI = Subtarget.getRegisterInfo();
+
----------------
Why do we need to override the default implementation here?   That shouldn't be necessary if we've set everything else up correctly.


================
Comment at: llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp:1190
+
+  // Arguments are located in the parent's frame. To identify this situation it
+  // is enough to check if the frame index refers to a fixed object.
----------------
In particular, this looks quite wrong.   It's true with most ABIs that arguments are in the parent's frame, but that should have been handled elsewhere, it shouldn't require special code in this routine.


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

https://reviews.llvm.org/D114457