On 02/12/13 10:11, Eric Botcazou wrote:
>> My apologies for taking so long to look at this.
>
> No problem, all the more so that...
>
>>> 2013-09-05 Eric Botcazou <[email protected]>
>>>
>>> * config/arm/arm.c (arm_expand_prologue): In a nested APCS frame with
>>> arguments to push onto the stack and no varargs, save ip into a stack
>>> slot if r3 isn't available on entry.
>>
>> Sorry, but this is not quite right either, as shown by the attached
>> testcase (in C this time, so we can commit it to gcc.target/arm :-)
>>
>> The problem is that if we have some alignment padding we end up storing
>> ip in one location but restoring it from another.
>>
>> str ip, [sp, #4]
>> add ip, sp, #8
>> stmfd sp!, {fp, ip, lr, pc}
>> sub fp, ip, #12
>> ldr ip, [fp, #4] // <==== Should be fp + 8
>> @ ip needed
>> str r3, [fp, #8]
>> ldr ip, [ip]
>
> ...ugh, right, the computation of the slot address was supposed to be clever
> but is totally broken instead. :-( Thanks for spotting it, I don't
> understand
> how we haven't seen this on VxWorks (although your testcase does not fail on
> VxWorks, since args_to_push is 4 instead of 8 as on arm-eabi).
>
> The computation of the slot address is:
>
> + {
> + rtx x = plus_constant (Pmode, stack_pointer_rtx,
> + args_to_push - 4);
> + insn = emit_insn (gen_addsi3 (stack_pointer_rtx,
> + stack_pointer_rtx,
> + GEN_INT (- args_to_push)));
> + emit_set_insn (gen_frame_mem (SImode, x), ip_rtx);
> + }
>
> meaning that IP will be saved into the first slot on the stack. But the
> restore code is:
>
> /* Recover the static chain register. */
> if (!arm_r3_live_at_start_p () || saved_pretend_args)
> insn = gen_rtx_REG (SImode, 3);
> else
> {
> insn = plus_constant (Pmode, hard_frame_pointer_rtx, 4);
> insn = gen_frame_mem (SImode, insn);
> }
> emit_set_insn (ip_rtx, insn);
>
> meaning that IP is restored from the last slot on the stack, so this can work
> only if args_to_push == 4.
>
> Revised patch attached, your testcase passes on arm-eabi with it. Does it
> look OK to you? If so, I'll run a testing cycle on arm-vxworks and arm-eabi.
>
>
> * config/arm/arm.c (arm_expand_prologue): In a nested APCS frame with
> arguments to push onto the stack and no varargs, save ip into the last
> stack slot if r3 isn't available on entry.
>
>
This generates:
sub sp, sp, #n
str ip, [sp]
which can be done in one instruction, vis:
str ip, [sp, #-n]!
Very similar code to do this essentially already exists a few lines
earlier (in the args_to_push == 0 case), but needs tweaking to use
pre-modify rather than pre-dec when n != 4.
I'll pre-approve a patch that makes that change.
R.
