This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Add support for Renesas RX architecture to GCC (take 2)
- From: Richard Henderson <rth at redhat dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 09 Oct 2009 14:56:24 -0700
- Subject: Re: RFA: Add support for Renesas RX architecture to GCC (take 2)
- References: <m3r5tefinf.fsf@redhat.com>
+ (define_constraint "Int08"
+ "@internal An signed or unsigned 8-bit immediate value"
+ (and (match_code "const_int")
+ (and (match_test "ival >= -256")
+ (match_test "ival < 256")
+ )
+ )
+ )
I think I mentioned this before -- IN_RANGE_P.
+ (define_memory_constraint "Q"
+ "A MEM which does not use REG+REG, REG+(REG*SCALE), --REG or REG++ addressing."
+ (and (match_code "mem")
+ (ior (and (match_test "GET_CODE (XEXP (op,0)) == PLUS")
+ (and (match_test "RX_REG_P (XEXP (XEXP (op, 0), 0))")
+ (match_test "CONST_INT_P (XEXP (XEXP (op, 0), 1))")
+ )
+ )
+ (match_test "GET_CODE (XEXP (op, 0)) == REG")
+ )
+ )
+ )
Consider using e.g.
(match_code "plus" "0")
(match_code "reg" "00")
(match_code "const_int" "01")
(match_code "reg" "0")
+ static unsigned int
+ bit_count (unsigned int x)
+ {
+ const unsigned int m1 = 0x55555555;
+ const unsigned int m2 = 0x33333333;
+ const unsigned int m4 = 0x0f0f0f0f;
+
+ x -= (x >> 1) & m1;
+ x = (x & m2) + ((x >> 2) & m2);
+ x = (x + (x >> 4)) & m4;
+ x += x >> 8;
+
+ return (x + (x >> 16)) & 0x3f;
We should probably have this function moved to somewhere generic. We
have several places within gcc that we compute a population count.
Search for popcount within the toplevel gcc sources.
+ ADD_RX_BUILTIN2 (TST, "tst", void, intSI, intSI);
One last cc0-setting builtin that isn't usable.
+ ADD_RX_BUILTIN1 (PUSHC, "pushc", void, integer);
+ ADD_RX_BUILTIN1 (POPC, "popc", void, integer);
These are a bit questionable, seeing as how they manipulate the stack
register. Consider either removing them, or having them force the
function to use a frame pointer?
+ /* These addressing modes only work for SImode. */
+ #define GO_IF_MODE_DEPENDENT_ADDRESS(ADDR, LABEL) \
+ do \
+ { \
+ if (GET_CODE (ADDR) == PRE_DEC || GET_CODE (ADDR) == POST_INC) \
+ goto LABEL; \
+ if (GET_CODE (ADDR) == PLUS \
+ && REG_P (XEXP (ADDR, 0)) \
+ && REG_P (XEXP (ADDR, 1))) \
+ goto LABEL; \
+ } \
+ while (0)
Aren't there lots more addressing modes that are mode dependant? E.g.
(mem:QI (plus:SI (reg) (const_int 1)))
(mem:SI (plus:SI (reg) (const_int 131074)))
(mem:SI (plus:SI (mult:SI (reg) (const_int 4)))
I.e. the only addresses that aren't mode dependent are plain registers
and reg+const where const is a multiple of 4 and also small enough to
fit a byte offset.
Please move the contents of the macro into a function.
+ /* Control registers: */ \
+ , { "psw", 0x0 } \
+ , { "pc", 0x1 } \
+ , { "usp", 0x2 } \
+ , { "fpsw", 0x3 } \
+ , { "bpsw", 0x8 } \
+ , { "bpc", 0x9 } \
+ , { "isp", 0xa } \
+ , { "fintv", 0xb } \
+ , { "intb", 0xc } \
These should not be in the same namespace with ADDITONAL_REGISTER_NAMES.
+ (define_insn "stack_pushm"
+ [(set:SI (reg:SI SP_REG)
+ (minus:SI (reg:SI SP_REG)
+ (match_operand:SI 0 "immediate_operand" "i")))
+ (match_parallel 1 "rx_store_multiple_vector"
+ [(set:SI (match_operand:SI 2 "memory_operand" "=m")
+ (match_operand:SI 3 "register_operand" "r"))])]
All of your uses of match_parallel are incorrect. You wind up
generating nested PARALLEL patterns. The only thing that should be
present within a define_insn that uses it is a top-level match_parallel.
This results in patterns like
(define_insn "stack_pushm"
[(match_parallel 1 "rx_pushm_parallel"
[(set (reg:SI SP_REG)
(minus:SI (reg:SI SP_REG)
(match_operand:SI 0 "const_int_operand" "n")))])]
where the stack adjust is pushed inside of the parallel
+ (define_insn "subsi3"
+ [(set (match_operand:SI 0 "register_operand" "=r,r,r,r,r")
+ (minus:SI (match_operand:SI 1 "register_operand" "0,0,0,r,0")
+ (match_operand:SI 2 "rx_source_operand" "r,Uint04,i,r,Q")))]
+ ""
+ "@
+ sub\t%2, %0
+ sub\t%2, %0
+ add\t%N2, %0
That's 'n' not 'i' since you can't negate symbol addresses. That said,
I'm not sure you should ever see a (minus reg const_int), since I'm
pretty sure we canonicalize on (plus reg const_int).
+ (define_insn "addsf3"
+ [(set (match_operand:SF 0 "register_operand" "=r,r,r")
+ (plus:SF (match_operand:SF 1 "register_operand" "%0,0,0")
+ (match_operand:SF 2 "rx_source_operand" "r,F,Q")))]
+ "! TARGET_64BIT_DOUBLES || fast_math_flags_set_p ()"
You took me literally with "-ffast-math" I see. I really expected you
to de-couple -m64bit-double from -mieee. I think -m64bit-double should
not affect SFmode *at all*.
Either you have a -mieee that disables the hw instructions that can't
handle subnormals, or you conditionalize all of these patterns on
flag_unsafe_math_optimizations only. Although really one could argue
for a new flag -f{no-,}subnormal-math that's also adjusted by
-ffast-math instead.
+ ;; Byte swap (single 32-bit value).
+ (define_insn "revl"
+ [(set (match_operand:SI 0 "register_operand" "+r")
+ (bswap:SI (match_operand:SI 1 "register_operand" "r")))]
+ ""
+ "revl\t%1, %0"
+ [(set_attr "length" "3")]
+ )
s/+/=/
+ (define_insn "rolc"
+ (define_insn "rorc"
+ (define_insn "satr"
Depends on cc0, and so isn't reliable.
I am moderately surprised that you aren't representing the accumulator
as a special hard register, kinda like the mips multiplier.
+ (define_insn "round"
+ [(set (match_operand:SI 0 "register_operand" "=r,r")
+ (unspec_volatile:SI [(match_operand:SF 1 "rx_compare_operand" "r,Q")]
+ UNSPEC_BUILTIN_ROUND))]
+ ""
+ "round\t%1, %0"
+ [(set_attr "cc" "set_zs")
+ (set_attr "timings" "22,44")
+ (set_attr "length" "3,5")]
+ )
This should be lrintsf2. And you really don't need the volatile.
+ (define_insn "sync_lock_test_and_setsi"
+ [(parallel
Nested parallel; define_expand needs an explicit parallel, but
define_insn doesn't.
+ (define_insn "xchg"
Why? You've already got the sync pattern.
r~