Re: [MIPS][Loongson2] SIMD improvement

[Richard Sandiford <rdsandiford at googlemail dot com>]

Hi,

Thanks for the patch.

Ruan Beihong <ruanbeihong@gmail.com> writes:
> This patch adds following insntructions:
> integer AND OR NOR XOR on fpu,
> integer sll srl sra on fpu,
> and their testsuite code.

Hmm.  Why do you use built-in functions here?  As far as I can tell,
the existing Loongson functions are all for real instructions:
paddb_u() maps to PADDB_U, pandn_*() map to PANDN, etc.  Here you're
adding pand_*() functions but mapping them to (cp1) AND.  Why not
instead just define:

(define_insn "and<mode>"
  [(set (match_operand:VWHBDI 0 "register_operand" "=f")
	(and:VWHBDI (match_operand:VWHBDI 1 "register_operand" "f")
		    (match_operand:VWHBDI 2 "register_operand" "f")))]
  "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
  "and\t%0,%1,%2"
  [(set_attr "type" "fmul")])

which will allow the compiler to use this instruction when you
use "&" to AND together vectors?  We already have similar patterns
for other well-known vector operations like addition.

For avoidance of doubt, the compiler should already let you use "&"
to AND vectors together.  It'll just do it inefficiently (in some cases
anyway).  Calling the pattern and<mode> will let the compiler optimise
things properly.

These vectors can be stored in GPRs too, so I think it'd be
worth adding a GPR alternative:

In mips.md:
;; The minimum word size that an instruction needs.  This attribute is
;; used in combination with the "enabled" attribute to disable some
;; alternatives on 32-bit targets.
(define_attr "min_word_size" "unset,32,64"
  (const_string "unset"))

(define_attr "enabled" ""
  (cond [(and (eq_attr "min_word_size" "64")
              (eq (symbol_ref "TARGET_64BIT") (const_int 0)))
	 (const_int 0)]
        (const_int 1)))

In loongson.md:
(define_insn "and<mode>"
  [(set (match_operand:VWHBDI 0 "register_operand" "=f,d")
	(and:VWHBDI (match_operand:VWHBDI 1 "register_operand" "f,d")
		    (match_operand:VWHBDI 2 "register_operand" "f,d")))]
  "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
  "and\t%0,%1,%2"
  [(set_attr "type" "fmul,logical")
   (set_attr "min_word_size" "*,64")])

That will give GCC more freedom.

I'm just using AND as an example here.  The same comment applies
to the other operations.

All these suggestions are untested BTW ;)

> It also adds a transparent union __vecpack
> to represent all vector types.

Could you explain the reason for this change a bit more?  I'm not
really in a position to choose between the two approaches myself,
but I got the impression that CodeSourcery had agreed the current API
with The Appropriate Parties.  It seems like something that ought to
be discussed a bit before going live.

Richard