Re: [PATCH]: GCC Scheduler support for R10000 on MIPS

[Richard Sandiford <rdsandiford at googlemail dot com>]

Kumba <kumba@gentoo.org> writes:
> gcc/
>      * config/mips/10000.md: Add R10000 scheduler
>      * config/mips/mips.c: Add r10000 params & costs
>      * config/mips/mips.h: Add R10k constant
>      * config/mips/mips.md: Add r10000 params & incl 10000.md
>      * doc/invoke.texi: List r1x000 family

Looks good, thanks.  Minor comment nits:

> +;; Integer add/sub + logic ops, and mt hi/lo can be done by alu1 or alu2.
> +;; Miscellaneous arith goes here too (this is a guess).
> +(define_insn_reservation "r10k_arith" 1
> +  (and (eq_attr "cpu" "r10000")
> +       (eq_attr "type" "arith,mthilo,slt,clz,const,nop,trap,logical"))
> +  "r10k_alu1 | r10k_alu2")

Not sure if this is really a guess.  "arith" is "general ALU stuff
that we haven't needed to split out as separate types".  So if
shifts and conditional moves are really the only non-branching
instructions that require a particular ALU (ALU1), then I think we
can be pretty confident this is right.

> +;; We treat mfhilo differently, because we need to know when
> +;; it's HI and when it's LO.
> +(define_insn_reservation "r10k_mfhi" 1
> +  (and (eq_attr "cpu" "r10000")
> +       (and (eq_attr "type" "mfhilo")
> +            (not (match_operand 1 "lo_operand"))))
> +  "r10k_alu1 | r10k_alu2")
> +
> +(define_insn_reservation "r10k_mflo" 1
> +  (and (eq_attr "cpu" "r10000")
> +       (and (eq_attr "type" "mfhilo")
> +            (match_operand 1 "lo_operand")))
> +  "r10k_alu1 | r10k_alu2")

s/We treat mfhilo differently/We use separate reservations for mfhilo/

> +;; Only ALU2 does int multiplications and divisions.
> +;;
> +;; According to the Vr10000 series user manual,
> +;; integer mult and div insns can be issued one
> +;; cycle earlier if using register Lo.  We model
> +;; this by using the Lo value by default, as it
> +;; is the more common value, and use a bypass
> +;; for the Hi value when needed.
> +;;
> +;; Also of note, There are different latencies
> +;; for MULT/DMULT (Lo 5/Hi 6) and MULTU/DMULTU (Lo 6/Hi 7).
> +;; However, gcc does not have separate types
> +;; for these insns.  Thus to strike a balance,
> +;; we use the Hi latency value for imul
> +;; operations to strike a balance until the
> +;; imul type can be split.

s/to strike a balance until/until/

> +;; Divides also keep ALU2 busy, but this isn't modeled
> +;; here.

Needs clarifying.  Integer divides _are_ modelled as using r10k_alu2.

OK otherwise.  Do you have a copyright assignment on file?

Let me know if you have svn write access.  I'll apply the patch
for you if not.

Richard