[IRA,LRA] Fix PR87466, all pseudos live across setjmp are spilled

Currently, both IRA and LRA spill all pseudo regs that are live across a
setjmp call.  If the target has a sane setjmp, then the compiler should not
have to treat the setjmp call any differently than is does any other normal
function call.  Namely, just mark all pseudos that are live across the setjmp
as conflicting with the volatile registers.

This issue was discussed in the following gcc mailing list thread:

  https://gcc.gnu.org/ml/gcc/2018-03/msg00014.html

...and some people mentioned that some systems do not have sane setjmp
implementations and therefore need the spill all pseudos live across setjmps
to get correct functionality.  It was decided in the thread above that we
should create a target hook that can allow targets to tell IRA and LRA
whether or not they have a sane setjmp implementation.  The following patch
implements that idea along with converting the rs6000 port to use the hook.

This patch passed bootstrap and regtesting on powerpc64le-linux with
no regressions.  Ok for trunk?

Peter


gcc/
	PR rtl-optimization/87466
	* target.def (is_reg_clobbering_setjmp_p): New target hook.
	* doc/tm.texi.in (TARGET_IS_REG_CLOBBERING_SETJMP_P): New hook.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_is_reg_clobbering_setjmp_p): Declare.
	* targhooks.h (default_is_reg_clobbering_setjmp_p): New function.
	* ira-lives.c (process_bb_node_lives): Use the new target hook.
	* lra-lives.c (process_bb_lives): Likewise.
	* config/rs6000/rs6000.c (TARGET_IS_REG_CLOBBERING_SETJMP_P): Define.
	(rs6000_is_reg_clobbering_setjmp_p): New function.

gcc/testsuite/
	PR rtl-optimization/87466
	* gcc.target/powerpc/pr87466.c: New test.

On Sat, Sep 29, 2018 at 5:12 AM Peter Bergner <bergner@linux.ibm.com> wrote:
>
> Currently, both IRA and LRA spill all pseudo regs that are live across a
> setjmp call.  If the target has a sane setjmp, then the compiler should not
> have to treat the setjmp call any differently than is does any other normal
> function call.  Namely, just mark all pseudos that are live across the setjmp
> as conflicting with the volatile registers.
>
> This issue was discussed in the following gcc mailing list thread:
>
>   https://gcc.gnu.org/ml/gcc/2018-03/msg00014.html
>
> ...and some people mentioned that some systems do not have sane setjmp
> implementations and therefore need the spill all pseudos live across setjmps
> to get correct functionality.  It was decided in the thread above that we
> should create a target hook that can allow targets to tell IRA and LRA
> whether or not they have a sane setjmp implementation.  The following patch
> implements that idea along with converting the rs6000 port to use the hook.
>
> This patch passed bootstrap and regtesting on powerpc64le-linux with
> no regressions.  Ok for trunk?

LGTM.  Please leave others the opportunity to comment.

Thanks,
Richard.

> Peter
>
>
> gcc/
>         PR rtl-optimization/87466
>         * target.def (is_reg_clobbering_setjmp_p): New target hook.
>         * doc/tm.texi.in (TARGET_IS_REG_CLOBBERING_SETJMP_P): New hook.
>         * doc/tm.texi: Regenerate.
>         * targhooks.c (default_is_reg_clobbering_setjmp_p): Declare.
>         * targhooks.h (default_is_reg_clobbering_setjmp_p): New function.
>         * ira-lives.c (process_bb_node_lives): Use the new target hook.
>         * lra-lives.c (process_bb_lives): Likewise.
>         * config/rs6000/rs6000.c (TARGET_IS_REG_CLOBBERING_SETJMP_P): Define.
>         (rs6000_is_reg_clobbering_setjmp_p): New function.
>
> gcc/testsuite/
>         PR rtl-optimization/87466
>         * gcc.target/powerpc/pr87466.c: New test.
>
> Index: gcc/target.def
> ===================================================================
> --- gcc/target.def      (revision 264698)
> +++ gcc/target.def      (working copy)
> @@ -3123,6 +3123,20 @@ In order to enforce the representation o
>   int, (scalar_int_mode mode, scalar_int_mode rep_mode),
>   default_mode_rep_extended)
>
> + DEFHOOK
> +(is_reg_clobbering_setjmp_p,
> + "On some targets, it is assumed that the compiler will spill all registers\n\
> +  that are live across a call to @code{setjmp}, while other targets treat\n\
> +  @code{setjmp} calls as normal function calls.\n\
> +  \n\
> +  This hook returns true if @var{insn} is a @code{setjmp} call that must\n\
> +  have all registers that are live across it spilled.  Define this to return\n\
> +  false if the target does not need to spill all registers across calls to\n\
> +  @code{setjmp} calls.  The default implementation conservatively assumes all\n\
> +  registers must be spilled across @code{setjmp} calls.",
> +bool, (const rtx_insn *insn),
> +default_is_reg_clobbering_setjmp_p)
> +
>  /* True if MODE is valid for a pointer in __attribute__((mode("MODE"))).  */
>  DEFHOOK
>  (valid_pointer_mode,
> Index: gcc/doc/tm.texi.in
> ===================================================================
> --- gcc/doc/tm.texi.in  (revision 264698)
> +++ gcc/doc/tm.texi.in  (working copy)
> @@ -7507,6 +7507,8 @@ You need not define this macro if it wou
>
>  @hook TARGET_MODE_REP_EXTENDED
>
> +@hook TARGET_IS_REG_CLOBBERING_SETJMP_P
> +
>  @defmac STORE_FLAG_VALUE
>  A C expression describing the value returned by a comparison operator
>  with an integral mode and stored by a store-flag instruction
> Index: gcc/doc/tm.texi
> ===================================================================
> --- gcc/doc/tm.texi     (revision 264698)
> +++ gcc/doc/tm.texi     (working copy)
> @@ -11000,6 +11000,18 @@ In order to enforce the representation o
>  @code{mode}.
>  @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_IS_REG_CLOBBERING_SETJMP_P (const rtx_insn *@var{insn})
> +On some targets, it is assumed that the compiler will spill all registers
> +  that are live across a call to @code{setjmp}, while other targets treat
> +  @code{setjmp} calls as normal function calls.
> +
> +  This hook returns true if @var{insn} is a @code{setjmp} call that must
> +  have all registers that are live across it spilled.  Define this to return
> +  false if the target does not need to spill all registers across calls to
> +  @code{setjmp} calls.  The default implementation conservatively assumes all
> +  registers must be spilled across @code{setjmp} calls.
> +@end deftypefn
> +
>  @defmac STORE_FLAG_VALUE
>  A C expression describing the value returned by a comparison operator
>  with an integral mode and stored by a store-flag instruction
> Index: gcc/targhooks.c
> ===================================================================
> --- gcc/targhooks.c     (revision 264698)
> +++ gcc/targhooks.c     (working copy)
> @@ -209,6 +209,15 @@ default_builtin_setjmp_frame_value (void
>    return virtual_stack_vars_rtx;
>  }
>
> +/* The default implementation of TARGET_IS_REG_CLOBBERING_SETJMP_P.  */
> +
> +bool
> +default_is_reg_clobbering_setjmp_p (const rtx_insn *insn)
> +{
> +  return CALL_P (insn)
> +        && find_reg_note (insn, REG_SETJMP, NULL_RTX) != NULL_RTX;
> +}
> +
>  /* Generic hook that takes a CUMULATIVE_ARGS pointer and returns false.  */
>
>  bool
> Index: gcc/targhooks.h
> ===================================================================
> --- gcc/targhooks.h     (revision 264698)
> +++ gcc/targhooks.h     (working copy)
> @@ -42,6 +42,7 @@ extern bool default_return_in_memory (co
>  extern rtx default_expand_builtin_saveregs (void);
>  extern void default_setup_incoming_varargs (cumulative_args_t, machine_mode, tree, int *, int);
>  extern rtx default_builtin_setjmp_frame_value (void);
> +extern bool default_is_reg_clobbering_setjmp_p (const rtx_insn *);
>  extern bool default_pretend_outgoing_varargs_named (cumulative_args_t);
>
>  extern scalar_int_mode default_eh_return_filter_mode (void);
> Index: gcc/ira-lives.c
> ===================================================================
> --- gcc/ira-lives.c     (revision 264698)
> +++ gcc/ira-lives.c     (working copy)
> @@ -1209,8 +1209,7 @@ process_bb_node_lives (ira_loop_tree_nod
>                      call, if this function receives a nonlocal
>                      goto.  */
>                   if (cfun->has_nonlocal_label
> -                     || find_reg_note (insn, REG_SETJMP,
> -                                       NULL_RTX) != NULL_RTX)
> +                     || targetm.is_reg_clobbering_setjmp_p (insn))
>                     {
>                       SET_HARD_REG_SET (OBJECT_CONFLICT_HARD_REGS (obj));
>                       SET_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
> Index: gcc/lra-lives.c
> ===================================================================
> --- gcc/lra-lives.c     (revision 264698)
> +++ gcc/lra-lives.c     (working copy)
> @@ -893,8 +893,7 @@ process_bb_lives (basic_block bb, int &c
>           sparseset_ior (pseudos_live_through_calls,
>                          pseudos_live_through_calls, pseudos_live);
>           if (cfun->has_nonlocal_label
> -             || find_reg_note (curr_insn, REG_SETJMP,
> -                               NULL_RTX) != NULL_RTX)
> +             || targetm.is_reg_clobbering_setjmp_p (curr_insn))
>             sparseset_ior (pseudos_live_through_setjumps,
>                            pseudos_live_through_setjumps, pseudos_live);
>         }
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 264698)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -1978,6 +1978,9 @@ static const struct attribute_spec rs600
>  #undef TARGET_ASM_GLOBALIZE_DECL_NAME
>  #define TARGET_ASM_GLOBALIZE_DECL_NAME rs6000_globalize_decl_name
>  #endif
> +
> +#undef TARGET_IS_REG_CLOBBERING_SETJMP_P
> +#define TARGET_IS_REG_CLOBBERING_SETJMP_P rs6000_is_reg_clobbering_setjmp_p
>
>
>  /* Processor table.  */
> @@ -38696,6 +38699,14 @@ rs6000_starting_frame_offset (void)
>      return 0;
>    return RS6000_STARTING_FRAME_OFFSET;
>  }
> +
> +/* Implement TARGET_IS_REG_CLOBBERING_SETJMP_P.  */
> +
> +static bool
> +rs6000_is_reg_clobbering_setjmp_p (const rtx_insn *insn ATTRIBUTE_UNUSED)
> +{
> +  return false;
> +}
>
>
>  /* Create an alias for a mangled name where we have changed the mangling (in
> Index: gcc/testsuite/gcc.target/powerpc/pr87466.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr87466.c  (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr87466.c  (working copy)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile { target powerpc*-*-* } } */
> +/* { dg-options "-O2" } */
> +
> +#include <stdlib.h>
> +#include <setjmp.h>
> +
> +extern void foo (jmp_buf);
> +
> +long
> +c (long var)
> +{
> +  jmp_buf env;
> +  if (setjmp(env) != 0)
> +    abort();
> +  foo (env);
> +  return var;
> +}
> +
> +/* { dg-final { scan-assembler {\mmr\M} } } */
>

Hi Peter,

On Fri, Sep 28, 2018 at 10:12:02PM -0500, Peter Bergner wrote:
> Currently, both IRA and LRA spill all pseudo regs that are live across a
> setjmp call.  If the target has a sane setjmp, then the compiler should not
> have to treat the setjmp call any differently than is does any other normal
> function call.  Namely, just mark all pseudos that are live across the setjmp
> as conflicting with the volatile registers.
> 
> This issue was discussed in the following gcc mailing list thread:
> 
>   https://gcc.gnu.org/ml/gcc/2018-03/msg00014.html
> 
> ...and some people mentioned that some systems do not have sane setjmp
> implementations and therefore need the spill all pseudos live across setjmps
> to get correct functionality.  It was decided in the thread above that we
> should create a target hook that can allow targets to tell IRA and LRA
> whether or not they have a sane setjmp implementation.  The following patch
> implements that idea along with converting the rs6000 port to use the hook.

> +bool
> +default_is_reg_clobbering_setjmp_p (const rtx_insn *insn)
> +{
> +  return CALL_P (insn)
> +	 && find_reg_note (insn, REG_SETJMP, NULL_RTX) != NULL_RTX;
> +}

Since all implementations of this hook will have to do the same, I think
it is better if you leave this test at the (only two) callers.  The hook
doesn't need an argument then, and maybe is better named something like
setjmp_is_normal_call?  (The original code did not test CALL_P btw).

(Whatever you end up with, the rs6000 part is of course pre-approved).


Segher

> Since all implementations of this hook will have to do the same, I think
> it is better if you leave this test at the (only two) callers.  The hook
> doesn't need an argument then, and maybe is better named something like
> setjmp_is_normal_call?  (The original code did not test CALL_P btw).

Seconded, but I'd be even more explicit in the naming of the hook, for example 
setjmp_preserves_nonvolatile_registers or somesuch.  (And I don't think that 
setjmp can be considered a normal call in any case since it returns twice).

On Mon, Oct 01, 2018 at 11:25:21AM +0200, Eric Botcazou wrote:
> > Since all implementations of this hook will have to do the same, I think
> > it is better if you leave this test at the (only two) callers.  The hook
> > doesn't need an argument then, and maybe is better named something like
> > setjmp_is_normal_call?  (The original code did not test CALL_P btw).
> 
> Seconded, but I'd be even more explicit in the naming of the hook, for example 
> setjmp_preserves_nonvolatile_registers or somesuch.  (And I don't think that 
> setjmp can be considered a normal call in any case since it returns twice).

Right...  I meant setjmp has the normal calling convention, the normal
call ABI.  It doesn't really matter to have a longer name here, it is
only used twice (and that code can be factored out to some helper
function, even).


Segher

On 10/1/18 4:25 AM, Eric Botcazou wrote:
>> Since all implementations of this hook will have to do the same, I think
>> it is better if you leave this test at the (only two) callers.  The hook
>> doesn't need an argument then, and maybe is better named something like
>> setjmp_is_normal_call?  (The original code did not test CALL_P btw).
> 
> Seconded, but I'd be even more explicit in the naming of the hook, for example 
> setjmp_preserves_nonvolatile_registers or somesuch.  (And I don't think that 
> setjmp can be considered a normal call in any case since it returns twice).

Ok, here is an updated patch that renames the hook using Eric's suggestion
and keeps the scanning of the SETJMP note in the callers of the hook like
Segher wanted.

This is currently bootstrapping right now, but ok for trunk assuming no
regressions?

Peter

gcc/
	PR rtl-optimization/87466
	* target.def (setjmp_preserves_nonvolatile_regs_p): New target hook.
	* doc/tm.texi.in (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P): New hook.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_setjmp_preserves_nonvolatile_regs_p): Declare.
	* targhooks.h (default_setjmp_preserves_nonvolatile_regs_p): New
	function.
	* ira-lives.c (process_bb_node_lives): Use the new target hook.
	* lra-lives.c (process_bb_lives): Likewise.
	* config/rs6000/rs6000.c (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P):
	Define.
	(rs6000_setjmp_preserves_nonvolatile_regs_p): New function.

gcc/testsuite/
	PR rtl-optimization/87466
	* gcc.target/powerpc/pr87466.c: New test.

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 264795)
+++ gcc/target.def	(working copy)
@@ -3123,6 +3123,21 @@ In order to enforce the representation o
  int, (scalar_int_mode mode, scalar_int_mode rep_mode),
  default_mode_rep_extended)
 
+ DEFHOOK
+(setjmp_preserves_nonvolatile_regs_p,
+ "On some targets, it is assumed that the compiler will spill all pseudos\n\
+  that are live across a call to @code{setjmp}, while other targets treat\n\
+  @code{setjmp} calls as normal function calls.\n\
+  \n\
+  This hook returns false if @code{setjmp} calls do not preserve all\n\
+  non-volatile registers so that gcc that must spill all pseudos that are\n\
+  live across @code{setjmp} calls.  Define this to return true if the\n\
+  target does not need to spill all pseudos live across @code{setjmp} calls.\n\
+  The default implementation conservatively assumes all pseudos must be\n\
+  spilled across @code{setjmp} calls.",
+ bool, (void),
+ default_setjmp_preserves_nonvolatile_regs_p)
+
 /* True if MODE is valid for a pointer in __attribute__((mode("MODE"))).  */
 DEFHOOK
 (valid_pointer_mode,
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 264795)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -7509,6 +7509,8 @@ You need not define this macro if it wou
 
 @hook TARGET_MODE_REP_EXTENDED
 
+@hook TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P
+
 @defmac STORE_FLAG_VALUE
 A C expression describing the value returned by a comparison operator
 with an integral mode and stored by a store-flag instruction
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 264795)
+++ gcc/doc/tm.texi	(working copy)
@@ -11008,6 +11008,19 @@ In order to enforce the representation o
 @code{mode}.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P (void)
+On some targets, it is assumed that the compiler will spill all pseudos
+  that are live across a call to @code{setjmp}, while other targets treat
+  @code{setjmp} calls as normal function calls.
+  
+  This hook returns false if @code{setjmp} calls do not preserve all
+  non-volatile registers so that gcc that must spill all pseudos that are
+  live across @code{setjmp} calls.  Define this to return true if the
+  target does not need to spill all pseudos live across @code{setjmp} calls.
+  The default implementation conservatively assumes all pseudos must be
+  spilled across @code{setjmp} calls.
+@end deftypefn
+
 @defmac STORE_FLAG_VALUE
 A C expression describing the value returned by a comparison operator
 with an integral mode and stored by a store-flag instruction
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	(revision 264795)
+++ gcc/targhooks.c	(working copy)
@@ -209,6 +209,14 @@ default_builtin_setjmp_frame_value (void
   return virtual_stack_vars_rtx;
 }
 
+/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
+
+bool
+default_setjmp_preserves_nonvolatile_regs_p (void)
+{
+  return false;
+}
+
 /* Generic hook that takes a CUMULATIVE_ARGS pointer and returns false.  */
 
 bool
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h	(revision 264795)
+++ gcc/targhooks.h	(working copy)
@@ -42,6 +42,7 @@ extern bool default_return_in_memory (co
 extern rtx default_expand_builtin_saveregs (void);
 extern void default_setup_incoming_varargs (cumulative_args_t, machine_mode, tree, int *, int);
 extern rtx default_builtin_setjmp_frame_value (void);
+extern bool default_setjmp_preserves_nonvolatile_regs_p (void);
 extern bool default_pretend_outgoing_varargs_named (cumulative_args_t);
 
 extern scalar_int_mode default_eh_return_filter_mode (void);
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c	(revision 264795)
+++ gcc/ira-lives.c	(working copy)
@@ -1207,8 +1207,9 @@ process_bb_node_lives (ira_loop_tree_nod
 		     call, if this function receives a nonlocal
 		     goto.  */
 		  if (cfun->has_nonlocal_label
-		      || find_reg_note (insn, REG_SETJMP,
-					NULL_RTX) != NULL_RTX)
+		      || (!targetm.setjmp_preserves_nonvolatile_regs_p ()
+			  && (find_reg_note (insn, REG_SETJMP, NULL_RTX)
+			      != NULL_RTX)))
 		    {
 		      SET_HARD_REG_SET (OBJECT_CONFLICT_HARD_REGS (obj));
 		      SET_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
Index: gcc/lra-lives.c
===================================================================
--- gcc/lra-lives.c	(revision 264795)
+++ gcc/lra-lives.c	(working copy)
@@ -895,8 +895,9 @@ process_bb_lives (basic_block bb, int &c
 	  sparseset_ior (pseudos_live_through_calls,
 			 pseudos_live_through_calls, pseudos_live);
 	  if (cfun->has_nonlocal_label
-	      || find_reg_note (curr_insn, REG_SETJMP,
-				NULL_RTX) != NULL_RTX)
+	      || (!targetm.setjmp_preserves_nonvolatile_regs_p ()
+		  && (find_reg_note (curr_insn, REG_SETJMP, NULL_RTX)
+		      != NULL_RTX)))
 	    sparseset_ior (pseudos_live_through_setjumps,
 			   pseudos_live_through_setjumps, pseudos_live);
 	}
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 264795)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1978,6 +1978,10 @@ static const struct attribute_spec rs600
 #undef TARGET_ASM_GLOBALIZE_DECL_NAME
 #define TARGET_ASM_GLOBALIZE_DECL_NAME rs6000_globalize_decl_name
 #endif
+
+#undef TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P
+#define TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P \
+  rs6000_setjmp_preserves_nonvolatile_regs_p
 
 
 /* Processor table.  */
@@ -38872,6 +38876,14 @@ rs6000_starting_frame_offset (void)
     return 0;
   return RS6000_STARTING_FRAME_OFFSET;
 }
+
+/* Implement TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
+
+static bool
+rs6000_setjmp_preserves_nonvolatile_regs_p (void)
+{
+  return true;
+}
 
 
 /* Create an alias for a mangled name where we have changed the mangling (in
Index: gcc/testsuite/gcc.target/powerpc/pr87466.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr87466.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr87466.c	(working copy)
@@ -0,0 +1,19 @@
+/* { dg-do compile { target powerpc*-*-* } } */
+/* { dg-options "-O2" } */
+
+#include <stdlib.h>
+#include <setjmp.h>
+
+extern void foo (jmp_buf);
+
+long
+c (long var)
+{
+  jmp_buf env;
+  if (setjmp(env) != 0)
+    abort();
+  foo (env);
+  return var;
+}
+
+/* { dg-final { scan-assembler {\mmr\M} } } */

On 10/2/18 11:42 AM, Peter Bergner wrote:
> On 10/1/18 4:25 AM, Eric Botcazou wrote:
> This is currently bootstrapping right now, but ok for trunk assuming no
> regressions?
> 
> gcc/
> 	PR rtl-optimization/87466
> 	* target.def (setjmp_preserves_nonvolatile_regs_p): New target hook.
> 	* doc/tm.texi.in (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P): New hook.
> 	* doc/tm.texi: Regenerate.
> 	* targhooks.c (default_setjmp_preserves_nonvolatile_regs_p): Declare.
> 	* targhooks.h (default_setjmp_preserves_nonvolatile_regs_p): New
> 	function.
> 	* ira-lives.c (process_bb_node_lives): Use the new target hook.
> 	* lra-lives.c (process_bb_lives): Likewise.
> 	* config/rs6000/rs6000.c (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P):
> 	Define.
> 	(rs6000_setjmp_preserves_nonvolatile_regs_p): New function.
> 
> gcc/testsuite/
> 	PR rtl-optimization/87466
> 	* gcc.target/powerpc/pr87466.c: New test.

My powerpc64le-linux bootstrap and regtesting showed no regressions.

Peter

Hi Peter,

On Tue, Oct 02, 2018 at 11:42:19AM -0500, Peter Bergner wrote:
> +/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
> +
> +bool
> +default_setjmp_preserves_nonvolatile_regs_p (void)
> +{
> +  return false;
> +}

You can just use hook_bool_void_false for this (and hook_bool_void_true
for e.g. the rs6000 implementation).


Segher

On 10/2/18 1:21 PM, Segher Boessenkool wrote:
> Hi Peter,
> 
> On Tue, Oct 02, 2018 at 11:42:19AM -0500, Peter Bergner wrote:
>> +/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
>> +
>> +bool
>> +default_setjmp_preserves_nonvolatile_regs_p (void)
>> +{
>> +  return false;
>> +}
> 
> You can just use hook_bool_void_false for this (and hook_bool_void_true
> for e.g. the rs6000 implementation).

Ah, I didn't know those existed.  Ok, I'll rework it to use that, thanks.

Peter

On 10/2/18 1:21 PM, Segher Boessenkool wrote:
> On Tue, Oct 02, 2018 at 11:42:19AM -0500, Peter Bergner wrote:
>> +/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
>> +
>> +bool
>> +default_setjmp_preserves_nonvolatile_regs_p (void)
>> +{
>> +  return false;
>> +}
> 
> You can just use hook_bool_void_false for this (and hook_bool_void_true
> for e.g. the rs6000 implementation).

Yes, much nicer and smaller patch using those functions!  So here's version 3,
which is the same as version 2 but using above mentioned hook functions.

This is currently bootstrapping right now, ok now assuming no regressions?

Peter

gcc/
	PR rtl-optimization/87466
	* target.def (setjmp_preserves_nonvolatile_regs_p): New target hook.
	* doc/tm.texi.in (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P): New hook.
	* doc/tm.texi: Regenerate.
	* ira-lives.c (process_bb_node_lives): Use the new target hook.
	* lra-lives.c (process_bb_lives): Likewise.
	* config/rs6000/rs6000.c (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P):
	Define.

gcc/testsuite/
	PR rtl-optimization/87466
	* gcc.target/powerpc/pr87466.c: New test.

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 264795)
+++ gcc/target.def	(working copy)
@@ -3123,6 +3123,21 @@ In order to enforce the representation o
  int, (scalar_int_mode mode, scalar_int_mode rep_mode),
  default_mode_rep_extended)
 
+ DEFHOOK
+(setjmp_preserves_nonvolatile_regs_p,
+ "On some targets, it is assumed that the compiler will spill all pseudos\n\
+  that are live across a call to @code{setjmp}, while other targets treat\n\
+  @code{setjmp} calls as normal function calls.\n\
+  \n\
+  This hook returns false if @code{setjmp} calls do not preserve all\n\
+  non-volatile registers so that gcc that must spill all pseudos that are\n\
+  live across @code{setjmp} calls.  Define this to return true if the\n\
+  target does not need to spill all pseudos live across @code{setjmp} calls.\n\
+  The default implementation conservatively assumes all pseudos must be\n\
+  spilled across @code{setjmp} calls.",
+ bool, (void),
+ hook_bool_void_false)
+
 /* True if MODE is valid for a pointer in __attribute__((mode("MODE"))).  */
 DEFHOOK
 (valid_pointer_mode,
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 264795)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -7509,6 +7509,8 @@ You need not define this macro if it wou
 
 @hook TARGET_MODE_REP_EXTENDED
 
+@hook TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P
+
 @defmac STORE_FLAG_VALUE
 A C expression describing the value returned by a comparison operator
 with an integral mode and stored by a store-flag instruction
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 264795)
+++ gcc/doc/tm.texi	(working copy)
@@ -11008,6 +11008,19 @@ In order to enforce the representation o
 @code{mode}.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P (void)
+On some targets, it is assumed that the compiler will spill all pseudos
+  that are live across a call to @code{setjmp}, while other targets treat
+  @code{setjmp} calls as normal function calls.
+  
+  This hook returns false if @code{setjmp} calls do not preserve all
+  non-volatile registers so that gcc that must spill all pseudos that are
+  live across @code{setjmp} calls.  Define this to return true if the
+  target does not need to spill all pseudos live across @code{setjmp} calls.
+  The default implementation conservatively assumes all pseudos must be
+  spilled across @code{setjmp} calls.
+@end deftypefn
+
 @defmac STORE_FLAG_VALUE
 A C expression describing the value returned by a comparison operator
 with an integral mode and stored by a store-flag instruction
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c	(revision 264795)
+++ gcc/ira-lives.c	(working copy)
@@ -1207,8 +1207,9 @@ process_bb_node_lives (ira_loop_tree_nod
 		     call, if this function receives a nonlocal
 		     goto.  */
 		  if (cfun->has_nonlocal_label
-		      || find_reg_note (insn, REG_SETJMP,
-					NULL_RTX) != NULL_RTX)
+		      || (!targetm.setjmp_preserves_nonvolatile_regs_p ()
+			  && (find_reg_note (insn, REG_SETJMP, NULL_RTX)
+			      != NULL_RTX)))
 		    {
 		      SET_HARD_REG_SET (OBJECT_CONFLICT_HARD_REGS (obj));
 		      SET_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
Index: gcc/lra-lives.c
===================================================================
--- gcc/lra-lives.c	(revision 264795)
+++ gcc/lra-lives.c	(working copy)
@@ -895,8 +895,9 @@ process_bb_lives (basic_block bb, int &c
 	  sparseset_ior (pseudos_live_through_calls,
 			 pseudos_live_through_calls, pseudos_live);
 	  if (cfun->has_nonlocal_label
-	      || find_reg_note (curr_insn, REG_SETJMP,
-				NULL_RTX) != NULL_RTX)
+	      || (!targetm.setjmp_preserves_nonvolatile_regs_p ()
+		  && (find_reg_note (curr_insn, REG_SETJMP, NULL_RTX)
+		      != NULL_RTX)))
 	    sparseset_ior (pseudos_live_through_setjumps,
 			   pseudos_live_through_setjumps, pseudos_live);
 	}
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 264795)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1978,6 +1978,9 @@ static const struct attribute_spec rs600
 #undef TARGET_ASM_GLOBALIZE_DECL_NAME
 #define TARGET_ASM_GLOBALIZE_DECL_NAME rs6000_globalize_decl_name
 #endif
+
+#undef TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P
+#define TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P hook_bool_void_true
 
 
 /* Processor table.  */
Index: gcc/testsuite/gcc.target/powerpc/pr87466.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr87466.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr87466.c	(working copy)
@@ -0,0 +1,19 @@
+/* { dg-do compile { target powerpc*-*-* } } */
+/* { dg-options "-O2" } */
+
+#include <stdlib.h>
+#include <setjmp.h>
+
+extern void foo (jmp_buf);
+
+long
+c (long var)
+{
+  jmp_buf env;
+  if (setjmp(env) != 0)
+    abort();
+  foo (env);
+  return var;
+}
+
+/* { dg-final { scan-assembler {\mmr\M} } } */

On 10/02/2018 02:51 PM, Peter Bergner wrote:
> On 10/2/18 1:21 PM, Segher Boessenkool wrote:
>> On Tue, Oct 02, 2018 at 11:42:19AM -0500, Peter Bergner wrote:
>>> +/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
>>> +
>>> +bool
>>> +default_setjmp_preserves_nonvolatile_regs_p (void)
>>> +{
>>> +  return false;
>>> +}
>> You can just use hook_bool_void_false for this (and hook_bool_void_true
>> for e.g. the rs6000 implementation).
> Yes, much nicer and smaller patch using those functions!  So here's version 3,
> which is the same as version 2 but using above mentioned hook functions.
>
> This is currently bootstrapping right now, ok now assuming no regressions?
>
Ok for me.  Thank you for working on this issue, Peter.
> gcc/
> 	PR rtl-optimization/87466
> 	* target.def (setjmp_preserves_nonvolatile_regs_p): New target hook.
> 	* doc/tm.texi.in (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P): New hook.
> 	* doc/tm.texi: Regenerate.
> 	* ira-lives.c (process_bb_node_lives): Use the new target hook.
> 	* lra-lives.c (process_bb_lives): Likewise.
> 	* config/rs6000/rs6000.c (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P):
> 	Define.
>
> gcc/testsuite/
> 	PR rtl-optimization/87466
> 	* gcc.target/powerpc/pr87466.c: New test.

On 10/2/18 12:51 PM, Peter Bergner wrote:
> On 10/2/18 1:21 PM, Segher Boessenkool wrote:
>> On Tue, Oct 02, 2018 at 11:42:19AM -0500, Peter Bergner wrote:
>>> +/* The default implementation of TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P.  */
>>> +
>>> +bool
>>> +default_setjmp_preserves_nonvolatile_regs_p (void)
>>> +{
>>> +  return false;
>>> +}
>>
>> You can just use hook_bool_void_false for this (and hook_bool_void_true
>> for e.g. the rs6000 implementation).
> 
> Yes, much nicer and smaller patch using those functions!  So here's version 3,
> which is the same as version 2 but using above mentioned hook functions.
> 
> This is currently bootstrapping right now, ok now assuming no regressions?
> 
> Peter
> 
> gcc/
> 	PR rtl-optimization/87466
> 	* target.def (setjmp_preserves_nonvolatile_regs_p): New target hook.
> 	* doc/tm.texi.in (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P): New hook.
> 	* doc/tm.texi: Regenerate.
> 	* ira-lives.c (process_bb_node_lives): Use the new target hook.
> 	* lra-lives.c (process_bb_lives): Likewise.
> 	* config/rs6000/rs6000.c (TARGET_SETJMP_PRESERVES_NONVOLATILE_REGS_P):
> 	Define.
> 
> gcc/testsuite/
> 	PR rtl-optimization/87466
> 	* gcc.target/powerpc/pr87466.c: New test.
Given the desire for reload to die and the difficulties testing this
change on a reload target, I don't think mirroring this optimization in
reload is necessary.

OK for the trunk.

jeff

On 10/3/18 10:08 PM, Jeff Law wrote:
> Given the desire for reload to die and the difficulties testing this
> change on a reload target, I don't think mirroring this optimization in
> reload is necessary.

Right, it's good to have incentives to move away from reload to LRA.
Maybe we could add a few random unneeded conflicts when using reload
to push targets a little more? ;-)


> OK for the trunk.

Thank you everyone for your reviews!  Patch committed.

Peter