Re: [PATCH] PR rtl-optimization/67029: gcc-5.2.0 unable to find a register to spill with O3 fsched-pressure fschedule-insns

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hongjiu.lu@intel.com> writes:
> Since ira_implicitly_set_insn_hard_regs may be called outside of
> ira-lives.c, it can't use the local variable, preferred_alternatives.
> This patch adds an alternative_mask argument to
> ira_implicitly_set_insn_hard_regs.
>
> OK for master and 5 branch if there are no regressions on Linux/x86-64?

Thanks for working on this.  The patch looks good to me FWIW.

I think this version is safer than the second one you posted.  With that
version we could end up with the same sort of bug, e.g. because a function
passes false to extract_insn without realising that one of the functions
that it calls later needs the preferred alternatives to be set.
Or maybe (as with my patch) a function starts to use preferred_alternatives
and one of its callers gets missed.

Sorry for the breakage.

Thanks,
Richard

> H.J.
> ---
> gcc/
>
> 	PR rtl-optimization/67029
> 	* ira-color.c: Include "recog.h" before including "ira-int.h".
> 	* target-globals.c: Likewise.
> 	* ira-lives.c (ira_implicitly_set_insn_hard_regs): Add an
> 	adds an alternative_mask argument and use it instead of
> 	preferred_alternatives.
> 	* ira.h (ira_implicitly_set_insn_hard_regs): Moved to ...
> 	* ira-int.h (ira_implicitly_set_insn_hard_regs): Here.
> 	* sched-deps.c: Include "ira-int.h" after including "ira.h".
> 	(sched_analyze_insn): Update call to
> 	ira_implicitly_set_insn_hard_regs.
> 	* sel-sched.c: Include "ira-int.h" after including "ira.h".
> 	(implicit_clobber_conflict_p): Update call to
> 	ira_implicitly_set_insn_hard_regs.
>
> gcc/testsuite/
>
> 	PR rtl-optimization/67029
> 	* gcc.dg/pr67029.c: New test.
> ---
>  gcc/ira-color.c                |  1 +
>  gcc/ira-int.h                  |  2 ++
>  gcc/ira-lives.c                |  4 ++--
>  gcc/ira.h                      |  1 -
>  gcc/sched-deps.c               |  4 +++-
>  gcc/sel-sched.c                |  4 +++-
>  gcc/target-globals.c           |  1 +
>  gcc/testsuite/gcc.dg/pr67029.c | 14 ++++++++++++++
>  8 files changed, 26 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr67029.c
>
> diff --git a/gcc/ira-color.c b/gcc/ira-color.c
> index 74d2c2e..c8f33ed 100644
> --- a/gcc/ira-color.c
> +++ b/gcc/ira-color.c
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgloop.h"
>  #include "ira.h"
>  #include "alloc-pool.h"
> +#include "recog.h"
>  #include "ira-int.h"
>  
>  typedef struct allocno_hard_regs *allocno_hard_regs_t;
> diff --git a/gcc/ira-int.h b/gcc/ira-int.h
> index a7c0f40..a993dfc 100644
> --- a/gcc/ira-int.h
> +++ b/gcc/ira-int.h
> @@ -1041,6 +1041,8 @@ extern void ira_debug_live_ranges (void);
>  extern void ira_create_allocno_live_ranges (void);
>  extern void ira_compress_allocno_live_ranges (void);
>  extern void ira_finish_allocno_live_ranges (void);
> +extern void ira_implicitly_set_insn_hard_regs (HARD_REG_SET *,
> +					       alternative_mask);
>  
>  /* ira-conflicts.c */
>  extern void ira_debug_conflicts (bool);
> diff --git a/gcc/ira-lives.c b/gcc/ira-lives.c
> index 1cb05c2..011d513 100644
> --- a/gcc/ira-lives.c
> +++ b/gcc/ira-lives.c
> @@ -831,7 +831,8 @@ single_reg_operand_class (int op_num)
>     might be used by insn reloads because the constraints are too
>     strict.  */
>  void
> -ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set)
> +ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set,
> +				   alternative_mask preferred)
>  {
>    int i, c, regno = 0;
>    enum reg_class cl;
> @@ -854,7 +855,6 @@ ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set)
>  	  mode = (GET_CODE (op) == SCRATCH
>  		  ? GET_MODE (op) : PSEUDO_REGNO_MODE (regno));
>  	  cl = NO_REGS;
> -	  alternative_mask preferred = preferred_alternatives;
>  	  for (; (c = *p); p += CONSTRAINT_LEN (c, p))
>  	    if (c == '#')
>  	      preferred &= ~ALTERNATIVE_BIT (0);
> diff --git a/gcc/ira.h b/gcc/ira.h
> index 504b5e6..881674b 100644
> --- a/gcc/ira.h
> +++ b/gcc/ira.h
> @@ -192,7 +192,6 @@ extern void ira_init (void);
>  extern void ira_setup_eliminable_regset (void);
>  extern rtx ira_eliminate_regs (rtx, machine_mode);
>  extern void ira_set_pseudo_classes (bool, FILE *);
> -extern void ira_implicitly_set_insn_hard_regs (HARD_REG_SET *);
>  extern void ira_expand_reg_equiv (void);
>  extern void ira_update_equiv_info_by_shuffle_insn (int, int, rtx_insn *);
>  
> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
> index 3ac66e8..0a8dcb0 100644
> --- a/gcc/sched-deps.c
> +++ b/gcc/sched-deps.c
> @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "alloc-pool.h"
>  #include "cselib.h"
>  #include "ira.h"
> +#include "ira-int.h"
>  #include "target.h"
>  
>  #ifdef INSN_SCHEDULING
> @@ -2891,7 +2892,8 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, rtx_insn *insn)
>  
>        extract_insn (insn);
>        preprocess_constraints (insn);
> -      ira_implicitly_set_insn_hard_regs (&temp);
> +      alternative_mask prefrred = get_preferred_alternatives (insn);
> +      ira_implicitly_set_insn_hard_regs (&temp, prefrred);
>        AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
>        IOR_HARD_REG_SET (implicit_reg_pending_clobbers, temp);
>      }
> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index ec2ab05..1860444 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "rtlhooks-def.h"
>  #include "emit-rtl.h"
>  #include "ira.h"
> +#include "ira-int.h"
>  #include "rtl-iter.h"
>  
>  #ifdef INSN_SCHEDULING
> @@ -2104,7 +2105,8 @@ implicit_clobber_conflict_p (insn_t through_insn, expr_t expr)
>    /* Calculate implicit clobbers.  */
>    extract_insn (insn);
>    preprocess_constraints (insn);
> -  ira_implicitly_set_insn_hard_regs (&temp);
> +  alternative_mask prefrred = get_preferred_alternatives (insn);
> +  ira_implicitly_set_insn_hard_regs (&temp, prefrred);
>    AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
>  
>    /* If any implicit clobber registers intersect with regular ones in
> diff --git a/gcc/target-globals.c b/gcc/target-globals.c
> index e174815..8bc44ca 100644
> --- a/gcc/target-globals.c
> +++ b/gcc/target-globals.c
> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgloop.h"
>  #include "ira.h"
>  #include "alloc-pool.h"
> +#include "recog.h"
>  #include "ira-int.h"
>  #include "builtins.h"
>  #include "gcse.h"
> diff --git a/gcc/testsuite/gcc.dg/pr67029.c b/gcc/testsuite/gcc.dg/pr67029.c
> new file mode 100644
> index 0000000..f0023e5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr67029.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { int128 && scheduling } } } */
> +/* { dg-options "-O2 -fschedule-insns" } */
> +/* { dg-additional-options "-fstack-protector" { target fstack_protector } } */
> +
> +extern void fn2 (char *);
> +__int128 a, b;
> +int
> +fn1 (void)
> +{
> +  char e[32];
> +  fn2 (e);
> +  b = 9 * (a >> 1);
> +  return 0;
> +}