Commit 2bc6986d by Richard Sandiford Committed by Richard Sandiford

Fix REG_ARGS_SIZE handling when pushing TLS addresses

The new assert in add_args_size_note triggered for gcc.dg/tls/opt-3.c
and others 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.

2018-01-02  Richard Sandiford  <richard.sandiford@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.

From-SVN: r256105
parent cd5ff7bc
2018-01-02 Richard Sandiford <richard.sandiford@linaro.org> 2018-01-02 Richard Sandiford <richard.sandiford@linaro.org>
* 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.
2018-01-02 Richard Sandiford <richard.sandiford@linaro.org>
* rtl.h (CONST_VECTOR_ELT): Redefine to const_vector_elt. * rtl.h (CONST_VECTOR_ELT): Redefine to const_vector_elt.
(const_vector_encoded_nelts): New function. (const_vector_encoded_nelts): New function.
(CONST_VECTOR_NUNITS): Redefine to use GET_MODE_NUNITS. (CONST_VECTOR_NUNITS): Redefine to use GET_MODE_NUNITS.
...@@ -4090,6 +4090,14 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last, ...@@ -4090,6 +4090,14 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last,
if (!NONDEBUG_INSN_P (insn)) if (!NONDEBUG_INSN_P (insn))
continue; continue;
/* We might have existing REG_ARGS_SIZE notes, e.g. when pushing
a call argument containing a TLS address that itself requires
a call to __tls_get_addr. The handling of stack_pointer_delta
in emit_single_push_insn is supposed to ensure that any such
notes are already correct. */
rtx note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX);
gcc_assert (!note || known_eq (args_size, get_args_size (note)));
poly_int64 this_delta = find_args_size_adjust (insn); poly_int64 this_delta = find_args_size_adjust (insn);
if (known_eq (this_delta, 0)) if (known_eq (this_delta, 0))
{ {
...@@ -4103,7 +4111,8 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last, ...@@ -4103,7 +4111,8 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last,
if (known_eq (this_delta, HOST_WIDE_INT_MIN)) if (known_eq (this_delta, HOST_WIDE_INT_MIN))
saw_unknown = true; saw_unknown = true;
add_args_size_note (insn, args_size); if (!note)
add_args_size_note (insn, args_size);
if (STACK_GROWS_DOWNWARD) if (STACK_GROWS_DOWNWARD)
this_delta = -poly_uint64 (this_delta); this_delta = -poly_uint64 (this_delta);
...@@ -4127,7 +4136,6 @@ emit_single_push_insn_1 (machine_mode mode, rtx x, tree type) ...@@ -4127,7 +4136,6 @@ emit_single_push_insn_1 (machine_mode mode, rtx x, tree type)
rtx dest; rtx dest;
enum insn_code icode; enum insn_code icode;
stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode));
/* If there is push pattern, use it. Otherwise try old way of throwing /* If there is push pattern, use it. Otherwise try old way of throwing
MEM representing push operation to move expander. */ MEM representing push operation to move expander. */
icode = optab_handler (push_optab, mode); icode = optab_handler (push_optab, mode);
...@@ -4214,6 +4222,14 @@ emit_single_push_insn (machine_mode mode, rtx x, tree type) ...@@ -4214,6 +4222,14 @@ emit_single_push_insn (machine_mode mode, rtx x, tree type)
emit_single_push_insn_1 (mode, x, type); emit_single_push_insn_1 (mode, x, type);
/* Adjust stack_pointer_delta to describe the situation after the push
we just performed. Note that we must do this after the push rather
than before the push in case calculating X needs pushes and pops of
its own (e.g. if calling __tls_get_addr). The REG_ARGS_SIZE notes
for such pushes and pops must not include the effect of the future
push of X. */
stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode));
last = get_last_insn (); last = get_last_insn ();
/* Notice the common case where we emitted exactly one insn. */ /* Notice the common case where we emitted exactly one insn. */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment