Skip to content

Commit d7ea903

Browse files
author
rsandifo
committed
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 <[email protected]> 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. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@256105 138bc75d-0d04-0410-961f-82ee72b054a4
1 parent 0f78b37 commit d7ea903

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

gcc/ChangeLog

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
2018-01-02 Richard Sandiford <[email protected]>
2+
3+
* expr.c (fixup_args_size_notes): Check that any existing
4+
REG_ARGS_SIZE notes are correct, and don't try to re-add them.
5+
(emit_single_push_insn_1): Move stack_pointer_delta adjustment to...
6+
(emit_single_push_insn): ...here.
7+
18
2018-01-02 Richard Sandiford <[email protected]>
29

310
* rtl.h (CONST_VECTOR_ELT): Redefine to const_vector_elt.

gcc/expr.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,6 +4090,14 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last,
40904090
if (!NONDEBUG_INSN_P (insn))
40914091
continue;
40924092

4093+
/* We might have existing REG_ARGS_SIZE notes, e.g. when pushing
4094+
a call argument containing a TLS address that itself requires
4095+
a call to __tls_get_addr. The handling of stack_pointer_delta
4096+
in emit_single_push_insn is supposed to ensure that any such
4097+
notes are already correct. */
4098+
rtx note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX);
4099+
gcc_assert (!note || known_eq (args_size, get_args_size (note)));
4100+
40934101
poly_int64 this_delta = find_args_size_adjust (insn);
40944102
if (known_eq (this_delta, 0))
40954103
{
@@ -4103,7 +4111,8 @@ fixup_args_size_notes (rtx_insn *prev, rtx_insn *last,
41034111
if (known_eq (this_delta, HOST_WIDE_INT_MIN))
41044112
saw_unknown = true;
41054113

4106-
add_args_size_note (insn, args_size);
4114+
if (!note)
4115+
add_args_size_note (insn, args_size);
41074116
if (STACK_GROWS_DOWNWARD)
41084117
this_delta = -poly_uint64 (this_delta);
41094118

@@ -4127,7 +4136,6 @@ emit_single_push_insn_1 (machine_mode mode, rtx x, tree type)
41274136
rtx dest;
41284137
enum insn_code icode;
41294138

4130-
stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode));
41314139
/* If there is push pattern, use it. Otherwise try old way of throwing
41324140
MEM representing push operation to move expander. */
41334141
icode = optab_handler (push_optab, mode);
@@ -4214,6 +4222,14 @@ emit_single_push_insn (machine_mode mode, rtx x, tree type)
42144222

42154223
emit_single_push_insn_1 (mode, x, type);
42164224

4225+
/* Adjust stack_pointer_delta to describe the situation after the push
4226+
we just performed. Note that we must do this after the push rather
4227+
than before the push in case calculating X needs pushes and pops of
4228+
its own (e.g. if calling __tls_get_addr). The REG_ARGS_SIZE notes
4229+
for such pushes and pops must not include the effect of the future
4230+
push of X. */
4231+
stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode));
4232+
42174233
last = get_last_insn ();
42184234

42194235
/* Notice the common case where we emitted exactly one insn. */

0 commit comments

Comments
 (0)