XCore target: Enable frames larger than 65535 to be lowered

[Richard Osborne]

On 07/11/13 12:48, Robert Lytton wrote:
> Hi,
>
> A reworking of XCoreFrameLowering::emitPrologue() and 
> XCoreFrameLowering::emitEpilogue() to handle frames greater than 
> 0xffff words.
> Still outstanding are changes to handle resultant large constants and 
> SP offsets.
>
> Robert
Hi Robert,

+static void EmitAdjustCfaOffset(MachineBasicBlock &MBB,
+                                MachineBasicBlock::iterator MBBI, 
DebugLoc dl,
+                                const TargetInstrInfo &TII,
+                                MachineModuleInfo *MMI, int Adjst) {
+  MCSymbol *Label = MMI->getContext().CreateTempSymbol();
+  BuildMI(MBB, MBBI, dl, TII.get(XCore::PROLOG_LABEL)).addSym(Label);
+  // What we would like here is:
+  // MMI->addFrameInst(MCCFIInstruction::createAdjustCfaOffset(Label, 
Adjst));
+  // However ".cfi_adjust_cfa_offset" is not part of the dwarf 2 standard.
+  // Instead we will extract the current Offset and add Adjst to it.
+  // We assume that this is the prolog and there are no branches.
+  int CfaOffset = 0;

Doesn't the caller know the absolute offset here (in which case it can 
just pass it in)?

+static void IfNeededExtSP(MachineBasicBlock &MBB,
+                          MachineBasicBlock::iterator MBBI, DebugLoc dl,

+static void IfNeededLDAWSP(MachineBasicBlock &MBB,
+                           MachineBasicBlock::iterator MBBI, DebugLoc dl,

These could do with a brief comment explaining what they do.

+    // Allocate space on the stack at the same time as saving LR.
+    int Offset = RemainingAdj % MaxImmU16;
+    if (!Offset)
+      Offset = MaxImmU16;   // Offset's range is 1 to MaxImmU16.
+    RemainingAdj -= Offset;

This can be simplified to Offset = RemainingAdj % (MaxImmU16 + 1)

+static void GetSpillList(std::pair<unsigned,int> (&SpillList)[2],
+                         MachineFrameInfo *MFI, XCoreFunctionInfo *XFI,
+                         bool fetchLR, bool fetchFP, bool RestoringSpill) {

I think it might be better to use 
SmallVector<std::pair<unsigned,int>,2>. This removes the need for the 
InvalidSpillSlot sentinel, simplifying the code. Also I'd prefer it if 
GetSpillList() always returned the list in the same order (i.e. 
RestoringSpill should be removed). The epilogue can iterate over the 
list in reverse order - this makes it clear that the epilogue is the 
reverse of the prologue.

Finally could you also add a test for a function with a frame pointer.

Thanks,

Richard

-- 
Richard Osborne | XMOS
http://www.xmos.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131111/e70924fb/attachment.html>