[PATCH] D17000: [AArch64] Reduce number of callee-save save/restores.

[Tim Northover via llvm-commits]

t.p.northover added a comment.

Hi Geoff,

Unfortunately this has implications for exception unwinding on iOS. We have fast-path .debug_frame equivalents that only have 32 bits to store the prologue's format, and the format assumes the registers are stored in pairs.

On the other hand, we really don't want frame lowering to be any more complex than it absolutely has to be. For now, we probably have to go with the existing format on Darwin but I think we can probably come up with a better determineCalleeSaves that serves both needs.

To start with, some observations (none of them your fault, the existing code did it):

- UnspilledCSFPRs is completely unused.
- Only one value is really needed in UnspilledCSGPRs: we need one to scavenge, but that's it.
- Only the sum of NumGPRSpilled and NumFPRSpilled is used.

So, I think we can rewrite the top loop as:

  // Figure out which callee-saved registers to save/restore.
  for (unsigned i = 0; CSRegs[i]; ++i) {
    const unsigned Reg = CSRegs[i];
    bool RegUsed = SavedRegs.test(Reg);
  
    if (!RegUsed && AArch64::GPR64RegClass.contains(Reg) && !RegInfo->isReservedReg(MF, Reg)) {
      // Save this one for later in case we need a scratch register.
      UnspilledGPR = Reg;
      continue;
    }
  
    // MachO's compact unwind format relies on all registers being stored in pairs.
    // FIXME: the usual format is actually better if unwinding isn't needed.
    unsigned PairedReg = CSRegs[i ^ 1];
    if (MF.getSubtarget().isTargetMachO() && !SavedRegs.test(PairedReg)) {
      SavedRegs.set(PairedReg);
      ExtraCSSpill = true;
    }
  }
  
  NumRegsSpilled = SavedRegs.size();
  CanEliminateFrame = NumRegsSpilled == 0;

Do you think you could make something like that work, or have I overlooked a key use that brings my whole edifice crumbling down?

Cheers.

Tim.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:746
@@ +745,3 @@
+  RegPairInfo &LastPair = RegPairs.back();
+  assert(AFI->getCalleeSavedStackSize() % 8 == 0);
+  LastPair.Offset = AFI->getCalleeSavedStackSize() / 8;
----------------
I think this should be asserting 16: it matches the comment a few lines above, the corresponding set call, and the ABI requirement.


http://reviews.llvm.org/D17000