<html dir="ltr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>
<body ocsi="0" fpstyle="1" bgcolor="#FFFFFF">
<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;">Hi Richard,<br>
<br>
Please could you re-review the changed patch - would a diff of the patches be helpful?<br>
<br>
<br>
+static void EmitAdjustCfaOffset(MachineBasicBlock &MBB,<br>
<div style="font-family: Times New Roman; color: #000000; font-size: 16px">
<div>> Doesn't the caller know the absolute offset here (in which case it can just pass it in)?<br>
I've recoded so that we can calculate the absolute offset.<br>
The function is now called EmitDefCfaOffset() to reflect this change.<br>
<br>
<br>
+static void IfNeededExtSP(MachineBasicBlock &MBB,<br>
+static void IfNeededLDAWSP(MachineBasicBlock &MBB,<br>
> These could do with a brief comment explaining what they do.<br>
Done-ish. Not so brief.<br>
<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>
> This can be simplified to Offset = RemainingAdj % (MaxImmU16 + 1)<br>
The original idea was to pick up the remainder at the start, leaving only jumps of MaxImmU16.<br>
However, I've recoded to do the remainder at the end (consistent with FP output)<br>
<br>
<br>
+static void GetSpillList(std::pair<unsigned,int> (&SpillList)[2],<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.<br>
> 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>
Done.<br>
<br>
> Finally could you also add a test for a function with a frame pointer.<br>
Sorry, I had some examples but they required large const handling... and were in the next patch.<br>
I have added a "-disable-fp-elim" test run to the epilogue_prologue.ll test file.<br>
<br>
Robert<br>
<br>
<br>
</div>
</div>
</div>
</body>
</html>