[PR45652] sel-sched: don't rename into regs with non-null REG_BASE_VALUE

Hi,

Register renaming in selective scheduler may affect alias information because
a part of it is tied to registers (as opposed to MEMs) via REG_BASE_VALUE.  We
cannot update it on the fly, because it would invalidate all computed
availability sets.  Therefore, we have to avoid using registers with non-null
REG_BASE_VALUE for renaming.  The following patch implements that.

It only affects renaming of hard registers (before reload, we would use a
fresh pseudo for renaming).  I think the biggest impact is that we will not
be able to use registers that can hold incoming arguments and are not assigned
somewhere in the function.  However, on ia64 the total amount of renaming in
spec 2k6 is the same with the patch, with +1/-1 differences on a couple of
files (i.e. we almost always can pick another register).

Bootstrapped on ia64-linux with sel-sched enabled for -O2, regtest is in
progress.  OK if that passes?

2010-11-18  Alexander Monakov  <amonakov@ispras.ru>

	PR rtl-optimization/45652
	* alias.c (get_reg_base_value): New.
	* rtl.h (get_reg_base_value): Add prototype.
	* sel-sched.c (init_regs_for_mode): Use it.  Don't use registers with
	non-null REG_BASE_VALUE for renaming.

testsuite:
	* gcc.dg/pr45652.c: New.

On Thu, Nov 18, 2010 at 5:25 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> Hi,
>
> Register renaming in selective scheduler may affect alias information because
> a part of it is tied to registers (as opposed to MEMs) via REG_BASE_VALUE.  We
> cannot update it on the fly, because it would invalidate all computed
> availability sets.  Therefore, we have to avoid using registers with non-null
> REG_BASE_VALUE for renaming.  The following patch implements that.
>
> It only affects renaming of hard registers (before reload, we would use a
> fresh pseudo for renaming).  I think the biggest impact is that we will not
> be able to use registers that can hold incoming arguments and are not assigned
> somewhere in the function.  However, on ia64 the total amount of renaming in
> spec 2k6 is the same with the patch, with +1/-1 differences on a couple of
> files (i.e. we almost always can pick another register).
>
> Bootstrapped on ia64-linux with sel-sched enabled for -O2, regtest is in
> progress.  OK if that passes?

This is ok if scheduler maintainers do not object within 24h.

Thanks,
Richard.

> 2010-11-18  Alexander Monakov  <amonakov@ispras.ru>
>
>        PR rtl-optimization/45652
>        * alias.c (get_reg_base_value): New.
>        * rtl.h (get_reg_base_value): Add prototype.
>        * sel-sched.c (init_regs_for_mode): Use it.  Don't use registers with
>        non-null REG_BASE_VALUE for renaming.
>
> testsuite:
>        * gcc.dg/pr45652.c: New.
>
> diff --git a/gcc/alias.c b/gcc/alias.c
> index 2e0ac06..e8c01b4 100644
> --- a/gcc/alias.c
> +++ b/gcc/alias.c
> @@ -1291,6 +1291,14 @@ record_set (rtx dest, const_rtx set, void *data ATTRIBUTE_UNUSED)
>   reg_seen[regno] = 1;
>  }
>
> +/* Return REG_BASE_VALUE for REGNO.  Selective scheduler uses this to avoid
> +   using hard registers with non-null REG_BASE_VALUE for renaming.  */
> +rtx
> +get_reg_base_value (unsigned int regno)
> +{
> +  return VEC_index (rtx, reg_base_value, regno);
> +}
> +
>  /* If a value is known for REGNO, return it.  */
>
>  rtx
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index 3e1df2c..22721a2 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -2508,6 +2508,7 @@ extern rtx find_base_term (rtx);
>  extern rtx gen_hard_reg_clobber (enum machine_mode, unsigned int);
>  extern rtx get_reg_known_value (unsigned int);
>  extern bool get_reg_known_equiv_p (unsigned int);
> +extern rtx get_reg_base_value (unsigned int);
>
>  #ifdef STACK_REGS
>  extern int stack_regs_mentioned (const_rtx insn);
> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index 70e831d..83d8c23 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -1139,6 +1139,9 @@ init_regs_for_mode (enum machine_mode mode)
>             /* Can't use regs which aren't saved by
>                the prologue.  */
>             || !TEST_HARD_REG_BIT (sel_hrd.regs_ever_used, cur_reg + i)
> +           /* Can't use regs with non-null REG_BASE_VALUE, because adjusting
> +              it affects aliasing globally and invalidates all AV sets.  */
> +           || get_reg_base_value (cur_reg + i)
>  #ifdef LEAF_REGISTERS
>             /* We can't use a non-leaf register if we're in a
>                leaf function.  */
> diff --git a/gcc/testsuite/gcc.dg/pr45652.c b/gcc/testsuite/gcc.dg/pr45652.c
> new file mode 100644
> index 0000000..8f55f0c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr45652.c
> @@ -0,0 +1,39 @@
> +/* { dg-do run { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
> +/* { dg-options "-O2 -fselective-scheduling2" } */
> +
> +struct S {
> +  double i[2];
> +};
> +
> +void __attribute__ ((noinline)) checkcd (struct S x)
> +{
> +  if (x.i[0] != 7.0 || x.i[1] != 8.0)
> +    __builtin_abort ();
> +}
> +
> +void __attribute__ ((noinline)) testvacd (int n, ...)
> +{
> +  int i;
> +  __builtin_va_list ap;
> +  __builtin_va_start (ap, n);
> +  for (i = 0; i < n; i++)
> +    {
> +      struct S t = __builtin_va_arg (ap, struct S);
> +      checkcd (t);
> +    }
> +  __builtin_va_end (ap);
> +}
> +
> +void
> +testitcd (void)
> +{
> +  struct S x = { { 7.0, 8.0 } };
> +  testvacd (2, x, x);
> +}
> +
> +int
> +main ()
> +{
> +  testitcd ();
> +  return 0;
> +}
>
>

On 18.11.2010 19:25, Alexander Monakov wrote:
> Hi,
>
> Register renaming in selective scheduler may affect alias information because
> a part of it is tied to registers (as opposed to MEMs) via REG_BASE_VALUE.  We
> cannot update it on the fly, because it would invalidate all computed
> availability sets.  Therefore, we have to avoid using registers with non-null
> REG_BASE_VALUE for renaming.  The following patch implements that.
>
> It only affects renaming of hard registers (before reload, we would use a
> fresh pseudo for renaming).  I think the biggest impact is that we will not
> be able to use registers that can hold incoming arguments and are not assigned
> somewhere in the function.  However, on ia64 the total amount of renaming in
> spec 2k6 is the same with the patch, with +1/-1 differences on a couple of
> files (i.e. we almost always can pick another register).
>
> Bootstrapped on ia64-linux with sel-sched enabled for -O2, regtest is in
> progress.  OK if that passes?
I am not a scheduler maintainer, but given Richard's approval I would 
qualify the sel-sched.c part as obvious.

Andrey