Hi Carl,

On Mon, Oct 23, 2017 at 08:36:03AM -0700, Carl Love wrote:
> The mode attribute wd will not work
> for the define expand as the V16QI maps to "b" not "q".  So I do need to
> have VSX_XXBR.

Why is that a problem?  Why do you want to swap the order of the whole
vector instead of changing the order of the bytes in each element (which
is a no-op in this case)?

>       * config/rs6000/vsx.md (revb_<mode>): Add define_expand to generate
>       power 8 instructions for the vec_revb builtin.

(You have a stray tab here (before "to").)

> diff --git a/gcc/config/rs6000/rs6000-protos.h 
> b/gcc/config/rs6000/rs6000-protos.h
> index f9be5d3..0cfafe5 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -142,6 +142,8 @@ extern int rs6000_emit_vector_cond_expr (rtx, rtx, rtx, 
> rtx, rtx, rtx);
>  extern void rs6000_emit_minmax (rtx, enum rtx_code, rtx, rtx);
>  extern void rs6000_split_signbit (rtx, rtx);
>  extern void rs6000_expand_atomic_compare_and_swap (rtx op[]);
> +extern rtx swap_endianess_selector_for_mode (machine_mode mode);

The word is endianness (two n's).

> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index b47eeac..0258360 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -73,6 +73,14 @@
>                            (TF        "FLOAT128_VECTOR_P (TFmode)")
>                            TI])
>  
> +(define_mode_attr VSX_XXBR  [(V16QI "q")
> +                    (V8HI  "h")
> +                    (V4SI  "w")
> +                    (V4SF  "w")
> +                    (V2DF  "d")
> +                    (V2DI  "d")
> +                    (V1TI  "q")])

So I think this is wrong for V16QI.  You can use <wd> fine, but you need
to avoid generating "xxbrb" insns; instead, do a register move?  xxbrq
isn't the insn you want, as far as I see.

> +;; Swap all bytes in each element of vector
> +(define_expand "revb_<mode>"
> +  [(set (match_operand:VEC_A 0 "vsx_register_operand")
> +     (bswap:VEC_A (match_operand:VEC_A 1 "vsx_register_operand")))]
> +  "TARGET_P9_VECTOR"
> +{
> +  rtx sel;

So a special case here:

  if (<MODE>mode == V16QImode)
    {
      emit_move_insn (operands[0], operands[1]);
      DONE;
    }

or something like it.

> +;; Swap all bytes in 128-byte vector
> +(define_expand "revb_v1ti"

I don't see what is different about this one?

> +  [(set (match_operand:V1TI 0 "vsx_register_operand")
> +     (bswap:V1TI (match_operand:V1TI 1 "vsx_register_operand")))]
> +  "TARGET_P9_VECTOR"

I don't think you want this condition, btw...  it makes the first
following if() trivially true:

> +{
> +  rtx sel;
> +
> +  if (TARGET_P9_VECTOR)
> +    emit_insn (gen_p9_xxbrq_v1ti (operands[0], operands[1]));


Segher

Reply via email to