FreeChainXenon/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Simplify setjmp and non-local goto implementation (PR84521)

Browse source 
This fixes and simplifies the setjmp and non-local goto implementation.
Currently the virtual frame pointer is saved when using __builtin_setjmp or
a non-local goto.  Depending on whether a frame pointer is used, this may
either save SP or FP with an immediate offset.  However the goto or longjmp
always updates the hard frame pointer.

A receiver veneer in the original function then assigns the hard frame pointer
to the virtual frame pointer, which should, if it works correctly, again assign
SP or FP.  However the special elimination code in eliminate_regs_in_insn
doesn't do this correctly unless the frame pointer is used, and even if it
worked by writing SP, the frame pointer would still be corrupted.

A much simpler implementation is to always save and restore the hard frame
pointer.  This avoids 2 redundant instructions which add/subtract the virtual
frame offset.  A large amount of code can be removed as a result, including all
implementations of TARGET_BUILTIN_SETJMP_FRAME_VALUE (all of which already use
the hard frame pointer).  The expansion of nonlocal_goto on PA can be simplied
to just restore the hard frame pointer. 

This fixes the most obvious issues, however there are still issues on targets
which define HARD_FRAME_POINTER_IS_FRAME_POINTER (arm, mips).
Each function could have a different hard frame pointer, so a non-local goto
may restore the wrong frame pointer (TARGET_BUILTIN_SETJMP_FRAME_VALUE could
be useful for this).

The i386 TARGET_BUILTIN_SETJMP_FRAME_VALUE was incorrect: if stack_realign_fp
is true, it would save the hard frame pointer value but restore the virtual
frame pointer which according to ix86_initial_elimination_offset can have a
non-zero offset from the hard frame pointer.

The ia64 implementation of nonlocal_goto seems incorrect since the helper
function moves the the frame pointer value into the static chain register
(so this patch does nothing to make it better or worse).

AArch64 + x86-64 bootstrap OK, new test passes on AArch64, x86-64 and Arm.

gcc/
	PR middle-end/84521
	* builtins.c (expand_builtin_setjmp_setup): Save
	hard_frame_pointer_rtx.
	(expand_builtin_setjmp_receiver): Do not emit sfp = fp move since we
	restore fp.
	* function.c (expand_function_start): Save hard_frame_pointer_rtx for
	non-local goto.
	* lra-eliminations.c (eliminate_regs_in_insn): Remove sfp = fp
	elimination code.
	(remove_reg_equal_offset_note): Remove unused function.
	* reload1.c (eliminate_regs_in_insn): Remove sfp = hfp elimination
	code.
	* config/arc/arc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
	(arc_builtin_setjmp_frame_value): Remove function.
	* config/avr/avr.c  (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
	(avr_builtin_setjmp_frame_value): Remove function.
	* config/i386/i386.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
	(ix86_builtin_setjmp_frame_value): Remove function.
	* config/pa/pa.md (nonlocal_goto): Remove FP adjustment.
	* config/sparc/sparc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
	(sparc_builtin_setjmp_frame_value): Remove function.
	* config/vax/vax.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
	(vax_builtin_setjmp_frame_value): Remove function.
	* config/xtensa/xtensa.c (xtensa_frame_pointer_required): Force frame
	pointer	if has_nonlocal_label.

testsuite/
	PR middle-end/84521
	* gcc.c-torture/execute/pr84521.c: New test.

From-SVN: r272473
This commit is contained in:
Wilco Dijkstra 2019-06-19 12:52:43 +00:00 • committed by Wilco Dijkstra
parent 2e83f583c2
commit 25403c416e
14 changed files with 91 additions and 291 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
gcc/ChangeLog
View file

@ -1,3 +1,31 @@
2019-06-19 Wilco Dijkstra <wdijkstr@arm.com>
PR middle-end/84521
* builtins.c (expand_builtin_setjmp_setup): Save
hard_frame_pointer_rtx.
(expand_builtin_setjmp_receiver): Do not emit sfp = fp move since we
restore fp.
* function.c (expand_function_start): Save hard_frame_pointer_rtx for
non-local goto.
* lra-eliminations.c (eliminate_regs_in_insn): Remove sfp = fp
elimination code.
(remove_reg_equal_offset_note): Remove unused function.
* reload1.c (eliminate_regs_in_insn): Remove sfp = hfp elimination
code.
* config/arc/arc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(arc_builtin_setjmp_frame_value): Remove function.
* config/avr/avr.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(avr_builtin_setjmp_frame_value): Remove function.
* config/i386/i386.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(ix86_builtin_setjmp_frame_value): Remove function.
* config/pa/pa.md (nonlocal_goto): Remove FP adjustment.
* config/sparc/sparc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(sparc_builtin_setjmp_frame_value): Remove function.
* config/vax/vax.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(vax_builtin_setjmp_frame_value): Remove function.
* config/xtensa/xtensa.c (xtensa_frame_pointer_required): Force frame
pointer if has_nonlocal_label.
2019-06-19 Jakub Jelinek <jakub@redhat.com>
* doc/md.texi: Document vec_shl_<mode> pattern.

27
gcc/builtins.c
View file

@ -981,7 +981,7 @@ expand_builtin_setjmp_setup (rtx buf_addr, rtx receiver_label)
mem = gen_rtx_MEM (Pmode, buf_addr);
set_mem_alias_set (mem, setjmp_alias_set);
emit_move_insn (mem, targetm.builtin_setjmp_frame_value ());
emit_move_insn (mem, hard_frame_pointer_rtx);
mem = gen_rtx_MEM (Pmode, plus_constant (Pmode, buf_addr,
GET_MODE_SIZE (Pmode))),
@ -1023,31 +1023,6 @@ expand_builtin_setjmp_receiver (rtx receiver_label)
if (chain && REG_P (chain))
emit_clobber (chain);
/* Now put in the code to restore the frame pointer, and argument
pointer, if needed. */
if (! targetm.have_nonlocal_goto ())
{
/* First adjust our frame pointer to its actual value. It was
previously set to the start of the virtual area corresponding to
the stacked variables when we branched here and now needs to be
adjusted to the actual hardware fp value.
Assignments to virtual registers are converted by
instantiate_virtual_regs into the corresponding assignment
to the underlying register (fp in this case) that makes
the original assignment true.
So the following insn will actually be decrementing fp by
TARGET_STARTING_FRAME_OFFSET. */
emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
/* Restoring the frame pointer also modifies the hard frame pointer.
Mark it used (so that the previous assignment remains live once
the frame pointer is eliminated) and clobbered (to represent the
implicit update from the assignment). */
emit_use (hard_frame_pointer_rtx);
emit_clobber (hard_frame_pointer_rtx);
}
if (!HARD_FRAME_POINTER_IS_ARG_POINTER && fixed_regs[ARG_POINTER_REGNUM])
{
/* If the argument pointer can be eliminated in favor of the

24
gcc/config/arc/arc.c
View file

@ -689,8 +689,6 @@ static rtx arc_legitimize_address_0 (rtx, rtx, machine_mode mode);
#undef TARGET_MODES_TIEABLE_P
#define TARGET_MODES_TIEABLE_P arc_modes_tieable_p
#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
#define TARGET_BUILTIN_SETJMP_FRAME_VALUE arc_builtin_setjmp_frame_value
/* Try to keep the (mov:DF _, reg) as early as possible so
that the d<add/sub/mul>h-lr insns appear together and can
@ -10960,28 +10958,6 @@ compact_memory_operand_p (rtx op, machine_mode mode,
return false;
}
/* Return the frame pointer value to be backed up in the setjmp buffer. */
static rtx
arc_builtin_setjmp_frame_value (void)
{
/* We always want to preserve whatever value is currently in the frame
pointer register. For frames that are using the frame pointer the new
value of the frame pointer register will have already been computed
(as part of the prologue). For frames that are not using the frame
pointer it is important that we backup whatever value is in the frame
pointer register, as earlier (more outer) frames may have placed a
value into the frame pointer register. It might be tempting to try
and use `frame_pointer_rtx` here, however, this is not what we want.
For frames that are using the frame pointer this will give the
correct value. However, for frames that are not using the frame
pointer this will still give the value that _would_ have been the
frame pointer value for this frame (if the use of the frame pointer
had not been removed). We really do want the raw frame pointer
register value. */
return gen_raw_REG (Pmode, HARD_FRAME_POINTER_REGNUM);
}
/* Return nonzero if a jli call should be generated for a call from
the current function to DECL. */

16
gcc/config/avr/avr.c
View file

@ -1302,22 +1302,6 @@ avr_build_builtin_va_list (void)
}
/* Implement `TARGET_BUILTIN_SETJMP_FRAME_VALUE'. */
/* Actual start of frame is virtual_stack_vars_rtx this is offset from
frame pointer by +TARGET_STARTING_FRAME_OFFSET.
Using saved frame = virtual_stack_vars_rtx - TARGET_STARTING_FRAME_OFFSET
avoids creating add/sub of offset in nonlocal goto and setjmp. */
static rtx
avr_builtin_setjmp_frame_value (void)
{
rtx xval = gen_reg_rtx (Pmode);
emit_insn (gen_subhi3 (xval, virtual_stack_vars_rtx,
gen_int_mode (avr_starting_frame_offset (), Pmode)));
return xval;
}
/* Return contents of MEM at frame pointer + stack size + 1 (+2 if 3-byte PC).
This is return address of function. */

14
gcc/config/i386/i386.c
View file

@ -5788,17 +5788,6 @@ ix86_initial_elimination_offset (int from, int to)
}
}
/* In a dynamically-aligned function, we can't know the offset from
stack pointer to frame pointer, so we must ensure that setjmp
eliminates fp against the hard fp (%ebp) rather than trying to
index from %esp up to the top of the frame across a gap that is
of unknown (at compile-time) size. */
static rtx
ix86_builtin_setjmp_frame_value (void)
{
return stack_realign_fp ? hard_frame_pointer_rtx : virtual_stack_vars_rtx;
}
/* Emits a warning for unsupported msabi to sysv pro/epilogues. */
void warn_once_call_ms2sysv_xlogues (const char *feature)
{
@ -22767,9 +22756,6 @@ ix86_run_selftests (void)
#undef TARGET_MACHINE_DEPENDENT_REORG
#define TARGET_MACHINE_DEPENDENT_REORG ix86_reorg
#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
#define TARGET_BUILTIN_SETJMP_FRAME_VALUE ix86_builtin_setjmp_frame_value
#undef TARGET_BUILD_BUILTIN_VA_LIST
#define TARGET_BUILD_BUILTIN_VA_LIST ix86_build_builtin_va_list

7
gcc/config/pa/pa.md
View file

@ -6909,10 +6909,7 @@
lab = copy_to_reg (lab);
/* Restore the stack and frame pointers. The virtual_stack_vars_rtx
is saved instead of the hard_frame_pointer_rtx in the save area.
As a result, an extra instruction is needed to adjust for the offset
of the virtual stack variables and the hard frame pointer. */
/* Restore the stack and frame pointers. */
fp = copy_to_reg (fp);
emit_stack_restore (SAVE_NONLOCAL, stack);
@ -6920,7 +6917,7 @@
emit_insn (gen_blockage ());
emit_clobber (hard_frame_pointer_rtx);
emit_clobber (frame_pointer_rtx);
emit_move_insn (hard_frame_pointer_rtx, plus_constant (Pmode, fp, -8));
emit_move_insn (hard_frame_pointer_rtx, fp);
emit_use (hard_frame_pointer_rtx);
emit_use (stack_pointer_rtx);

12
gcc/config/sparc/sparc.c
View file

@ -679,7 +679,6 @@ static void sparc_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
static void sparc_file_end (void);
static bool sparc_frame_pointer_required (void);
static bool sparc_can_eliminate (const int, const int);
static rtx sparc_builtin_setjmp_frame_value (void);
static void sparc_conditional_register_usage (void);
static bool sparc_use_pseudo_pic_reg (void);
static void sparc_init_pic_reg (void);
@ -878,9 +877,6 @@ char sparc_hard_reg_printed[8];
#undef TARGET_FRAME_POINTER_REQUIRED
#define TARGET_FRAME_POINTER_REQUIRED sparc_frame_pointer_required
#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
#define TARGET_BUILTIN_SETJMP_FRAME_VALUE sparc_builtin_setjmp_frame_value
#undef TARGET_CAN_ELIMINATE
#define TARGET_CAN_ELIMINATE sparc_can_eliminate
@ -13003,14 +12999,6 @@ sparc_can_eliminate (const int from ATTRIBUTE_UNUSED, const int to)
return to == HARD_FRAME_POINTER_REGNUM || !sparc_frame_pointer_required ();
}
/* Return the hard frame pointer directly to bypass the stack bias. */
static rtx
sparc_builtin_setjmp_frame_value (void)
{
return hard_frame_pointer_rtx;
}
/* If !TARGET_FPU, then make the fp registers and fp cc regs fixed so that
they won't be allocated. */

10
gcc/config/vax/vax.c
View file

@ -59,7 +59,6 @@ static rtx vax_function_arg (cumulative_args_t, machine_mode,
static void vax_function_arg_advance (cumulative_args_t, machine_mode,
const_tree, bool);
static rtx vax_struct_value_rtx (tree, int);
static rtx vax_builtin_setjmp_frame_value (void);
static void vax_asm_trampoline_template (FILE *);
static void vax_trampoline_init (rtx, tree, rtx);
static poly_int64 vax_return_pops_args (tree, tree, poly_int64);
@ -99,9 +98,6 @@ static HOST_WIDE_INT vax_starting_frame_offset (void);
#undef TARGET_STRUCT_VALUE_RTX
#define TARGET_STRUCT_VALUE_RTX vax_struct_value_rtx
#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
#define TARGET_BUILTIN_SETJMP_FRAME_VALUE vax_builtin_setjmp_frame_value
#undef TARGET_LRA_P
#define TARGET_LRA_P hook_bool_void_false
@ -1067,12 +1063,6 @@ vax_struct_value_rtx (tree fntype ATTRIBUTE_UNUSED,
return gen_rtx_REG (Pmode, VAX_STRUCT_VALUE_REGNUM);
}
static rtx
vax_builtin_setjmp_frame_value (void)
{
return hard_frame_pointer_rtx;
}
/* Worker function for NOTICE_UPDATE_CC. */
void

2
gcc/config/xtensa/xtensa.c
View file

@ -2739,7 +2739,7 @@ xtensa_frame_pointer_required (void)
This seems wrong but maybe it's necessary for other architectures.
This function is derived from the i386 code. */
if (cfun->machine->accesses_prev_frame)
if (cfun->machine->accesses_prev_frame || cfun->has_nonlocal_label)
return true;
return false;

2
gcc/function.c
View file

@ -5135,7 +5135,7 @@ expand_function_start (tree subr)
r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE);
gcc_assert (GET_MODE (r_save) == Pmode);
emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ());
emit_move_insn (r_save, hard_frame_pointer_rtx);
update_nonlocal_goto_save_area ();
}

92
gcc/lra-eliminations.c
View file

@ -876,32 +876,6 @@ mark_not_eliminable (rtx x, machine_mode mem_mode)
#ifdef HARD_FRAME_POINTER_REGNUM
/* Search INSN's reg notes to see whether the destination is equal to
WHAT + C for some constant C. Return true if so, storing C in
*OFFSET_OUT and removing the reg note. */
static bool
remove_reg_equal_offset_note (rtx_insn *insn, rtx what, poly_int64 *offset_out)
{
rtx link, *link_loc;
for (link_loc = &REG_NOTES (insn);
(link = *link_loc) != NULL_RTX;
link_loc = &XEXP (link, 1))
if (REG_NOTE_KIND (link) == REG_EQUAL
&& GET_CODE (XEXP (link, 0)) == PLUS
&& XEXP (XEXP (link, 0), 0) == what
&& poly_int_rtx_p (XEXP (XEXP (link, 0), 1), offset_out))
{
*link_loc = XEXP (link, 1);
return true;
}
return false;
}
#endif
/* Scan INSN and eliminate all eliminable hard registers in it.
If REPLACE_P is true, do the replacement destructively. Also
@ -939,72 +913,6 @@ eliminate_regs_in_insn (rtx_insn *insn, bool replace_p, bool first_p,
return;
}
/* Check for setting an eliminable register. */
if (old_set != 0 && REG_P (SET_DEST (old_set))
&& (ep = get_elimination (SET_DEST (old_set))) != NULL)
{
for (ep = reg_eliminate; ep < &reg_eliminate[NUM_ELIMINABLE_REGS]; ep++)
if (ep->from_rtx == SET_DEST (old_set) && ep->can_eliminate)
{
bool delete_p = replace_p;
#ifdef HARD_FRAME_POINTER_REGNUM
if (ep->from == FRAME_POINTER_REGNUM
&& ep->to == HARD_FRAME_POINTER_REGNUM)
/* If this is setting the frame pointer register to the
hardware frame pointer register and this is an
elimination that will be done (tested above), this
insn is really adjusting the frame pointer downward
to compensate for the adjustment done before a
nonlocal goto. */
{
rtx src = SET_SRC (old_set);
poly_int64 offset = 0;
/* We should never process such insn with non-zero
UPDATE_SP_OFFSET. */
lra_assert (known_eq (update_sp_offset, 0));
if (remove_reg_equal_offset_note (insn, ep->to_rtx, &offset)
|| strip_offset (src, &offset) == ep->to_rtx)
{
if (replace_p)
{
SET_DEST (old_set) = ep->to_rtx;
lra_update_insn_recog_data (insn);
return;
}
offset -= (ep->offset - ep->previous_offset);
src = plus_constant (Pmode, ep->to_rtx, offset);
/* First see if this insn remains valid when we
make the change. If not, keep the INSN_CODE
the same and let the constraint pass fit it
up. */
validate_change (insn, &SET_SRC (old_set), src, 1);
validate_change (insn, &SET_DEST (old_set),
ep->from_rtx, 1);
if (! apply_change_group ())
{
SET_SRC (old_set) = src;
SET_DEST (old_set) = ep->from_rtx;
}
lra_update_insn_recog_data (insn);
/* Add offset note for future updates. */
add_reg_note (insn, REG_EQUAL, copy_rtx (src));
return;
}
}
#endif
/* This insn isn't serving a useful purpose. We delete it
when REPLACE is set. */
if (delete_p)
lra_delete_dead_insn (insn);
return;
}
}
/* We allow one special case which happens to work on all machines we
currently support: a single set with the source or a REG_EQUAL
note being a PLUS of an eliminable register and a constant. */

90
gcc/reload1.c
View file

@ -3234,96 +3234,6 @@ eliminate_regs_in_insn (rtx_insn *insn, int replace)
return 0;
}
if (old_set != 0 && REG_P (SET_DEST (old_set))
&& REGNO (SET_DEST (old_set)) < FIRST_PSEUDO_REGISTER)
{
/* Check for setting an eliminable register. */
for (ep = reg_eliminate; ep < &reg_eliminate[NUM_ELIMINABLE_REGS]; ep++)
if (ep->from_rtx == SET_DEST (old_set) && ep->can_eliminate)
{
/* If this is setting the frame pointer register to the
hardware frame pointer register and this is an elimination
that will be done (tested above), this insn is really
adjusting the frame pointer downward to compensate for
the adjustment done before a nonlocal goto. */
if (!HARD_FRAME_POINTER_IS_FRAME_POINTER
&& ep->from == FRAME_POINTER_REGNUM
&& ep->to == HARD_FRAME_POINTER_REGNUM)
{
rtx base = SET_SRC (old_set);
rtx_insn *base_insn = insn;
HOST_WIDE_INT offset = 0;
while (base != ep->to_rtx)
{
rtx_insn *prev_insn;
rtx prev_set;
if (GET_CODE (base) == PLUS
&& CONST_INT_P (XEXP (base, 1)))
{
offset += INTVAL (XEXP (base, 1));
base = XEXP (base, 0);
}
else if ((prev_insn = prev_nonnote_insn (base_insn)) != 0
&& (prev_set = single_set (prev_insn)) != 0
&& rtx_equal_p (SET_DEST (prev_set), base))
{
base = SET_SRC (prev_set);
base_insn = prev_insn;
}
else
break;
}
if (base == ep->to_rtx)
{
rtx src = plus_constant (Pmode, ep->to_rtx,
offset - ep->offset);
new_body = old_body;
if (! replace)
{
new_body = copy_insn (old_body);
if (REG_NOTES (insn))
REG_NOTES (insn) = copy_insn_1 (REG_NOTES (insn));
}
PATTERN (insn) = new_body;
old_set = single_set (insn);
/* First see if this insn remains valid when we
make the change. If not, keep the INSN_CODE
the same and let reload fit it up. */
validate_change (insn, &SET_SRC (old_set), src, 1);
validate_change (insn, &SET_DEST (old_set),
ep->to_rtx, 1);
if (! apply_change_group ())
{
SET_SRC (old_set) = src;
SET_DEST (old_set) = ep->to_rtx;
}
val = 1;
goto done;
}
}
/* In this case this insn isn't serving a useful purpose. We
will delete it in reload_as_needed once we know that this
elimination is, in fact, being done.
If REPLACE isn't set, we can't delete this insn, but needn't
process it since it won't be used unless something changes. */
if (replace)
{
delete_dead_insn (insn);
return 1;
}
val = 1;
goto done;
}
}
/* We allow one special case which happens to work on all machines we
currently support: a single set with the source or a REG_EQUAL
note being a PLUS of an eliminable register and a constant. */

5
gcc/testsuite/ChangeLog
View file

@ -1,3 +1,8 @@
2019-06-19 Wilco Dijkstra <wdijkstr@arm.com>
PR middle-end/84521
* gcc.c-torture/execute/pr84521.c: New test.
2019-06-19 Jakub Jelinek <jakub@redhat.com>
* gcc.dg/vect/vect-simd-8.c: If main is defined, don't include

53
gcc/testsuite/gcc.c-torture/execute/pr84521.c Normal file
View file

@ -0,0 +1,53 @@
/* { dg-require-effective-target indirect_jumps } */
/* { dg-additional-options "-fomit-frame-pointer -fno-inline" } */
extern void abort (void);
void
broken_longjmp (void *p)
{
__builtin_longjmp (p, 1);
}
volatile int x = 256;
void *volatile p = (void*)&x;
void *volatile p1;
void
test (void)
{
void *buf[5];
void *volatile q = p;
if (!__builtin_setjmp (buf))
broken_longjmp (buf);
/* Fails if stack pointer corrupted. */
if (p != q)
abort ();
}
void
test2 (void)
{
void *volatile q = p;
p1 = __builtin_alloca (x);
test ();
/* Fails if frame pointer corrupted. */
if (p != q)
abort ();
}
int
main (void)
{
void *volatile q = p;
test ();
test2 ();
/* Fails if stack pointer corrupted. */
if (p != q)
abort ();
return 0;
}