<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 07/11/13 12:48, Robert Lytton wrote:<br>
</div>
<blockquote
cite="mid:E55040AE4CA5DE4A84D2754CE295AF30014BB88D@EXMAILBOX1.vo.spidergroup.co.uk"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<style id="owaParaStyle" type="text/css">P {margin-top:0;margin-bottom:0;}</style>
<div style="direction: ltr;font-family: Tahoma;color:
#000000;font-size: 10pt;">Hi,<br>
<br>
A reworking of XCoreFrameLowering::emitPrologue() and
XCoreFrameLowering::emitEpilogue() to handle frames greater than
0xffff words.<br>
Still outstanding are changes to handle resultant large
constants and SP offsets.<br>
<br>
Robert<br>
</div>
</blockquote>
Hi Robert,<br>
<br>
+static void EmitAdjustCfaOffset(MachineBasicBlock &MBB,<br>
+ MachineBasicBlock::iterator MBBI,
DebugLoc dl,<br>
+ const TargetInstrInfo &TII,<br>
+ MachineModuleInfo *MMI, int Adjst)
{<br>
+ MCSymbol *Label = MMI->getContext().CreateTempSymbol();<br>
+ BuildMI(MBB, MBBI, dl,
TII.get(XCore::PROLOG_LABEL)).addSym(Label);<br>
+ // What we would like here is:<br>
+ //
MMI->addFrameInst(MCCFIInstruction::createAdjustCfaOffset(Label,
Adjst));<br>
+ // However ".cfi_adjust_cfa_offset" is not part of the dwarf 2
standard.<br>
+ // Instead we will extract the current Offset and add Adjst to
it.<br>
+ // We assume that this is the prolog and there are no branches.<br>
+ int CfaOffset = 0;<br>
<div style="direction: ltr;font-family: Tahoma;color:
#000000;font-size: 10pt;"><br>
</div>
Doesn't the caller know the absolute offset here (in which case it
can just pass it in)?<br>
<br>
+static void IfNeededExtSP(MachineBasicBlock &MBB,<br>
+ MachineBasicBlock::iterator MBBI,
DebugLoc dl,<br>
<br>
+static void IfNeededLDAWSP(MachineBasicBlock &MBB,<br>
+ MachineBasicBlock::iterator MBBI,
DebugLoc dl,<br>
<br>
These could do with a brief comment explaining what they do.<br>
<br>
+ // Allocate space on the stack at the same time as saving LR.<br>
+ int Offset = RemainingAdj % MaxImmU16;<br>
+ if (!Offset)<br>
+ Offset = MaxImmU16; // Offset's range is 1 to MaxImmU16.<br>
+ RemainingAdj -= Offset;<br>
<br>
This can be simplified to Offset = RemainingAdj % (MaxImmU16 + 1)<br>
<br>
+static void GetSpillList(std::pair<unsigned,int>
(&SpillList)[2],<br>
+ MachineFrameInfo *MFI, XCoreFunctionInfo
*XFI,<br>
+ bool fetchLR, bool fetchFP, bool
RestoringSpill) {<br>
<br>
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.<br>
<br>
Finally could you also add a test for a function with a frame
pointer.<br>
<br>
Thanks,<br>
<br>
Richard<br>
<pre class="moz-signature" cols="72">--
Richard Osborne | XMOS
<a class="moz-txt-link-freetext" href="http://www.xmos.com">http://www.xmos.com</a>
</pre>
</body>
</html>