Re: RFA: 4.2 reload infrastructure to fix PR target/21623

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

Ian Lance Taylor wrote:

>  
>
> So, how does the hook return this information?  Well, currently it
> does so using this complicated dance which you describe in the
> paragraph above, in which the class returned by the target is matched
> against the class described by the insn to decide whether we need
> another register.  Is there any good reason to retain this?
>  
Yes, there is a good reason: reload inheritance.  Some simple patterns
might even be define_insns.  And we can modularize reload chains, as
complicated cases naturally build on less complicated cases.
Moreover, this matching is pretty straightforward when you match only
patterns that were supposed to be matched.  What makes this matching
such a tightrope walk at the moment is the requirement to select one
pattern for all in/output reload cases of a given mode.

> What if:
>
> 1) the hook returns the number of intermediate registers required, I
>   (where an intermediate register has the property that it will hold
>   the value after the instructions are executed)
>  
Although cases that need multiple intermediate / scratch registers are now
quite visible because the current interface makes it very hard or usually
impossible to reload them properly in call cases, most of the cases - both
statically and dynamically - only require a single intermediate register.
Thus, being able to handle these simple cases by simply returning the 
register
class of the required intermediate register is a plus.

> 2) the insn pattern must have >= 2 + I operands (in particular, may
>   have only 2 operands, though that would be unusual)
>
> 3) operand 0 is the destination
>
> 4) operand 1 is the source
>
> 5) operands 2 <= op < 2 + I describe the intermediate registers
>
> 6) operands >= 2 + I describe the scratch registers
>
> 7) all operands >= 2 must use a single register class constraint,
>   must use '=', may use '&'
>  
More optimizations are possible when intermediate registers are handled by
reload itself.  And meaningful reload inheritance of an intermediate 
register
requires that you know how to do the reload starting with the intermediate
register.  Thus, the goal should be that the intermediate registers are only
inputs and/or outputs of reload patterns, but not scratch registers.

Two intermediate registers at the reload register end can be described 
with the
hook return value and the reload register constraint.  If more are 
needed, the
hook should return the register class for the intermediate register 
closest to
the original reload register, and  set the icode of a two-operand 
pattern. so
that its input and its output describe the two next-most intermediate 
registers.
The constraint that corresponds to the reloaded value will than be
single-reg-class constraint, and fail to match the reloaded operand (if it
matched, only two intermediates woul be needed).  When this condition
is given, push_secondary_reload should recurse to match the intermediate
reload register from the pattern with the reloaded operand.

For example let's assume:

- A_REGS can only be copied from/to B_REGS
- B_REGS can only copied from/to C_REGS with a D_REGS scratch
- C_REGS can only be copied from/to E_REGS
- E_REGS can only be copied from/to F_REGS
- A "Foo" constant can only be copied from/to F_REGS.

The hook code for this can be:

 if (CONST_OK_FOR_Foo (x))
   switch ((int) reload_class)
     {
     case F_REGS: return NO_REGS;
     case E_REGS: return F_REGS;
     case C_REGS: sri->icode = in_p ? CODE_FOR_movEC : CODE_FOR_movCE; 
return E_REGS;
     case B_REGS:
     case A_REGS: sri->icode = in_p ? CODE_FOR_movCB : CODE_FOR_movBC; 
return NO_REGS;
   }

And the patterns:
(define_expand "movEC" [(set (match_operand 0 "" "=C") (match_operand 1 
"" "E))] "" "")
(define_expand "movCE" [(set (match_operand 0 "" "=E") (match_operand 1 
"" "C))] "" "")
(define_expand "movCB" [(set (match_operand 0 "" "=B") (match_operand 1 
"" "C)) (clobber (match_operand 2 "" "=&D"))] "" "")
(define_expand "movBC" [(set (match_operand 0 "" "=C") (match_operand 1 
"" "B)) (clobber (match_operand 2 "" "=D"))] "" "")

You scheme would describe the A_REGS <-> Foo case with a single, rather
large pattern.  But that wouldn't help, since you still have to describe 
how to
moves of Foo values from/to registers of the intermediate register classes,
since these can arise as reload problems in their own right.  And your 
scheme
also doesn't cover how to inherit a Foo value that has already been 
copied in
a C_REGS reload register and is wanted again in an A_REGS register.  With
my scheme, that is immediately obvious and requires no extra coding.