RFA: Add support for Renesas RX architectiure to GCC

[Nick Clifton]

Hi Richard,

   Thanks very much for the comprehensive review or the RX .md files.  I 
will be submitting a revised patch shortly, but I wanted to answer a few 
points you raised first:


> Ew.  While I can see -mieee enabling 64-bit doubles, I can't see why
> -m64bit-doubles -ffast-math isn't a valid combination which would use
> the RX FP instructions for floats.

-m64bit-doubles -ffast-math is allowed, but it will not use RX FP
instructions.  The FX FP instructions only support a 32-bit long doubles
and since Renesas did not ask us to provide a 64-bit support using
32-bit FP instructions I did not implement it.


> Why is this a cc0 target, instead of using a flags hard register?
> Is it because you don't have an add insn that doesn't clobber the flags?

Precisely.

>> (define_insn "stack_push"
>>   [(set (reg:SI SP_REG) (minus:SI (reg:SI SP_REG) (const_int 4)))
>>    (unspec_volatile [(match_operand:SI 0 "immediate_operand" "i")] UNSPEC_LOW_REG)]
>>   ""
>>   "push.l\tr%c0"
>>   [(set_attr "length" "2")]
>> )
> 
> Why are you hiding the register affected? 

Because the registers concerned are those being pushed or popped and if
I use the the actual register RTXes they can become renumbered.  For
example I tried switching the patterns to use "register_operand"
constraints and newlib fails to build because the cprop_hardreg pass
replaces:

   (parallel [(set (reg/f:SI r0)(plus:SI (reg/f:SI r0)(const_int 32)))
             (unspec_volatile [(reg:SI 6 r6)] 0)
             (unspec_volatile [(reg:SI 13 r13)] 1)])

with:

   (parallel [(set (reg/f:SI r0)(plus:SI (reg/f:SI r0)(const_int 32)))
             (unspec_volatile [(reg:SI 6 r6)] 0)
             (unspec_volatile [(reg:SI 3 r3 [13])] 1)])

which is completely bogus.
	

>> (define_insn "sminsi3"
>>   [(set (match_operand:SI          0 "register_operand" "=r,r,r,r,r,r,r")
>>         (smin:SI (match_operand:SI 1 "register_operand" "%0,0,0,0,0,0,r")
>>                  (match_operand:SI 2 "rx_source_operand" 
>> "r,Sint08,Sint16,Sint24,i,Q,r")))]
> 
> Why the last alternative?  Can't reload figure out it should use the 
> first alternative?

Not in the tests that I ran. :-(  This was a month or so ago, so maybe
reload has improved since then, but I figured that leaving it in would
not hurt.

> Here and with the other string patterns, I think you'd do well to load 
> up all of the operands into pseudos and move them to hard registers 
> immediately before the string instructions.  That might minimize the 
> lifetime of the hard registers and prevent some cases of spill failures.

I thought that I was doing that already.

> That said, I'd also try to write the string patterns with register 
> constraints for the operands, rather than hard registers.

I have had a couple of goes at this so far but not had much success.  I 
would prefer to leave these patterns as they are for now and revisit 
them later.

>> (define_insn "adc"
>>   [(set (match_operand:SI                      0 "register_operand" 
>> "+r,r,r")
>>         (unspec_volatile:SI [(match_operand:SI 1 "register_operand" 
>> "%0,0,0")
>>                              (match_operand:SI 2 "rx_source_operand" 
>> "r,i,Q")]
>>                             UNSPEC_BUILTIN_ADC))
>>    (use (cc0))]
> 
> Err... don't you have to have a cc0 setter immediately before?
> Frankly, I don't see how this builtin is useful as a builtin.
> The user has no control over what happens immediately beforehand.

Indeed I had lots of trouble with this pattern and never managed to get 
it working properly.  I'll delete it for now.


> Why are mostly of these builtins unspec_volatile instead of unspec?

Because I could not get reload to play nicely with them without the 
unspec_volatile.


Cheers
   Nick