Hi Carl,
On Mon, Oct 09, 2017 at 08:36:57AM -0700, Carl Love wrote:
> *gcc.target/powerpc/builtins-revb-runnable.c: New
> runnable test file for the vec_revb builtin.
(Missing space after the asterisk).
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index b47eeac..fccde97 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -4712,6 +4712,94 @@
> "xxbrw %x0,%x1"
> [(set_attr "type" "vecperm")])
>
> +;; Swap all bytes in each 32-bit element
Elements aren't always 32 bits; commment out of date?
> +(define_expand "revb_<mode>"
> + [(set (match_operand:VSX_L 0 "vsx_register_operand" "=wa")
> + (bswap:VSX_L (match_operand:VSX_L 1 "vsx_register_operand" "wa")))]
"bswap" should align with the "match_operand", if you can. Constraints
in expands are not useful, just drop those :-)
> + "TARGET_P9_VECTOR || TARGET_VSX"
TARGET_VSX is always set if TARGET_P9_VECTOR is, so drop the latter?
> +{
> + int vals[16];
> + rtx mask = gen_reg_rtx (V16QImode);
> + int i;
> + rtvec v;
> + rtx rvals[16];
> +
> + if (TARGET_P9_VECTOR)
> + {
> + if (<MODE>mode == V1TImode)
> + emit_insn (gen_p9_xxbrq_v1ti (operands[0], operands[1]));
> + else if (<MODE>mode == V2DImode)
> + emit_insn (gen_p9_xxbrd_v2di (operands[0], operands[1]));
> + else if (<MODE>mode == V2DFmode)
> + emit_insn (gen_p9_xxbrd_v2df (operands[0], operands[1]));
> + else if (<MODE>mode == V4SImode)
> + emit_insn (gen_p9_xxbrw_v4si (operands[0], operands[1]));
> + else if (<MODE>mode == V4SFmode)
> + emit_insn (gen_p9_xxbrw_v4sf (operands[0], operands[1]));
> + else if (<MODE>mode == V8HImode)
> + emit_insn (gen_p9_xxbrh_v8hi (operands[0], operands[1]));
> + else if (<MODE>mode == V16QImode)
> + emit_insn (gen_p9_xxbrq_v16qi (operands[0], operands[1]));
> + }
if (TARGET_P9_VECTOR)
emit_insn (gen_p9_xxbr<wd>_<mode> (operands[0], operands[1]);
(Mode attributes are great! And indent by two spaces).
> + else
> + {
> + if (<MODE>mode == V1TImode)
> + {
> + vals[0] = 15; vals[1] = 14; vals[2] = 13; vals[3] = 12;
> + vals[4] = 11; vals[5] = 10; vals[6] = 9; vals[7] = 8;
> + vals[8] = 7; vals[9] = 6; vals[10] = 5; vals[11] = 4;
> + vals[12] = 3; vals[13] = 2; vals[14] = 1; vals[15] = 0;
> + }
> + else if ((<MODE>mode == V2DImode) || (<MODE>mode == V2DFmode))
> + {
> + vals[0] = 7; vals[1] = 6; vals[2] = 5; vals[3] = 4;
> + vals[4] = 3; vals[5] = 2; vals[6] = 1; vals[7] = 0;
> + vals[8] = 15; vals[9] = 14; vals[10] = 13; vals[11] = 12;
> + vals[12] = 11; vals[13] = 10; vals[14] = 9; vals[15] = 8;
> + }
> + else if ((<MODE>mode == V4SImode) || (<MODE>mode == V4SFmode))
> + {
> + vals[0] = 3; vals[1] = 2; vals[2] = 1; vals[3] = 0;
> + vals[4] = 7; vals[5] = 6; vals[6] = 5; vals[7] = 4;
> + vals[8] = 11; vals[9] = 10; vals[10] = 9; vals[11] = 8;
> + vals[12] = 15; vals[13] = 14; vals[14] = 13; vals[15] = 12;
> + }
> + else if (<MODE>mode == V8HImode)
> + {
> + vals[0] = 1; vals[1] = 0; vals[2] = 3; vals[3] = 2;
> + vals[4] = 5; vals[5] = 4; vals[6] = 7; vals[7] = 6;
> + vals[8] = 9; vals[9] = 8; vals[10] = 11; vals[11] = 10;
> + vals[12] = 13; vals[13] = 12; vals[14] = 15; vals[15] = 14;
> + }
> + else if (<MODE>mode == V16QImode)
> + {
> + vals[0] = 15; vals[1] = 14; vals[2] = 13; vals[3] = 12;
> + vals[4] = 11; vals[5] = 10; vals[6] = 9; vals[7] = 8;
> + vals[8] = 7; vals[9] = 6; vals[10] = 5; vals[11] = 4;
> + vals[12] = 3; vals[13] = 2; vals[14] = 1; vals[15] = 0;
> + }
> + else
> + {
> + vals[0] = 0; vals[1] = 0; vals[2] = 0; vals[3] = 0;
> + vals[4] = 0; vals[5] = 0; vals[6] = 0; vals[7] = 0;
> + vals[8] = 0; vals[9] = 0; vals[10] = 0; vals[11] = 0;
> + vals[12] = 0; vals[13] = 0; vals[14] = 0; vals[15] = 0;
> + }
What is this last one for? It seems bogus.
> + for (i = 0; i < 16; i++)
> + rvals[i] = GEN_INT (vals[i]);
> +
> + v = gen_rtvec_v (16, rvals);
> +
> + emit_insn (gen_vec_initv16qiqi (mask, gen_rtx_PARALLEL (V8HImode, v)));
> + emit_insn (gen_altivec_vperm_<mode> (operands[0], operands[1],
> + operands[1], mask));
> + }
Please use swap_selector_for_mode for this?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-revb-runnable.c
> @@ -0,0 +1,354 @@
> +/* { dg-do run { target { powerpc*-*-* } } } */
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> "-mcpu=power8" } } */
> +/* { dg-require-effective-target vsx_hw } */
> +/* { dg-options "-mcpu=power8 -O3" } */
This should only run on a power8, so p8vector_hw (because of the -mcpu=
there can be p8 insns generated, so we cannot run the testcase binary
on an older cpu).
Looks good otherwise.
Segher
