[SystemZ] Implement the packed stack layout

[jonpa]

Summary

(In progress)

• Store GPRs topmost in the Register Save Area, and then any FP arg regs below them. Make sure to update the offset for each register.

• Do not always increase the stack size with CallFrameSize, but instead with the amount used of the Register save area.

• Extend getFrameIndexReference() to increase all offsets into the local area so that in effect the Local area now begins below the last used byte of the register save area.

Command line option not yet implemented - I suppose this could be done the same way as with -mnop-mcount?

[uweigand]

I don't think this approach will really do everything -mpacked-stack is supposed to do. Fundamentally, with the packed stack layout, only some N (< 160) bytes of the incoming register save area are reserved for register save slots, instead of all 160 bytes. Everything below "(incoming SP) + 160 - N" is supposed to be freely usable for any use, including FPR/VR save slots and outgoing function args / register save area, but also just local stack variables and reload spill slots in general.

I don't see where in this patch you actually make this area available for this type of general allocation. I believe the fundamental problem is that you still set the TargetFrameLowering's "LocalAreaOffset" to -160, which still makes all those bytes reserved. You apparently try to still force in some of the special areas (like FPR spill slots or the outgoing register save area), but this doesn't address the general case. (Conversely, if the general case were addressed, you actually wouldn't have to implement anything special for those special cases.

So ideally, you'd want to set LocalAreaOffset to -N (where N depends on the number of GPRs to be saved) instead of -160 for the packed stack layout. Unfortunately, I believe the LocalAreaOffset mechanism doesn't support a variable value for this field. One option would be to find (or create) some way to instantiate a per-function FrameLowering. Another way might be to just always set the LocalAreaOffset to 0 and then force common code to still leave the register save area fully (non-packed layout) or partially (packed layout) empty by placing fixed frame objects there (cf. the logic in PEI::calculateFrameObjectOffsets).

[jonpa]

I don't think this approach will really do everything -mpacked-stack is supposed to do. Fundamentally, with the packed stack layout, only some N (< 160) bytes of the incoming register save area are reserved for register save slots, instead of all 160 bytes. Everything below "(incoming SP) + 160 - N" is supposed to be freely usable for any use, including FPR/VR save slots and outgoing function args / register save area, but also just local stack variables and reload spill slots in general.

I think it does achieve that... The local area offset is not changed, but my intention is to instead move up all the slots in the local area with the amount that was packed:

• First, the GPRs are stored topmost and then any (anyregcc) FP arg regs below them in the reg save area. The amount used of the reg save area is tracked during this with ZFI->incRegSaveAreaUsage(). So any registers which already belong in the reg save area still go there, but with new offets. The amount used is then not 160, but ZFI->getRegSaveAreaUsage().

• The stack is then grown by this amount instead of CallFrameSize. This means that we should have just enough space to pass over to callee.

• When Frame indexes are replaced, getFrameIndexReference() still returns the right offsets for all the fixed objects (incoming stack args and regs in the reg save area), since the stack size is part of that computation. The outgoing stack arguments do not have any frame objects, they are hard coded to begin at SP+160 at the call site, so the remaining objects are those that are non-fixed and all now ordinarily begin at CFA-160 (Local Area Offset). So by simply moving up any of these non-fixed objects by the amount that was packed/saved in the newly derived SystemZFrameLowering::getFrameIndexReference(), we end up with the 160 bytes bottom most for callee...

This for me seems like the most natural way to extend the current machinery while changing as little as possible...

(Conversely, if the general case were addressed, you actually wouldn't have to implement anything special for those special cases.

It would have been simpler to just spill everything without any fixed offets, but since we want to do STMG I wasn't really sure that was a good idea...

So ideally, you'd want to set LocalAreaOffset to -N (where N depends on the number of GPRs to be saved) instead of -160 for the packed stack layout. Unfortunately, I believe the LocalAreaOffset mechanism doesn't support a variable value for this field. One option would be to find (or create) some way to instantiate a per-function FrameLowering. Another way might be to just always set the LocalAreaOffset to 0 and then force common code to still leave the register save area fully (non-packed layout) or partially (packed layout) empty by placing fixed frame objects there (cf. the logic in PEI::calculateFrameObjectOffsets).

Perhaps we could just set a new smaller LAO value once we know it in emitPrologue(), instead of adding the amount for each offset in SystemZFrameLowering::getFrameIndexReference()?

Getting rid of the LAO and creating fixed objects as needed also sounds like a nice idea. That way those offsets wouldn't have to be changed again as per this current patch. But then I think we would have to first rework all the places that assume the LAO as per the patch I first suggested...

[jonpa]

Forgot: Patch was updated with a bugfix so that the RegSaveAreaUsage is set in the constructor already, so that this is always done.

Everything seems NFC now with

   bool usePackedStack(MachineFunction &MF) {
+    return false;
     bool HasPackedStackAttr = true;

[uweigand]

In D70821#1763645, @jonpa wrote:

I don't think this approach will really do everything -mpacked-stack is supposed to do. Fundamentally, with the packed stack layout, only some N (< 160) bytes of the incoming register save area are reserved for register save slots, instead of all 160 bytes. Everything below "(incoming SP) + 160 - N" is supposed to be freely usable for any use, including FPR/VR save slots and outgoing function args / register save area, but also just local stack variables and reload spill slots in general.

I think it does achieve that... The local area offset is not changed, but my intention is to instead move up all the slots in the local area with the amount that was packed:

Hmm, I missed that. That would probably indeed work, but it does appear confusing to have the frame object offsets not really match what's being emitted in the end ...

First, the GPRs are stored topmost and then any (anyregcc) FP arg regs below them in the reg save area. The amount used of the reg save area is tracked during this with ZFI->incRegSaveAreaUsage(). So any registers which already belong in the reg save area still go there, but with new offets. The amount used is then not 160, but ZFI->getRegSaveAreaUsage().

I still don't understand why the FP special case is necessary. Currently, FP regs are just spilled into freely-allocatable frame objects. Given that you move those frame object (the part I originally missed), shouldn't that simply move the FP regs just the same? (The only difference is the anyregcc FP regs, which should not go into the fixed slot; instead, they can just be spilled into nonfixed frame objects like the others.)

This for me seems like the most natural way to extend the current machinery while changing as little as possible...

Changing as little as possible isn't really a primary goal :-) The primary goal is make the final result as easily understandable as possible ...

It would have been simpler to just spill everything without any fixed offets, but since we want to do STMG I wasn't really sure that was a good idea...

Well, not the GPRs -- they should stay at fixed offsets, of course. But the FPRs don't need fixed offsets.

So ideally, you'd want to set LocalAreaOffset to -N (where N depends on the number of GPRs to be saved) instead of -160 for the packed stack layout. Unfortunately, I believe the LocalAreaOffset mechanism doesn't support a variable value for this field. One option would be to find (or create) some way to instantiate a per-function FrameLowering. Another way might be to just always set the LocalAreaOffset to 0 and then force common code to still leave the register save area fully (non-packed layout) or partially (packed layout) empty by placing fixed frame objects there (cf. the logic in PEI::calculateFrameObjectOffsets).

Perhaps we could just set a new smaller LAO value once we know it in emitPrologue(), instead of adding the amount for each offset in SystemZFrameLowering::getFrameIndexReference()?

I don't think there is currently any way to ever change the LAO value.

Getting rid of the LAO and creating fixed objects as needed also sounds like a nice idea. That way those offsets wouldn't have to be changed again as per this current patch. But then I think we would have to first rework all the places that assume the LAO as per the patch I first suggested...

I don't see why anything here would need to be changed. Frame index offsets would remain relative to the CFA with this approach, we'd just be using a different method to keep the first 160 byte unoccupied. (You first patch changed frame index offsets to be relative to incoming SP, which required all those changes.)

[jonpa]

In D70821#1763704, @uweigand wrote:

In D70821#1763645, @jonpa wrote:

I don't think this approach will really do everything -mpacked-stack is supposed to do. Fundamentally, with the packed stack layout, only some N (< 160) bytes of the incoming register save area are reserved for register save slots, instead of all 160 bytes. Everything below "(incoming SP) + 160 - N" is supposed to be freely usable for any use, including FPR/VR save slots and outgoing function args / register save area, but also just local stack variables and reload spill slots in general.

I think it does achieve that... The local area offset is not changed, but my intention is to instead move up all the slots in the local area with the amount that was packed:

Hmm, I missed that. That would probably indeed work, but it does appear confusing to have the frame object offsets not really match what's being emitted in the end ...

Yes, I agree...

First, the GPRs are stored topmost and then any (anyregcc) FP arg regs below them in the reg save area. The amount used of the reg save area is tracked during this with ZFI->incRegSaveAreaUsage(). So any registers which already belong in the reg save area still go there, but with new offets. The amount used is then not 160, but ZFI->getRegSaveAreaUsage().

I still don't understand why the FP special case is necessary. Currently, FP regs are just spilled into freely-allocatable frame objects. Given that you move those frame object (the part I originally missed), shouldn't that simply move the FP regs just the same? (The only difference is the anyregcc FP regs, which should not go into the fixed slot; instead, they can just be spilled into nonfixed frame objects like the others.)

As long as we return our table of fixed offsets from getCalleeSavedSpillSlots() a register included there will get that offset in assignCalleeSavedSpillSlots(), instead of a created on-fixed object. I thought about changing SystemZFrameLowering::getCalleeSavedSpillSlots(), for instance by returning a NumEntries that could just be 4 less since the FP arg regs are last... But that method does currently not have MF as an argument, and I thought maybe it wouldn't be so bad to just put them below the GPRs... This is called before spillCalleeSavedRegisters() which is where we compute the range of GPRs to save...

This for me seems like the most natural way to extend the current machinery while changing as little as possible...

Changing as little as possible isn't really a primary goal :-) The primary goal is make the final result as easily understandable as possible ...

good point :-)

So ideally, you'd want to set LocalAreaOffset to -N (where N depends on the number of GPRs to be saved) instead of -160 for the packed stack layout. Unfortunately, I believe the LocalAreaOffset mechanism doesn't support a variable value for this field. One option would be to find (or create) some way to instantiate a per-function FrameLowering. Another way might be to just always set the LocalAreaOffset to 0 and then force common code to still leave the register save area fully (non-packed layout) or partially (packed layout) empty by placing fixed frame objects there (cf. the logic in PEI::calculateFrameObjectOffsets).

Perhaps we could just set a new smaller LAO value once we know it in emitPrologue(), instead of adding the amount for each offset in SystemZFrameLowering::getFrameIndexReference()?

I don't think there is currently any way to ever change the LAO value.

Getting rid of the LAO and creating fixed objects as needed also sounds like a nice idea. That way those offsets wouldn't have to be changed again as per this current patch. But then I think we would have to first rework all the places that assume the LAO as per the patch I first suggested...

I don't see why anything here would need to be changed. Frame index offsets would remain relative to the CFA with this approach, we'd just be using a different method to keep the first 160 byte unoccupied. (You first patch changed frame index offsets to be relative to incoming SP, which required all those changes.)

We could add a setOffsetOfLocalArea(), to set it to a new smaller value if we also could make sure to always set it correctly for each function. But I think your idea of just adding the fixed objects as needed sounds better, so maybe I should give that a try..?

[uweigand]

In D70821#1763719, @jonpa wrote:

As long as we return our table of fixed offsets from getCalleeSavedSpillSlots() a register included there will get that offset in assignCalleeSavedSpillSlots(), instead of a created on-fixed object. I thought about changing SystemZFrameLowering::getCalleeSavedSpillSlots(), for instance by returning a NumEntries that could just be 4 less since the FP arg regs are last... But that method does currently not have MF as an argument, and I thought maybe it wouldn't be so bad to just put them below the GPRs... This is called before spillCalleeSavedRegisters() which is where we compute the range of GPRs to save...

I see. Maybe it would be better then to just not use the static "getCalleeSavedSpillSlots" method at all, and instead use the dynamic "assignCalleeSavedSpillSlots" method instead (like most other targets) ...

We could add a setOffsetOfLocalArea(), to set it to a new smaller value if we also could make sure to always set it correctly for each function. But I think your idea of just adding the fixed objects as needed sounds better, so maybe I should give that a try..?

Agreed.

[jonpa]

This version of the patch is the experiment of setting the Local Area Offset to 0, and then creating fixed stack objects to represent the incoming register save area. This means that we are pretending that incoming SP and CFA are the same, and debug dumps show [SP - ...] where SP really is CFA. In order to make this work, the offset between the incoming SP and the CFA now has to be added in SystemZFrameLowering::getFrameIndexReference() instead. I am not sure if this is simpler or not compared to the previous version...

One question I have is about the decision as to when to create of the outgoing Reg Save Area. It seems that on trunk, this is to be done if any stack space is needed outside the incoming register save area. I am a little unsure about the reasoning here, as the only reason I can think of to allocate it in this case is in the case of an exception in which case that routine would need the incoming reg save area. But if we are already using our incoming reg save area, then those registers would be overwritten... I so far just mimic this current behavior and allocate the outgoing reg save area only when CSRs stack space usage includes registers that would not go into the incoming reg save area.

• In SystemZFrameLowering::determineCalleeSaves() the incoming use of the full reg save area in is always added for the default stack layout.

• Implement SystemZFrameLowering::assignCalleeSavedSpillSlots() (instead of getCalleeSavedSpillSlots()):
  • Create fixed stack objects for those registers that do not go into the Register Save Area, in the same way as currently is the result of the common code (CSR saves topmost).
  • Remember the RegSaveAreaUsage() with packed stack. This is used later when deciding if to allocate a new Reg save area.

• processFunctionBeforeFrameFinalized() should now be correct but it is not giving the exact same results on SPEC. In one case it looked like trunk was adding too much, while being conservatively correct:

# Machine code for function scan_one_insn: NoPHIs, TracksLiveness, NoVRegs
Frame Objects:
  fi#-3: size=8, align=8, fixed, at location [SP+120]
  fi#-2: size=8, align=8, fixed, at location [SP+112]
  fi#-1: size=8, align=8, fixed, at location [SP+104]
  fi#0: size=240, align=8, at location [SP+160]
  fi#1: size=120, align=4, at location [SP+160]
  fi#2: size=240, align=8, at location [SP+160]
  fi#3: size=3120, align=4, at location [SP+160]

(gdb) p StackSize
$519 = 3936
(gdb) p MaxArgOffset
$520 = 160
(gdb) p MaxReach
$521 = 4096

StackSize + CallFrameSize = 3776 + 160 = 3936
MFFrame.estimateStackSize(MF) adds 56 to the stack size based on the -56 offset for %R13. This shouldn't be done, since CallFrameSize is added later...

The patch seems to be NFC except for a few cases were spill emergency slots are no longer allocated, per above.

[uweigand]

processFunctionBeforeFrameFinalized() should now be correct but it is not giving the exact same results on SPEC.

I believe the original processFunctionBeforeFrameFinalized computation was correct:

// Get the size of our stack frame to be allocated ...
uint64_t StackSize = (MFFrame.estimateStackSize(MF) +
                      SystemZMC::CallFrameSize);

At this point, estimateStackSize is supposed to return the size of the objects allocated in the local area, excluding both incoming and outgoing register save areas. We add back the size of the outgoing register save area (which we are certain to need in all nontrivial cases).

MFFrame.estimateStackSize(MF) adds 56 to the stack size based on the -56 offset for %R13. This shouldn't be done, since CallFrameSize is added later...

This should not have happened in the old code because the 56 was within the "LocalAreaOffset" and therefore did not count. It is true that with the new logic this will have to change now ...

// ... and the maximum offset we may need to reach into the
 // caller's frame to access the save area or stack arguments.
 int64_t MaxArgOffset = SystemZMC::CallFrameSize;
 for (int I = MFFrame.getObjectIndexBegin(); I != 0; ++I)
   if (MFFrame.getObjectOffset(I) >= 0) {
     int64_t ArgOffset = SystemZMC::CallFrameSize +
                         MFFrame.getObjectOffset(I) +
                         MFFrame.getObjectSize(I);
     MaxArgOffset = std::max(MaxArgOffset, ArgOffset);
   }

Given the above, we know current SP + StackSize suffices to reach the bottom of the incoming register save area. MaxArgOffset is now supposed to cover everything we may need to reach beyond that. This includes everything within the incoming register save are (as we might need to restore from there), and all the incoming arguments.

Now, the difference with the new logic is that estimateStackSize will include the incoming register save area already (all of it in the default case, the parts that are used in the -mpacked-stack case). Therefore, we need to change the computation of MaxArgOffset, exactly as your patch already does. Everything else should be the same.

Therefore, I'm not sure why you're seeing any difference in results -- it should be exactly the same.

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

52

Why is the RegSpillOffsets change above needed?

62

Since this is now the only user of the SpillOffsetTable, we can simplify it again.

76

There's already MFI below?

79

This seems strange. The packed-stack case should ignore all predefined offsets and just place the GPRs that need to be saved at the top of the area.

83

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.

325

Moving the + SystemZMC::CallFrameSize down seems a no-op and doesn't really make anything clearer IMO ...

436

It would be nice if we could avoid the duplicate bookkeeping in RegSaveAreaUsage here.

What we really want to check is whether there is any non-dead, non-fixed object on the stack or not. Maybe something along those lines:

bool haveStackObject = false;
for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) {
  if (!MFI.isDeadObjectIndex(i)) {
    HaveStackObject = true;
    break;
  }

[jonpa]

In D70821#1766858, @uweigand wrote:

processFunctionBeforeFrameFinalized() should now be correct but it is not giving the exact same results on SPEC.

I believe the original processFunctionBeforeFrameFinalized computation was correct:

// Get the size of our stack frame to be allocated ...
uint64_t StackSize = (MFFrame.estimateStackSize(MF) +
                      SystemZMC::CallFrameSize);

At this point, estimateStackSize is supposed to return the size of the objects allocated in the local area, excluding both incoming and outgoing register save areas. We add back the size of the outgoing register save area (which we are certain to need in all nontrivial cases).

MFFrame.estimateStackSize(MF) adds 56 to the stack size based on the -56 offset for %R13. This shouldn't be done, since CallFrameSize is added later...

This should not have happened in the old code because the 56 was within the "LocalAreaOffset" and therefore did not count. It is true that with the new logic this will have to change now ...

// ... and the maximum offset we may need to reach into the
 // caller's frame to access the save area or stack arguments.
 int64_t MaxArgOffset = SystemZMC::CallFrameSize;
 for (int I = MFFrame.getObjectIndexBegin(); I != 0; ++I)
   if (MFFrame.getObjectOffset(I) >= 0) {
     int64_t ArgOffset = SystemZMC::CallFrameSize +
                         MFFrame.getObjectOffset(I) +
                         MFFrame.getObjectSize(I);
     MaxArgOffset = std::max(MaxArgOffset, ArgOffset);
   }

Given the above, we know current SP + StackSize suffices to reach the bottom of the incoming register save area. MaxArgOffset is now supposed to cover everything we may need to reach beyond that. This includes everything within the incoming register save are (as we might need to restore from there), and all the incoming arguments.

Now, the difference with the new logic is that estimateStackSize will include the incoming register save area already (all of it in the default case, the parts that are used in the -mpacked-stack case). Therefore, we need to change the computation of MaxArgOffset, exactly as your patch already does. Everything else should be the same.

Therefore, I'm not sure why you're seeing any difference in results -- it should be exactly the same.

tc_estimatestacksize.ll2 KBDownload

llc -mtriple=s390x-linux-gnu ./tc_estimatestacksize.ll -o -

This is the reduced test case I was looking at. It seems that PEI::calculateFrameObjectOffsets() is taking the Local Area Offset into account, while in fact the very "similar" MFFrame.estimateStackSize() in fact does not (but probably should).

On entry to SystemZFrameLowering::processFunctionBeforeFrameFinalized(), the MF looks like

(gdb) p MF.dump()
# Machine code for function scan_one_insn: NoPHIs, TracksLiveness, NoVRegs
Frame Objects:
  fi#-3: size=8, align=8, fixed, at location [SP+120]
  fi#-2: size=8, align=8, fixed, at location [SP+112]
  fi#-1: size=8, align=8, fixed, at location [SP+104]
  fi#0: size=240, align=8, at location [SP+160]
  fi#1: size=120, align=4, at location [SP+160]
  fi#2: size=240, align=8, at location [SP+160]
  fi#3: size=3120, align=4, at location [SP+160]

The allocas occupy 3720 bytes, and there are fixed frame objects for %R13-%R15. MachineFrameInfo::estimateStackSize() starts with Offset = 0, then checks the greatest fixed offset, and then adds the remaining object sizes, so instead of 3720 (like you expected), it returns 3776...

[uweigand]

It seems that PEI::calculateFrameObjectOffsets() is taking the Local Area Offset into account, while in fact the very "similar" MFFrame.estimateStackSize() in fact does not (but probably should).

Ah, I see. Yes, the current MFFrame.estimateStackSize looks incorrect -- it does not take account the Local Area Offset, but it still accounts for whatever fixed objects happen to be placed inside that offset; that's inconsistent. Either it should always fully include the Local Area Offset in the stack size, or else it should always exclude it, but then it must also exclude objects that happen to be placed there.

It seems this problem wasn't noticed so far since MFFrame.estimateStackSize is never used by common code, it is simply a helper routine that can be used by targets. And all targets that use it (except SystemZ!) just happen to use a zero Local Area Offset, so the problem doesn't occur. And even we didn't use the helper until recently; I didn't notice that problem when I added it.

[jonpa]

Patch updated per review.

Instead of caring religiously about which saved CSRs actually belonged in the reg save area and based on that deciding if the outgoing reg save are should be allocated, pach has been simplified per review comments:

• Save all the CSRs in fixed stack slots
  • packed stack: GPRs on top, then the rest in any order.
  • default: Per the offsets in table, and the rest below the incoming reg save area.
• Allocate the outgoing reg save area in the precence of any non-dead, non-fixed stack object per your suggestion ("HasStackObject"). This changes a few test cases, since e.g. F8 is now saved on a fixed stack slot instead of a non-fixed one as was done previously. The reason all CSRs now have fixed stack objects relates to MinCSFrameIndex / MaxCSFrameIndex as explained previously. This also makes a dozen or so files on SPEC not get the outgoing reg save area allocated anymore.
• Don't use the offset table with packed stack, instead rely on register numbers. Assert added to check that this is really ok.

• A new getOrCreateFramePointerSaveIndex() was created to share with SystemZISelLowering.
• I am not sure usePackedStack() is complete/correct, but it's a good start...
• More tests are probably needed.
• Front End patch yet todo.

[jonpa]

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

52

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?

83

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

[uweigand]

This changes a few test cases, since e.g. 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 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.

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

445

The hasVarSizedObjects check is probably unnecessary now, since if there are any variable-sized stack objects, HasStackObject will now already be true.

llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h

29

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?

[jonpa]

Patch updated.

[jonpa]

In D70821#1774071, @uweigand wrote:

This changes a few test cases, since e.g. 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 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 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...

llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h

29

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?

[uweigand]

This now looks all functionally correct to me, just a few more cosmetic issues ...

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

52

Well, it can't hurt, so this is fine with me.

132

This ignores alignment. That's in fact OK here since for all register save slots we only require 8-byte alignment and all register sizes are a multiple of 8, so this is always OK. An assert (or at least a comment) would still be good.

178

I still don't quite like how like logic is now split between two routines. At the very least, the comment needs to be updated (there no longer is any "loop above"). Even better would be to find a way to move the vararg handling to assignCalleeSavedSpillSlots as well ...

llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h

14

Having to add all these includes here seems to indicate a bit of a layering violation. This is about the getOrCreateFramePointerSaveIndex routine below? I think that should probably be a SystemZFrameLowering function instead ...

[jonpa]

Updated per review. See inline comments...

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

132

Added assert.

178

I just updated the comment for now.
It kind of makes a little sense to me to have the modification of LowGPR and StartSPOffset as it is in spillCalleeSavedRegisters() since this is only a local modification while the unmodified values are used later by restoreCalleeSavedRegisters().
If that was moved into assignCalleeSavedSpillSlots, it seems we would either have to extend SystemZMachineFunctionInfo with e.g. getLowSavedGPR_Varargs() etc, or perhaps do some handling in restoreCalleeSavedRegisters instead.

llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h

14

Moved the two new methods to SystemZFrameLowering (which both needed the includes).

[uweigand]

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

48

Why does this even need to be a SystemZFrameLowering member function? Could be just static in this file ...

178

Yes, we'd use something like LowSpillGPR/HighSpillGPR/SpillGPROffset and LowRestoreGPR/HighRestoreGPR/RestoreGPROffset (could also be grouped instead of having separate accessors) to indicate the different register ranges & offset for the STMG vs. LMG.

Basically, have assignCalledSavedSpillSlots fully prepare the arguments of STMG and LMG, and then have spillCalleeSavedRegister/restoreCalleeSavedRegister just emit those instructions.

The reason I'm concerned is that the logic below really makes assumptions about how the LowGPR and the offset are associated. In fact, it would not work with the packed layout (which is why this is disabled above for now ...). It just seems cleaner to have this in one place.

llvm/lib/Target/SystemZ/SystemZFrameLowering.h

12

What is this needed for?

[jonpa]

Updated per review.

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

48

right.

178

Followed your suggestions including grouping into a struct which seemed neater.

llvm/lib/Target/SystemZ/SystemZFrameLowering.h

12

removed

[uweigand]

I like this much better, thanks for making that change. Just two more questions:

• Why are you calling getOrCreateFramePointerSaveIndex in determineCalleeSaves? Wouldn't it be preferable to move that to assignCalleeSavedSpillSlots? Then all the code depending on usePackedStack would be in one place -- and maybe this wouldn't even to be a seperate subroutine at all, but just a local decision in assignCalleeSavedSpillSlots.

• Maybe it would be better to move the IsVarArg code even within assignCalleeSavedSpillSlots to inside the usePackedStack block (after all, in the !usePackedStack block that code would be incorrect -- and yes, currently we never use packed stack for varargs, but that's just a limitation of the current code; in principle, it would of course be possible to do so, as GCC does as well). (But that's just a minor issue at this point and could also be addressed later.)

[jonpa]

Varargs part of argument handling in assignCalleeSavedSpillSlots() moved into the !usePackedStack section.

Why are you calling getOrCreateFramePointerSaveIndex in determineCalleeSaves?

This has to always be done, and since the PrologEpilogInserter.cpp:assignCalleeSavedSpillSlots() has an early exit if SavedRegs is empty, TFI->assignCalleeSavedSpillSlots() will not always be reached. So to me that wouldn't work...
Maybe the constructor of SystemZMachineFunctionInfo would actually be the best place, although that would force the usePackedStack() to be moved back there as well...

[uweigand]

In D70821#1779889, @jonpa wrote:

Varargs part of argument handling in assignCalleeSavedSpillSlots() moved into the !usePackedStack section.

Thanks!

Why are you calling getOrCreateFramePointerSaveIndex in determineCalleeSaves?

This has to always be done, and since the PrologEpilogInserter.cpp:assignCalleeSavedSpillSlots() has an early exit if SavedRegs is empty, TFI->assignCalleeSavedSpillSlots() will not always be reached. So to me that wouldn't work...
Maybe the constructor of SystemZMachineFunctionInfo would actually be the best place, although that would force the usePackedStack() to be moved back there as well...

Ah, I see. I'm still a bit concerned about using determineCalleeSaves, as it is a bit unexpected for this routine to do anything else than filling in the SavedRegs array. For example, there are other callers of determineCalleeSaves like in ShrinkWrap.cpp, I'm not sure if this (or some future caller) might get confused.

Possibly the best choice then is to put it at the top of the processFunctionBeforeFrameFinalized routine? That routine seems to be the place for target-specific stuff.

[jonpa]

getOrCreateFramePointerSaveIndex() call moved to processFunctionBeforeFrameFinalized() per suggestion.

Renamed 'O' arguments to 'Offs' in setSpillGPRRegs/setRestoreGPRRegs.

[uweigand]

Ok, I think now this all looks good to me. Thanks!

[jonpa]

Thanks for review!

61f5ba5

llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.h

 namespace SystemZMC {
 // How many bytes are in the ABI-defined, caller-allocated part of
 // a stack frame.
 const int64_t CallFrameSize = 160;
 
 // The offset of the DWARF CFA from the incoming stack pointer.
 const int64_t CFAOffsetFromInitialSP = CallFrameSize;
 
+const int64_t FPArgRegSaveAreaSize = 32;
+
 // Maps of asm register numbers to LLVM register numbers, with 0 indicating
 // an invalid register.  In principle we could use 32-bit and 64-bit register
 // classes directly, provided that we relegated the GPR allocation order
 // in SystemZRegisterInfo.td to an AltOrder and left the default order
 // as %r0-%r15.  It seems better to provide the same interface for
 // all classes though.
 extern const unsigned GR32Regs[16];
 extern const unsigned GRH32Regs[16];

llvm/lib/Target/SystemZ/SystemZFrameLowering.h

 //===-- SystemZFrameLowering.h - Frame lowering for SystemZ -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZFRAMELOWERING_H
 #define LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZFRAMELOWERING_H
 
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 
 namespace llvm {
 class SystemZTargetMachine;
 class SystemZSubtarget;
 
 class SystemZFrameLowering : public TargetFrameLowering {
   IndexedMap<unsigned> RegSpillOffsets;
bool restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
                                    const TargetRegisterInfo *TRI) const
     override;
   void processFunctionBeforeFrameFinalized(MachineFunction &MF,
                                            RegScavenger *RS) const override;
   void emitPrologue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
   void emitEpilogue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
   bool hasFP(const MachineFunction &MF) const override;
   bool hasReservedCallFrame(const MachineFunction &MF) const override;
+  int getFrameIndexReference(const MachineFunction &MF, int FI,
+                             unsigned &FrameReg) const override;
   MachineBasicBlock::iterator
   eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
                                 MachineBasicBlock::iterator MI) const override;
 
   // Return the byte offset from the incoming stack pointer of Reg's
   // ABI-defined save slot.  Return 0 if no slot is defined for Reg.
   unsigned getRegSpillOffset(unsigned Reg) const {
     return RegSpillOffsets[Reg];
   }
 };
 } // end namespace llvm
 
 #endif

llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

static const TargetFrameLowering::SpillSlot SpillOffsetTable[] = {
   { SystemZ::R15D, -SystemZMC::CallFrameSize + 0x78 },
   { SystemZ::F0D,  -SystemZMC::CallFrameSize + 0x80 },
   { SystemZ::F2D,  -SystemZMC::CallFrameSize + 0x88 },
   { SystemZ::F4D,  -SystemZMC::CallFrameSize + 0x90 },
   { SystemZ::F6D,  -SystemZMC::CallFrameSize + 0x98 }
 };
 } // end anonymous namespace
 
 SystemZFrameLowering::SystemZFrameLowering()
     : 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.
   // These offsets are relative to the start of the register save area.
   RegSpillOffsets.grow(SystemZ::NUM_TARGET_REGS);
   for (unsigned I = 0, E = array_lengthof(SpillOffsetTable); I != E; ++I)
     RegSpillOffsets[SpillOffsetTable[I].Reg] =
       SystemZMC::CallFrameSize + SpillOffsetTable[I].Offset;
 }
 
 const TargetFrameLowering::SpillSlot *
 SystemZFrameLowering::getCalleeSavedSpillSlots(unsigned &NumEntries) const {
   NumEntries = array_lengthof(SpillOffsetTable);
   return SpillOffsetTable;
 }
 
 void SystemZFrameLowering::determineCalleeSaves(MachineFunction &MF,
                                                 BitVector &SavedRegs,
                                                 RegScavenger *RS) const {
   TargetFrameLowering::determineCalleeSaves(MF, SavedRegs, RS);
 
   MachineFrameInfo &MFFrame = MF.getFrameInfo();
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
   bool HasFP = hasFP(MF);
   SystemZMachineFunctionInfo *MFI = MF.getInfo<SystemZMachineFunctionInfo>();
   bool IsVarArg = MF.getFunction().isVarArg();
 
   // va_start stores incoming FPR varargs in the normal way, but delegates
   // the saving of incoming GPR varargs to spillCalleeSavedRegisters().
   // Record these pending uses, which typically include the call-saved
   // argument register R6D.
   if (IsVarArg)
     for (unsigned I = MFI->getVarArgsFirstGPR(); I < SystemZ::NumArgGPRs; ++I)
       SavedRegs.set(SystemZ::ArgGPRs[I]);
 
   // If there are any landing pads, entering them will modify r6/r7.
   if (!MF.getLandingPads().empty()) {

 static void addSavedGPR(MachineBasicBlock &MBB, MachineInstrBuilder &MIB,
                         unsigned GPR64, bool IsImplicit) {
   const TargetRegisterInfo *RI =
       MBB.getParent()->getSubtarget().getRegisterInfo();
   Register GPR32 = RI->getSubReg(GPR64, SystemZ::subreg_l32);
   bool IsLive = MBB.isLiveIn(GPR64) || MBB.isLiveIn(GPR32);
   if (!IsLive || !IsImplicit) {
     MIB.addReg(GPR64, getImplRegState(IsImplicit) | getKillRegState(!IsLive));
     if (!IsLive)
       MBB.addLiveIn(GPR64);
   }
 }
 
 bool SystemZFrameLowering::
 spillCalleeSavedRegisters(MachineBasicBlock &MBB,
                           MachineBasicBlock::iterator MBBI,
                           const std::vector<CalleeSavedInfo> &CSI,
                           const TargetRegisterInfo *TRI) const {
-  if (CSI.empty())
-    return false;
 
   MachineFunction &MF = *MBB.getParent();
   const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
   SystemZMachineFunctionInfo *ZFI = MF.getInfo<SystemZMachineFunctionInfo>();
+  MachineFrameInfo &MFFrame = MF.getFrameInfo();
   bool IsVarArg = MF.getFunction().isVarArg();
   DebugLoc DL;
 
+  if (!ZFI->usePackedStack(MF))
+    ZFI->incRegSaveAreaUsage(SystemZMC::CallFrameSize);
+
+  if (CSI.empty())
+    return false;
+
   // Scan the call-saved GPRs and find the bounds of the register spill area.
   unsigned LowGPR = 0;
   unsigned HighGPR = SystemZ::R15D;
   unsigned StartOffset = -1U;
   for (unsigned I = 0, E = CSI.size(); I != E; ++I) {
     unsigned Reg = CSI[I].getReg();
     if (SystemZ::GR64BitRegClass.contains(Reg)) {
       unsigned Offset = RegSpillOffsets[Reg];
       assert(Offset && "Unexpected GPR save");
       if (StartOffset > Offset) {
         LowGPR = Reg;
         StartOffset = Offset;
       }
     }
   }
 
   // Save the range of call-saved registers, for use by the epilogue inserter.
   ZFI->setLowSavedGPR(LowGPR);
   ZFI->setHighSavedGPR(HighGPR);
 
   // Include the GPR varargs, if any.  R6D is call-saved, so would
   // be included by the loop above, but we also need to handle the
   // call-clobbered argument registers.
   if (IsVarArg) {
     unsigned FirstGPR = ZFI->getVarArgsFirstGPR();
     if (FirstGPR < SystemZ::NumArgGPRs) {
       unsigned Reg = SystemZ::ArgGPRs[FirstGPR];
       unsigned Offset = RegSpillOffsets[Reg];
       if (StartOffset > Offset) {
         LowGPR = Reg; StartOffset = Offset;
       }
     }
   }
 
   // Save GPRs
   if (LowGPR) {
     assert(LowGPR != HighGPR && "Should be saving %r15 and something else");
 
+    if (ZFI->usePackedStack(MF)) {
+      // Packed stack: save GPRs topmost in the Register save area.
+      StartOffset += SystemZMC::FPArgRegSaveAreaSize;
+      ZFI->incRegSaveAreaUsage(SystemZMC::CallFrameSize - StartOffset);
+      for (unsigned I = 0, E = CSI.size(); I != E; ++I) {
+        unsigned Reg = CSI[I].getReg();
+        if (SystemZ::GR64BitRegClass.contains(Reg)) {
+          int FI = CSI[I].getFrameIdx();
+          int Offset = MFFrame.getObjectOffset(FI);
+          MFFrame.setObjectOffset(FI, Offset + SystemZMC::FPArgRegSaveAreaSize);
+        }
+      }
+    }
+
     // Build an STMG instruction.
     MachineInstrBuilder MIB = BuildMI(MBB, MBBI, DL, TII->get(SystemZ::STMG));
 
     // Add the explicit register operands.
     addSavedGPR(MBB, MIB, LowGPR, false);
     addSavedGPR(MBB, MIB, HighGPR, false);
 
     // Add the address.
if (IsVarArg)
         addSavedGPR(MBB, MIB, SystemZ::ArgGPRs[I], true);
   }
 
   // Save FPRs/VRs in the normal TargetInstrInfo way.
   for (unsigned I = 0, E = CSI.size(); I != E; ++I) {
     unsigned Reg = CSI[I].getReg();
     if (SystemZ::FP64BitRegClass.contains(Reg)) {
       MBB.addLiveIn(Reg);
-      TII->storeRegToStackSlot(MBB, MBBI, Reg, true, CSI[I].getFrameIdx(),
+      int FI = CSI[I].getFrameIdx();
+      if (ZFI->usePackedStack(MF) && MFFrame.isFixedObjectIndex(FI)) {
+        // Packed stack: save any FP arg regs below the GPRs in the Register
+        // save area.
+        ZFI->incRegSaveAreaUsage(8);
+        int Offset = ZFI->getRegSaveAreaUsage();
+        MFFrame.setObjectOffset(FI, -Offset);
+      }
+      TII->storeRegToStackSlot(MBB, MBBI, Reg, true, FI,
                                &SystemZ::FP64BitRegClass, TRI);
     }
     if (SystemZ::VR128BitRegClass.contains(Reg)) {
       MBB.addLiveIn(Reg);
       TII->storeRegToStackSlot(MBB, MBBI, Reg, true, CSI[I].getFrameIdx(),
                                &SystemZ::VR128BitRegClass, TRI);
     }
   }
if (SystemZ::VR128BitRegClass.contains(Reg))
                                 &SystemZ::VR128BitRegClass, TRI);
   }
 
   // Restore call-saved GPRs (but not call-clobbered varargs, which at
   // this point might hold return values).
   unsigned LowGPR = ZFI->getLowSavedGPR();
   unsigned HighGPR = ZFI->getHighSavedGPR();
   unsigned StartOffset = RegSpillOffsets[LowGPR];
+  if (ZFI->usePackedStack(MF))
+    StartOffset += SystemZMC::FPArgRegSaveAreaSize;
   if (LowGPR) {
     // If we saved any of %r2-%r5 as varargs, we should also be saving
     // and restoring %r6.  If we're saving %r6 or above, we should be
     // restoring it too.
     assert(LowGPR != HighGPR && "Should be loading %r15 and something else");
 
     // Build an LMG instruction.
     MachineInstrBuilder MIB = BuildMI(MBB, MBBI, DL, TII->get(SystemZ::LMG));

 }
 
 void SystemZFrameLowering::
 processFunctionBeforeFrameFinalized(MachineFunction &MF,
                                     RegScavenger *RS) const {
   MachineFrameInfo &MFFrame = MF.getFrameInfo();
   // Get the size of our stack frame to be allocated ...
   uint64_t StackSize = (MFFrame.estimateStackSize(MF) +
                         SystemZMC::CallFrameSize);
   // ... and the maximum offset we may need to reach into the
   // caller's frame to access the save area or stack arguments.
   int64_t MaxArgOffset = SystemZMC::CallFrameSize;
   for (int I = MFFrame.getObjectIndexBegin(); I != 0; ++I)
     if (MFFrame.getObjectOffset(I) >= 0) {
       int64_t ArgOffset = SystemZMC::CallFrameSize +
                           MFFrame.getObjectOffset(I) +
                           MFFrame.getObjectSize(I);
if (MBBI != MBB.end() && MBBI->getOpcode() == SystemZ::STMG)
       ++MBBI;
     else
       llvm_unreachable("Couldn't skip over GPR saves");
 
     // Add CFI for the GPR saves.
     for (auto &Save : CSI) {
       unsigned Reg = Save.getReg();
       if (SystemZ::GR64BitRegClass.contains(Reg)) {
-        int64_t Offset = SPOffsetFromCFA + RegSpillOffsets[Reg];
+        int FI = Save.getFrameIdx();
+        int64_t Offset = MFFrame.getObjectOffset(FI);
         unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createOffset(
             nullptr, MRI->getDwarfRegNum(Reg, true), Offset));
         BuildMI(MBB, MBBI, DL, ZII->get(TargetOpcode::CFI_INSTRUCTION))
             .addCFIIndex(CFIIndex);
       }
     }
   }
 
   uint64_t StackSize = MFFrame.getStackSize();
   // We need to allocate the ABI-defined 160-byte base area whenever
   // we allocate stack space for our own use and whenever we call another
-  // function.
+  // function (this amount may be less with packed stack).
   if (StackSize || MFFrame.hasVarSizedObjects() || MFFrame.hasCalls()) {
-    StackSize += SystemZMC::CallFrameSize;
+    StackSize += ZFI->getRegSaveAreaUsage();
     MFFrame.setStackSize(StackSize);
   }
 
   if (StackSize) {
     // Determine if we want to store a backchain.
     bool StoreBackchain = MF.getFunction().hasFnAttribute("backchain");
 
     // If we need backchain, save current stack pointer.  R1 is free at this
     // point.
     if (StoreBackchain)
       BuildMI(MBB, MBBI, DL, ZII->get(SystemZ::LGR))
         .addReg(SystemZ::R1D, RegState::Define).addReg(SystemZ::R15D);
 
     // Allocate StackSize bytes.
     int64_t Delta = -int64_t(StackSize);
     emitIncrement(MBB, MBBI, DL, SystemZ::R15D, Delta, ZII);
 
     // Add CFI for the allocation.
     unsigned CFIIndex = MF.addFrameInst(
         MCCFIInstruction::createDefCfaOffset(nullptr, SPOffsetFromCFA + Delta));
     BuildMI(MBB, MBBI, DL, ZII->get(TargetOpcode::CFI_INSTRUCTION))
         .addCFIIndex(CFIIndex);
     SPOffsetFromCFA += Delta;
 
     if (StoreBackchain)
       BuildMI(MBB, MBBI, DL, ZII->get(SystemZ::STG))
-        .addReg(SystemZ::R1D, RegState::Kill).addReg(SystemZ::R15D).addImm(0).addReg(0);
+        .addReg(SystemZ::R1D, RegState::Kill).addReg(SystemZ::R15D).addImm(0)
+        .addReg(0);
   }
 
   if (HasFP) {
     // Copy the base of the frame to R11.
     BuildMI(MBB, MBBI, DL, ZII->get(SystemZ::LGR), SystemZ::R11D)
       .addReg(SystemZ::R15D);
 
     // Add CFI for the new frame location.

 SystemZFrameLowering::hasReservedCallFrame(const MachineFunction &MF) const {
   // The ABI requires us to allocate 160 bytes of stack space for the callee,
   // with any outgoing stack arguments being placed above that.  It seems
   // better to make that area a permanent feature of the frame even if
   // we're using a frame pointer.
   return true;
 }
 
+int SystemZFrameLowering::getFrameIndexReference(const MachineFunction &MF,
+                                                 int FI,
+                                                 unsigned &FrameReg) const {
+  const MachineFrameInfo &MFFrame = MF.getFrameInfo();
+  const SystemZMachineFunctionInfo *ZFI =
+    MF.getInfo<SystemZMachineFunctionInfo>();
+  // With packed stack the local area typically begins above the incoming SP.
+  int64_t PackedAmt = MFFrame.isFixedObjectIndex(FI) ?
+                      0 : SystemZMC::CallFrameSize - ZFI->getRegSaveAreaUsage();
+
+  int64_t Offset =
+    TargetFrameLowering::getFrameIndexReference(MF, FI, FrameReg);
+
+  return Offset + PackedAmt;
+}
+
+
 MachineBasicBlock::iterator SystemZFrameLowering::
 eliminateCallFramePseudoInstr(MachineFunction &MF,
                               MachineBasicBlock &MBB,
                               MachineBasicBlock::iterator MI) const {
   switch (MI->getOpcode()) {
   case SystemZ::ADJCALLSTACKDOWN:
   case SystemZ::ADJCALLSTACKUP:
     assert(hasReservedCallFrame(MF) &&
            "ADJSTACKDOWN and ADJSTACKUP should be no-ops");
     return MBB.erase(MI);
     break;
 
   default:
     llvm_unreachable("Unexpected call frame instruction");
   }
 }

llvm/lib/Target/SystemZ/SystemZMachineFunctionInfo.h

 //=== SystemZMachineFunctionInfo.h - SystemZ machine function info -*- C++ -*-//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZMACHINEFUNCTIONINFO_H
 #define LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZMACHINEFUNCTIONINFO_H
 
+#include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/Function.h"
 
 namespace llvm {
 
 class SystemZMachineFunctionInfo : public MachineFunctionInfo {
   virtual void anchor();
   unsigned LowSavedGPR;
   unsigned HighSavedGPR;
   unsigned VarArgsFirstGPR;
   unsigned VarArgsFirstFPR;
   unsigned VarArgsFrameIndex;
   unsigned RegSaveFrameIndex;
   int FramePointerSaveIndex;
   bool ManipulatesSP;
   unsigned NumLocalDynamics;
+  unsigned RegSaveAreaUsage;
 
 public:
   explicit SystemZMachineFunctionInfo(MachineFunction &MF)
     : LowSavedGPR(0), HighSavedGPR(0), VarArgsFirstGPR(0), VarArgsFirstFPR(0),
       VarArgsFrameIndex(0), RegSaveFrameIndex(0), FramePointerSaveIndex(0),
-      ManipulatesSP(false), NumLocalDynamics(0) {}
+      ManipulatesSP(false), NumLocalDynamics(0), RegSaveAreaUsage(0) {}
 
   // Get and set the first call-saved GPR that should be saved and restored
   // by this function.  This is 0 if no GPRs need to be saved or restored.
   unsigned getLowSavedGPR() const { return LowSavedGPR; }
   void setLowSavedGPR(unsigned Reg) { LowSavedGPR = Reg; }
 
   // Get and set the last call-saved GPR that should be saved and restored
   // by this function.
public:
   // Get and set whether the function directly manipulates the stack pointer,
   // e.g. through STACKSAVE or STACKRESTORE.
   bool getManipulatesSP() const { return ManipulatesSP; }
   void setManipulatesSP(bool MSP) { ManipulatesSP = MSP; }
 
   // Count number of local-dynamic TLS symbols used.
   unsigned getNumLocalDynamicTLSAccesses() const { return NumLocalDynamics; }
   void incNumLocalDynamicTLSAccesses() { ++NumLocalDynamics; }
+
+  unsigned getRegSaveAreaUsage() const { return RegSaveAreaUsage; }
+  void incRegSaveAreaUsage(unsigned Inc) { RegSaveAreaUsage += Inc; }
+
+  bool usePackedStack(MachineFunction &MF) {
+    bool HasPackedStackAttr = true;
+    bool IsVarArg = MF.getFunction().isVarArg();
+    bool CallConv = MF.getFunction().getCallingConv() != CallingConv::GHC;
+    bool BackChain = MF.getFunction().hasFnAttribute("backchain");
+    bool FramAddressTaken = MF.getFrameInfo().isFrameAddressTaken();
+    return HasPackedStackAttr && !IsVarArg && CallConv && !BackChain &&
+           !FramAddressTaken;
+  }
 };
 
 } // end namespace llvm
 
 #endif

llvm/test/CodeGen/SystemZ/frame-22.ll

+; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s
+;
+; Test the packed stack layout.
+
+; Test spill/restore of an FPR and a GPR.
+define void @f1() {
+; CHECK-LABEL: f1:
+; CHECK: stmg %r12, %r15, 128(%r15)
+; CHECK-NEXT: .cfi_offset %r12, -32
+; CHECK-NEXT: .cfi_offset %r15, -8
+; CHECK-NEXT: aghi %r15, -40
+; CHECK-NEXT: .cfi_def_cfa_offset 200
+; CHECK-NEXT: std %f8, 160(%r15)          # 8-byte Folded Spill
+; CHECK-NEXT: .cfi_offset %f8, -40
+; CHECK-NEXT: #APP
+; CHECK-NEXT: #NO_APP
+; CHECK-NEXT: ld %f8, 160(%r15)          # 8-byte Folded Reload
+; CHECK-NEXT: lmg %r12, %r15, 168(%r15)
+; CHECK-NEXT: br %r14
+  call void asm sideeffect "", "~{f8},~{r12}"() nounwind
+  ret void
+}
+
+; Test spill/restore with anyregcc, including an FP argument register.
+define anyregcc void @f2() {
+; CHECK-LABEL: f2:
+; CHECK: stmg %r3, %r15, 56(%r15)
+; CHECK-NEXT: .cfi_offset %r3, -104
+; CHECK-NEXT: .cfi_offset %r15, -8
+; CHECK-NEXT: aghi %r15, -120
+; CHECK-NEXT: .cfi_def_cfa_offset 280
+; CHECK-NEXT: std %f0, 168(%r15)          # 8-byte Folded Spill
+; CHECK-NEXT: std %f1, 160(%r15)          # 8-byte Folded Spill
+; CHECK-NEXT: .cfi_offset %f0, -112
+; CHECK-NEXT: .cfi_offset %f1, -120
+; CHECK-NEXT: #APP
+; CHECK-NEXT: #NO_APP
+; CHECK-NEXT: ld %f0, 168(%r15)          # 8-byte Folded Reload
+; CHECK-NEXT: ld %f1, 160(%r15)          # 8-byte Folded Reload
+; CHECK-NEXT: lmg %r3, %r15, 176(%r15)
+; CHECK-NEXT: br %r14
+  call void asm sideeffect "", "~{f0},~{f1},~{r3}"() nounwind
+  ret void
+}
+
+; Test spill/restore in local area with incoming stack arguments.
+define i64 @f3(i64 %a, i64 %b, i64 %c, i64 %d, i64 %e, i64 %f, 
+               double %A, double %B, double %C, double %D, double %E) {
+; CHECK-LABEL: f3:
+; CHECK: aghi %r15, -8
+; CHECK-NEXT: .cfi_def_cfa_offset 168
+; CHECK-NEXT: std %f8, 160(%r15)          # 8-byte Folded Spill
+; CHECK-NEXT: .cfi_offset %f8, -8
+; CHECK-NEXT: ld %f0, 176(%r15)
+; CHECK-NEXT: cgdbr %r2, 5, %f0
+; CHECK-NEXT: ag %r2, 168(%r15)
+; CHECK-NEXT: #APP
+; CHECK-NEXT: #NO_APP
+; CHECK-NEXT: ld %f8, 160(%r15)          # 8-byte Folded Reload
+; CHECK-NEXT: aghi %r15, 8
+; CHECK-NEXT: br %r14
+  call void asm sideeffect "", "~{f8}"() nounwind
+  %Ei = fptosi double %E to i64
+  %S = add i64 %f, %Ei
+  ret i64 %S
+}
+
+; Test spill/restore in local area with outgoing stack arguments.
+define i64 @f4() {
+; CHECK-LABEL: f4:
+; CHECK-NEXT: stmg    %r6, %r15, 80(%r15)
+; CHECK-NEXT: .cfi_offset %r6, -80
+; CHECK-NEXT: .cfi_offset %r14, -16
+; CHECK-NEXT: .cfi_offset %r15, -8
+; CHECK-NEXT: aghi    %r15, -104
+; CHECK-NEXT: .cfi_def_cfa_offset 264
+; CHECK-NEXT: std     %f8, 176(%r15)          # 8-byte Folded Spill
+; CHECK-NEXT: .cfi_offset %f8, -88
+; CHECK-NEXT: #APP
+; CHECK-NEXT: #NO_APP
+; CHECK-NEXT: llihh   %r0, 16404
+; CHECK-NEXT: stg     %r0, 168(%r15)
+; CHECK: mvghi   160(%r15), 6
+; CHECK-NEXT: brasl   %r14, f3@PLT
+; CHECK-NEXT: ld      %f8, 176(%r15)          # 8-byte Folded Reload
+; CHECK-NEXT: lmg     %r6, %r15, 184(%r15)
+; CHECK-NEXT: br      %r14
+  call void asm sideeffect "", "~{f8}"() nounwind
+  %C = call i64 @f3 (i64 1, i64 2, i64 3, i64 4, i64 5, i64 6,
+                     double 1.0, double 2.0, double 3.0, double 4.0, double 5.0)
+  ret i64 %C
+}