Re: Add unwind information to mips epilogues

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Richard Henderson <rth@redhat.com> writes:
> On 09/05/2011 01:36 PM, Richard Sandiford wrote:
>> Richard Henderson <rth@redhat.com> writes:
>>> On 09/01/2011 12:13 AM, Richard Sandiford wrote:
>>>> Also, for the frame_pointer_required case, it looks like there's a
>>>> window between the restoration of the frame pointer and the deallocation
>>>> of the stack in which the CFA is still defined in terms of the frame
>>>> pointer register.  Is that significant?  If not (e.g. because we
>>>> should never need to unwind at that point) then why do we still
>>>> update the CFA here:
>>>
>>> What window are you referring to?
>>>
>>> Best I can tell from looking at the patch we restore the CFA to the
>>> stack pointer before anything else.
>> 
>> Well, I'm probably showing my ignorance here, but what I meant was:
>> we attach the DEF_CFA and the CFA_RESTORE notes to the final stack
>> deallocation.  I was just worried that, immediately before that
>> deallocation, the CFA is still defined in terms of the frame pointer,
>> even though the frame pointer has already been restored.  So it seemed
>> like someone trying to unwind at that point would get the wrong CFA.
>
> I don't see that.  I'm pasting from mainline but not the patch, but here:
>
>>   /* Copy TARGET into the stack pointer.  */
>>   if (target != stack_pointer_rtx)
>>     mips_emit_move (stack_pointer_rtx, target);
>
> We restore the stack pointer from the frame pointer before restoring
> the frame pointer.  The patch changes this spot with DEF_CFA.  So the
> problem you're suggesting ought not be true.

But that DEF_CFA is conditional on !frame_pointer_needed:

   /* Copy TARGET into the stack pointer.  */
   if (target != stack_pointer_rtx)
-    mips_emit_move (stack_pointer_rtx, target);
+    {
+      insn = mips_emit_move (stack_pointer_rtx, target);
+      if (!frame_pointer_needed)
+	{
+	  add_reg_note (insn, REG_CFA_DEF_CFA,
+			plus_constant (stack_pointer_rtx, step2));
+	  RTX_FRAME_RELATED_P (insn) = 1;
+	}
+    }

So I think this DEF_CFA is only added if the CFA is already defined in
terms of the stack pointer.  I don't think it triggers if the CFA is
currently defined in terms of the frame pointer (which is the case
where that window might occur).  When I asked Bernd about that, he said:

>> ?  If the window is significant, could we avoid it by removing the
>> !frame_pointer_needed checks from the code above?
>
> That causes aborts due to inconsistent information in dwarf2cfi, since
> we can reach the same instruction with either fp or sp as the CFA register.

Richard