[MIPS][Loongson2] SIMD improvement

Ruan Beihong ruanbeihong@gmail.com
Sun Feb 1 08:19:00 GMT 2009


2009/1/31 Richard Sandiford <rdsandiford@googlemail.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
>
Thanks for your suggestion. I will run the test.
As to the __vecpack issue, I believed programmer some times need to
assign a double-word value directly to a vector type, or make
conversions between different kinds of vectors, this union will be a
better one. There is a -flax-vector-conversions option in gcc. Most
people will advice not to use implicit conversion, so it's better to
provide an easy way of explicit conversion. __vecpack helps programmer
to remember the type of vector (by access its members of different
type), and a unified interface to those builtin functions seems
better.
To be frank, these reason are not strong enough compared with The
Appropriate Parties. I did the change just in an impulse to unify the
interface.

-- 
James Ruan



More information about the Gcc-patches mailing list