Re: [sh PATCH] PR/27717, sh backend lies to reload about index registers

[Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>]

> I'm confused by this patch.  The PR implies that the problem has to do
> with different pointer sizes.  That is, INDEX_REG_CLASS_FOR_MODE needs
> to use the mode in which the pointers are being computed.
>   
Yes.  In fact the problematic pattern in sh.md (divsi_inv_m0) includes this:

 mem = gen_const_mem (QImode, gen_rtx_PLUS (DImode, tab_base, tab_ix));

which means indeed this is using a DImode index register to access QImode.

> > -	  scan_rtx_address (insn, locI, INDEX_REG_CLASS, action, mode);
> > +	  scan_rtx_address (insn, locI, INDEX_REG_CLASS_FOR_MODE (GET_MODE (x)),
> > +			    action, mode)
>
> But here GET_MODE (x) is not the mode in which the address is being
> computed.  It is the mode in which the memory is being accessed.  And
> that is not what I under Joern's comment in the PR to mean.
Is that right?  GET_MODE (x) is the mode of the PLUS (this occurrence is 
under "case PLUS").

It is not clear from the documentation what MODE_BASE_REG_CLASS is 
supposed to be.  The documentation says only:

@defmac MODE_BASE_REG_CLASS (@var{mode})
This is a variation of the @code{BASE_REG_CLASS} macro which allows
the selection of a base register in a mode dependent manner.  If
@var{mode} is VOIDmode then it should return the same value as
@code{BASE_REG_CLASS}.
@end defmac

If I have to guess, I would say that MODE_BASE_REG_CLASS is for 
different MEM modes, while Joern's INDEX_REG_CLASS_FOR_MODE is for 
different pointer (MEM operand) modes.

Whether sh really needs that is beyond my understanding.  The more I 
read the patch, the more I hope it doesn't.  For example, another way to 
achieve the same would be to emit the memory access as an UNSPEC.  I 
would hope that we perform enough tree optimizations, that it is not 
possible to optimize further in the RTL path something like *(div_table 
+ (divisor >> 58)).

Thanks for the review.  As I said, I only read the patch to ensure it 
was not changing behavior on non-SH targets.

Paolo