Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints)

[Ian Lance Taylor <ian at wasabisystems dot com>]

Peter Barada <peter@the-baradas.com> writes:

> >Here is the current insn for reference:
> >
> >(define_insn "*addsi3_5200"
> >  [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r")
> >	(plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0")
> >		 (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLs")))]
> >  "TARGET_COLDFIRE"
> >  "* return output_addsi3 (operands);")
> >
> >Alternative 0 is memory += data register.  Alternative 1 is address
> >register plus address register plus (register or signed 16 bit
> >constant or any constant which moveq can't handle).  Alternative 2 is
> >(register or signed 16 bit constant or any constant which moveq can't
> >handle) plus address register.  Alternative 3 is any register +=
> >(memory or any register or addq constant or any constant which moveq
> >can't handle or subq constant or unknown constant).
> 
> I'm still wondering why replacing 's' with 'i' in the last alternative
> makes the issue go away, and produces(in this particular case), good
> code.

The constraint 's' matches any integer whose value is not known.  A
typical example would be the address of a variable.  In your example,
the value of the second operand was (I think) the constant -32.  Since
that value is known, the constraint 's' does not match.  The other
constraints, "mrIKL", also do not match a constant -32.  So the last
alternative does not match.  The constraint 'i', on the other hand,
matches any constant integer.  So when we change 's' to 'i', the last
alternative does match, and gcc uses it.

Alternative 2 does match with -32, but that alternatives requires
adding an address register to an address register.  My guess is that
gcc chooses that alternative, and tries to find an address register to
use.  It appears that, it can't, perhaps because they have all been
used for other purposes.  That leads to the "unable to find a register
to spill in class ADDR_REGS" message.  This is a plausible scenario,
though it's hard to be certain without the full test case (which I
don't particularly want).

> As for the peephole2 to use moveq to load the constant into a data
> register, does this look right?
> 
> (define_peephole2
>   [(match_scratch:SI 2 "d")
>    (set (match_operand:SI 0 "register_or_memory_operand" "rm")
>         (plus:SI (match_dup 0)
>                  (match_operand:SI 1 "immediate_operand" "K")))]
>   "!symbolic_operand (operands[1], SImode)"
>   [(set (match_dup 2) (match_dup 1))
>    (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))]
>   "")

That's not quite what I was thinking of.  I was thinking that would
match constants which moveq can handle, which is the reverse of the
constraints 'K' (if you can't use moveq for the constant, this
optimization doesn't really help, as far as I can see).  Also, you
don't need to check symbolic_operand condition, because the constraint
'K' will only match a CONST_INT anyhow.  I would try something like
this:

(define_peephole2
  [(match_scratch:SI 2 "d")
   (set (match_operand:SI 0 "register_or_memory_operand" "rm")
        (plus:SI (match_dup 0)
                 (match_operand:SI 1 "immediate_operand" "J")))]
  "INTVAL (operand[1]) >= -0x80 && INTVAL (operand[1]) < 0x80"
  [(set (match_dup 2) (match_dup 1))
   (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))]
  "")

Ian