Add unwind information to mips epilogues

[Richard Sandiford]

Richard Henderson <rth@redhat.com> writes:
> On 09/08/2011 03:08 PM, Richard Sandiford wrote:
>> Bernd Schmidt <bernds@codesourcery.com> writes:
>>> @@ -10227,17 +10236,30 @@ mips_expand_prologue (void)
>>>      emit_insn (gen_blockage ());
>>>  }
>>>  
>>> -/* Emit instructions to restore register REG from slot MEM.  */
>>> +/* Emit instructions to restore register REG from slot MEM.  Also update
>>> +   the cfa_restores list.  */
>>>  
>>>  static void
>>>  mips_restore_reg (rtx reg, rtx mem)
>>>  {
>>> +  rtx orig_reg = reg;
>>> +  rtx last;
>>> +
>>>    /* There's no MIPS16 instruction to load $31 directly.  Load into
>>>       $7 instead and adjust the return insn appropriately.  */
>>>    if (TARGET_MIPS16 && REGNO (reg) == RETURN_ADDR_REGNUM)
>>>      reg = gen_rtx_REG (GET_MODE (reg), GP_REG_FIRST + 7);
>>>  
>>>    mips_emit_save_slot_move (reg, mem, MIPS_EPILOGUE_TEMP (GET_MODE (reg)));
>>> +  if (reg == orig_reg)
>>> +    cfa_restores = alloc_reg_note (REG_CFA_RESTORE, reg, cfa_restores);
>>> +
>>> +  if (!frame_pointer_needed || REGNO (reg) != HARD_FRAME_POINTER_REGNUM)
>>> +    return;
>>> +  last = get_last_insn ();
>>> +  add_reg_note (last, REG_CFA_DEF_CFA, plus_constant (stack_pointer_rtx,
>>> +						      cfa_sp_offset));
>>> +  RTX_FRAME_RELATED_P (last) = 1;
>> 
>> I suppose I still don't get why this is OK but this:
>> 
>>> @@ -10324,12 +10350,26 @@ mips_expand_epilogue (bool sibcall_p)
>>>        if (!TARGET_MIPS16)
>>>  	target = stack_pointer_rtx;
>>>  
>>> -      emit_insn (gen_add3_insn (target, base, adjust));
>>> +      insn = emit_insn (gen_add3_insn (target, base, adjust));
>>> +      if (!frame_pointer_needed && target == stack_pointer_rtx)
>>> +	{
>>> +	  RTX_FRAME_RELATED_P (insn) = 1;
>>> +	  add_reg_note (insn, REG_CFA_DEF_CFA,
>>> +			plus_constant (stack_pointer_rtx, step2));
>>> +	}
>> 
>> triggered ICEs without the !frame_pointer_required.  I think I need
>> to play around with it a bit before I understand enough to review.
>> I'll try to find time this weekend.
>
> I'd be curious to understand in what situation the second hunk causes
> failures without frame_pointer_required.  But as I'm supposed to be on
> holiday atm I can't spend any real time on it til I get back.  ;-)
>
> That said, the first hunk causes the CFA to get swapped back to the
> stack pointer on the very insn that restores the frame pointer.
>
> Which to me looks to do the right thing in all situations.  Is there
> a different window you are worried about?

Nah, nothing like that.  I agree the new patch is setting the CFA at the
right time.  It's just that if those !frame_pointer_needed tests are
needed to avoid ICEs, then it feels like there's still some magic that
I don't understand.

(I'm not saying the checks should be removed from the new patch --
at best it would achieve nothing, and at worst it would bloat the
dwarf2 output.  I'm just curious.)

Richard