Richard Henderson <r...@redhat.com> writes:
> On 09/08/2011 03:08 PM, Richard Sandiford wrote:
>> Bernd Schmidt <ber...@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
