Hi!

On Wed, Oct 23, 2019 at 05:42:45PM +0800, Kewen.Lin wrote:
> Following the previous one 2/3, this patch is to update the
> vector conversions between fixed point and floating point
> with different element unit sizes, such as: SP <-> DI, DP <-> SI.

>       (vsx_xvcvdp[su]xws): New define_expand, old one split to...

You mean <su> here, please fix (never use wildcards like [su] in changelogs:
people grep for things in changelogs, which misses entries with wildcards).

> +/* Half VMX/VSX vector (for select)  */
> +VECTOR_MODE (FLOAT, SF, 2);   /*                 V2SF  */
> +VECTOR_MODE (INT, SI, 2);     /*                 V2SI  */

Or "for internal use", in general.  What happens if a user tries to create
something of such a mode?  I hope we don't ICE :-/

> -(define_insn "vsx_xvcvspdp"
> +(define_insn "vsx_xvcvspdp_be"
>    [(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
> -     (unspec:V2DF [(match_operand:V4SF 1 "vsx_register_operand" "wa,wa")]
> -                           UNSPEC_VSX_CVSPDP))]
> -  "VECTOR_UNIT_VSX_P (V4SFmode)"
> +     (float_extend:V2DF
> +       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
> +      (parallel [(const_int 0) (const_int 2)]))))]
> +  "VECTOR_UNIT_VSX_P (V4SFmode) && BYTES_BIG_ENDIAN"
> +  "xvcvspdp %x0,%x1"
> +  [(set_attr "type" "vecdouble")])
> +
> +(define_insn "vsx_xvcvspdp_le"
> +  [(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
> +     (float_extend:V2DF
> +       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
> +      (parallel [(const_int 1) (const_int 3)]))))]
> +  "VECTOR_UNIT_VSX_P (V4SFmode) && !BYTES_BIG_ENDIAN"
>    "xvcvspdp %x0,%x1"
>    [(set_attr "type" "vecdouble")])

It would be nice if there was a nicer way to describe this, but this is
the best I can see to do as well.

> +;; Convert vector of 64-bit floating point numbers to vector of
> +;; 32-bit signed/unsigned integers.
> +(define_insn "vsx_xvcvdp<su>xws_be"
>    [(set (match_operand:V4SI 0 "vsx_register_operand" "=v,?wa")
> -     (unspec:V4SI [(match_operand:V2DF 1 "vsx_register_operand" "wa,wa")]
> -                  UNSPEC_VSX_CVDPSXWS))]
> -  "VECTOR_UNIT_VSX_P (V2DFmode)"
> -  "xvcvdpsxws %x0,%x1"
> +     (any_fix:V4SI
> +       (vec_concat:V4DF (match_operand:V2DF 1 "vsx_register_operand" "wa,wa")
> +      (vec_select:V2DF (match_dup 1)
> +        (parallel [(const_int 1) (const_int 0)])))))]
> +  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
> +  "xvcvdp<su>xws %x0,%x1"
>    [(set_attr "type" "vecdouble")])

This doesn't work, I think: the insns actually leaves words 1 and 3
undefined, but this pattern gives them a meaning.

I don't think we can do better than unspecs for such insns.  Or change
the pattern to only describe the defined parts (this works for e.g. mulhw
that describes its result as SImode: its DImode result has the high half
undefined).

> +;; Convert vector of 32-bit signed/unsigned integers to vector of
> +;; 64-bit floating point numbers.
> +(define_insn "vsx_xvcv<su>xwdp_be"
> +  [(set (match_operand:V2DF 0 "vsx_register_operand" "=wa")
> +     (any_float:V2DF
> +       (vec_select:V2SI (match_operand:V4SI 1 "vsx_register_operand" "wa")
> +      (parallel [(const_int 0) (const_int 2)]))))]
> +  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
> +  "xvcv<su>xwdp %x0,%x1"
>    [(set_attr "type" "vecdouble")])

This one for example is fine: words 1 and 3 of the input are unused, but
that is just fine.


Segher

Reply via email to