Re: Problem with ColdFire restricted addressing modes (mode5.6combination)

[James E Wilson <wilson at specifixinc dot com>]

On Thu, 2004-08-19 at 22:28, Peter Barada wrote:
> I'm thinking that recog() is not called for the instruction that reload
> has cobbled together to see if its actually legal....

recog doesn't check the constraints, it only checks the predicates.  It
makes no sense to do this in reload, as we are only looking at the
constraints.  The constraints have to be correct.  If the constraints
disagree with the predicates, then the md file is broken and needs to be
fixed.

I took a look at this.  I did what I suggested, I stepped through
find_reloads to see what was going on.

First a little bit of info about how reload works.  Reload has to know
two things, how to check whether an operand matches a constraint, and
how to fix an operand to make it match a constraint.  The first part is
easy.  This is what things like EXTRA_CONSTRAINTS do.  The second part
is harder.  Reload knows how to fix the standard things, e.g. if you
have a 'm' constraint, and you have a mem with an invalid address, then
you fix it by loading the address into a base reg, because all targets
support (mem (reg)).  If you have a 'm' constraint, and you don't have a
mem, then you force the operand into a stack slot.  If you have an 'r'
constraint, and you don't have a reg, then you fix it by loading it into
a reg.  Now consider EXTRA_CONSTRAINTS.  There is nothing reload can do
if an operand doesn't match an extra constraint, because it doesn't know
what a 'Q' or 'U' operand is.  If something doesn't match an
EXTRA_CONSTRAINT, reload just has to pick a different one, preferably
ones it knows like 'm' or 'r'.

Now, for find_reloads, stepping through it, it first generates reloads
for the operands, and does some conversions.  One of them is converting
(subreg (pseudo)) to a stack slot.  This gives us
(set (mem/s:HI (plus:SI (reg/f:SI 7 %d7) (const_int 20 [0x14])))
     (mem:HI (plus:SI (reg/f:SI 14 %a6) (const_int -1458016))))
Now we check the constraints, and discover that both match 'U'.

Unfortunately, there is a problem here, as operand 0 uses d7 which is
not a valid base register, and operand 1 uses a constant that does not
fit into 16 bits.  Since reload doesn't know anything about the 'U'
constraint, all it can do is fall back on its default rules.  If you
have an invalid memory address, you load it into a base register.  Also,
since this target supports REG+REG addresses, it does this individually
for the operands of a plus.

For operand 0 this is OK, this just gives us (mem (plus a1 20)) which is
OK.

However, for operand 1, this is not OK, as this gives us (mem (plus a6
a1)) which now no longer matches the 'U' constraint.

(Hmm, I just noticed that both operands are using a1 which can't be
right, but this problem goes away when the original problem is fixed, so
I am ignoring it.)

In order to fix this, you need to modify the 'U' constraint so it won't
accept things that won't match a 'U' constraint after they are reloaded,
namely, you have to stop accepting out of range constants.

That is enough to fix the bug.  We now match the g/r alternative
instead, and operand 1 is fixed by loading it into a register.

However, you might get better code if you give reload more info. 
EXTRA_MEMORY_CONSTRAINT can be defined for those constraints for which
(mem (reg)) is valid, which includes the 'Q' constraint in your case.

Also, it might be useful to extend the 'U' constraint so that it also
accepts (mem (reg)) so that it can also go in EXTRA_MEMORY_CONSTRAINT. 
This is less clear though.  It might be simpler to just add 'Q' to the
existing alternative, i.e. use QU/QU instead of just U/U, so that reload
knows it can fix something by reloading the entire address into a reg. 
I am not sure whether this will actually give any improvement.  You
would have to try it and see.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com