On 12/28/2017 01:37 PM, Richard Sandiford wrote:
> Andreas Schwab <sch...@linux-m68k.org> writes:
>> On Dez 23 2017, Richard Sandiford <richard.sandif...@linaro.org> wrote:
>>> gcc/
>>> * expr.c (fixup_args_size_notes): Check that any existing
>>> REG_ARGS_SIZE notes are correct, and don't try to re-add them.
>>> (emit_single_push_insn_1): Move stack_pointer_delta adjustment to...
>>> (emit_single_push_insn): ...here.
>>
>> Successfully regtested on m68k-linux.
>
> Thanks. Now also tested on aarch64-linux-gnu, x86_64-linux-gnu and
> powerpc64-linux-gnu (not that that will give mucn coverage). Also
> tested with a before-and-after comparison of testsuite output for
> a range of targets. OK to install?
>
> Richard
>
>
> The new assert in add_args_size_note triggered for gcc.dg/tls/opt-3.c
> and other on m68k. This looks like a pre-existing bug: if we pushed
> a value that needs a call to something like __tls_get_addr, we ended
> up with two different REG_ARGS_SIZE notes on the same instruction.
>
> It seems to be OK for emit_single_push_insn to push something that
> needs a call to __tls_get_addr:
>
> /* We have to allow non-call_pop patterns for the case
> of emit_single_push_insn of a TLS address. */
> if (GET_CODE (pat) != PARALLEL)
> return 0;
>
> so I think the bug is in the way this is handled rather than the fact
> that it occurs at all.
>
> If we're pushing a value X that needs a call C to calculate, we'll
> add REG_ARGS_SIZE notes to the pushes and pops for C as part of the
> call sequence. Then emit_single_push_insn calls fixup_args_size_notes
> on the whole push sequence (the calculation of X, including C,
> and the push of X itself). This is where the double notes came from.
> But emit_single_push_insn_1 adjusted stack_pointer_delta *before* the
> push, so the notes added for C were relative to the situation after
> the future push of X rather than before it.
>
> Presumably this didn't matter in practice because the note added
> second tended to trump the note added first. But code is allowed to
> walk REG_NOTES without having to disregard secondary notes.
>
> 2017-12-23 Richard Sandiford <richard.sandif...@linaro.org>
>
> gcc/
> * expr.c (fixup_args_size_notes): Check that any existing
> REG_ARGS_SIZE notes are correct, and don't try to re-add them.
> (emit_single_push_insn_1): Move stack_pointer_delta adjustment to...
> (emit_single_push_insn): ...here.
OK.
jeff
