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

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

Bernd Schmidt wrote:

>  
> Bizarre encoding, converting a variable of type icode to a variable of 
> type machine_mode; littering gencodes with odd crud.  It should be 
> immediately obvious that this is poor programming style.  Anyone who 
> reads that stuff for the first time is going to have his head explode.

I suppose it might look odd to a C++ programmer.  In C, enums are not 
actually disctinct types.

>  
> I don't think assigning three variables is a maintenance burden.

You'd have to do this for dozens of ports, for very every case that is 
handled.

> > - The taking of the address of these variables will make it hader to 
> > optimize the code in Ian's -combine
> >   scenario.
>
>
> Not sure what you meant by "Ian't -combine scenario", couldn't find 
> the reference in the thread.  Are you saying you're doing this to save 
> cycles?  Premature optimization is the root of all evil...

He said that when targetm is const, the target function can be inlined 
and thus optimzied with the caller.

> > - This scheme is not as easily extendable to multiple scratch 
> > registers - then you need arrays with
> >  arbitrary bounds, and another way to convey how many scratches you 
> > made.
>
>
> I don't see how your method provides for multiple scratches either; is 
> that something you intend to add soon?

A pattern can have multiple scratch registers, so it is merely a matter 
of looking at n_operands and the constraints
of the individual operands.  Since this ability is provided 
intrinsically by the interface, there is no hurry to
make the implementation support it, since that support can be added 
later without changing the interface.

>   I'm not fixed on any particular way of providing the information 
> from the backend to reload.  If you can find a clean way that's better 
> than the code quoted above or my suggestion, feel free.  I just object 
> to general bizarreness without a good reason.  FWIW, Ian actually did 
> suggest an interface with an array of classes a bit earlier in the 
> thread.

Apart for the simplicity of writing the port and the benefits for 
optimization, the
advantage of returning a single value is that there can be no ambiguity 
what happens
when the register class and the scratch constraint in the pattern disagree.
I suppose the best solution for that problem is to make the class 
describe exclusively a
temp register.  If no separate temp register is required, reload_class 
can be returned
(alternatively, we could say NO_REGS should be returned, and make
push_secondary_reload test the icode as well as the class when deciding 
if there is any
reload at all.)

And using separate number ranges in a single returned integer, you can 
add new ranges
for new purposes without changing the interface.
The natural way to extend an interface with multiple special-purpose 
pointers is to add
more pointers, this changing the interface.  The source code interface 
stability problem
(we don't care about binary compatibility) can be overcome by using a 
struct, as in

struct secondary_reload_info
{
 enum insn_code icode;
}  secondary_reload_info;

 enum reg_class (*secondary_reload) (int in_p, rtx x, enum reg_class 
reload_class, enum machine_mode reload_mode, secondary_reload_info *);

And in push_secondary_reload:

 struct secondary_reload_info sri;

 sri.icode = CODE_FOR_nothing;

class = targetm.secondary_reload (in_p, x, reload_class, reload_mode, 
&sri);
 if (class == NO_REGS)
   return -1;
 icode = sri.icode;
 if (icode == CODE_FOR_nothing)
   /* Allocate a secondary reload for a temp reg of CLASS.  */
   ...
 else
   {
     gcc_assert (insn_data[(int) icode].n_operands >= 2)
 
       /* icode describes how to move the reloaded object from/to the 
secondary reload
           register of class CLASS - or from/to the original reload 
register, if CLASS
           and RELOAD_CLASS are the same.
           If constraints of operand[0] / [1] don't match,  arrange for 
additional copy reloads.
           If there are more than two operands, the additional operands 
describe scratch registers.
           A mismatch of CLASS with the operand[!in_p] constraint will 
give a simple one-step
           copy reload; a mismatch of the relaoded operand with the 
operand[in_p] constraint will
           cause recursion, requiring the constraint to be for a 
register class.  */
       ...
    
   }