Re: reload1: detect chained reloads

[Joern RENNECKE <joern dot rennecke at st dot com>]

DJ Delorie wrote:

>  
>
> My current patch is thusly, which attemps to tell reload that it can
> at least use the same register for two RELOAD_FOR_OPERAND_ADDRESS
> reloads if they're for the same operand, and appear to be chained.
>
> Index: reload1.c
> ===================================================================
> --- reload1.c	(revision 111581)
> +++ reload1.c	(working copy)
> @@ -4715,6 +4715,32 @@ reload_reg_reaches_end_p (unsigned int r
>     }
> }
> 
> +/* Returns 1 if the reloads denoted by R1 and R2 are uniquely chained
> +   - the output of one is the input of the other, and that
> +   intermediate is not used by any other reload for this insn.  */
>  
That's not what the function does.  It checks specifically if r1 is used 
in r2.
And it will crash if r1 has no in field set, which can happen for 
tertiary reloads.

> +static int
> +reloads_unique_chain (int r1, int r2)
> +{
> +  int i;
> +
> +  /* "chained" means one reload is a component of the other reload,
> +     not the same as the other reload.  */
> +  if (! reg_mentioned_p (rld[r1].in, rld[r2].in)
> +      || rtx_equal_p (rld[r1].in, rld[r2].in)
> +      || rld[r1].opnum != rld[r2].opnum
> +      || rld[r1].optional || rld[r2].optional)
> +    return 0;
>  
The test for rld[r1].opnum != rld[r2].opnum is much quicker than the 
function calls,
and can be expected to avoid some unnecessary function calls.  
Therefore, it should
come first.

> +  for (i=0; i<n_reloads; i++)
> +    /* Look for input reloads that aren't our two */
> +    if (i != r1 && i != r2 && rld[i].in)
> +      {
> +	/* If our reload is mentioned at all, it isn't a simple chain.  */
> +	if (reg_mentioned_p (rld[r1].in, rld[i].in))
> +	  return 0;
> +      }
> +  return 1;
> +}
> +
> /* Return 1 if the reloads denoted by R1 and R2 cannot share a register.
>    Return 0 otherwise.
>
> @@ -4763,7 +4789,8 @@ reloads_conflict (int r1, int r2)
>
>     case RELOAD_FOR_OPERAND_ADDRESS:
>       return (r2_type == RELOAD_FOR_INPUT || r2_type == RELOAD_FOR_INSN
> -	      || r2_type == RELOAD_FOR_OPERAND_ADDRESS);
> +	      || (r2_type == RELOAD_FOR_OPERAND_ADDRESS
> +		  && !reloads_unique_chain (r1, r2)));
>  
This makes reloads_conflict asymmetric, i.e. reloads_conflict (r1, r2) 
!= reloads_conflict (r2, r1)
It also breaks the relationship between reloads_conflict and 
reload_reg_free_p .

We have a dual reload register allocation, first during select_reload_regs,
to make sure we satisfy all reload register requirements, and later 
during choose_reload_regs
we do the actual allocation with reload register inheritance and related 
optimizations.

roughly, the hierarchy of the functions is:

reload
select_reload_regs
 find_reload_regs
  find_reg
   reloads_conflict: does not take values into account
reload_as_needed
 choose_reload_regs
  allocate_reload_reg
   reload_reg_free_p: supposed to use same algorithm as reloads_conflict
   free_for_value_p: takes values into account to allow to inherit the 
same value more than once in an insn, and share reload registers for the 
same non-inherited value.