Re: [PATCH v3, rs6000] Add vec_reve support

[Segher Boessenkool]

Hi Carl,

On Fri, Jun 23, 2017 at 02:59:05PM -0700, Carl Love wrote:
> +(define_expand "altivec_vreve2"
> +  [(set (match_operand:VEC_A 0 "register_operand" "=v")
> + (unspec:VEC_A [(match_operand:VEC_A 1 "register_operand" "v")]
> +   UNSPEC_VREVEV))]
> +  "TARGET_ALTIVEC"
> +{
> +  int i, j, size, num_elements;
> +  rtvec v = rtvec_alloc (16);
> +  rtx mask = gen_reg_rtx (V16QImode);
> +
> +  size = GET_MODE_UNIT_SIZE (mode);
> +  num_elements = GET_MODE_NUNITS (mode);
> +
> +  for (j = num_elements - 1; j >= 0; j--)

You're still running this loop backwards, is that on purpose?  If not,
please fix.

> +for (i = 0; i < size; i++)
> +  RTVEC_ELT (v, i + j * size)
> + =  gen_rtx_CONST_INT (QImode, i + (num_elements - 1 - j) * size);

Why not just GEN_INT?

> +/* { dg-do run { target { powerpc*-*-linux* } } } */
> +/* { dg-require-effective-target vsx_hw } */
> +/* { dg-options "-O2 -mvsx" } */

Does it actually use VSX?  The condition on the expander is just
TARGET_ALTIVEC.  Or won't the testcase not work without VSX for some
other reason?

The rest looks good.  Okay for trunk if you can take care of these final
few things.

Thanks!


Segher

[PATCH v3, rs6000] Add vec_reve support

[Carl Love]

GCC maintainers:

I have updated the patch per the comments from Segher.  The vec_reve
builtin does work on Power 7, the test file was fixed to run on Power 7
as well.

The updated patch was tested on  on powerpc64le-unknown-linux-gnu
(Power 8 LE), powerpc64-unknown-linux-gnu(Power 8 BE),
powerpc64-unknown-linux-gnu (Power 7).

Please let me know if you see anything else that needs fixing.  Thanks.

   Carl Love


gcc/ChangeLog:

2017-06-23  Carl Love  

* config/rs6000/rs6000-c.c: Add support for built-in functions
vector bool char vec_reve (vector bool char);
vector signed char vec_reve (vector signed char);
vector unsigned char vec_reve (vector unsigned char);
vector bool int vec_reve (vector bool int);
vector signed int vec_reve (vector signed int);
vector unsigned int vec_reve (vector unsigned int);
vector bool long long vec_reve (vector bool long long);
vector signed long long vec_reve (vector signed long long);
vector unsigned long long vec_reve (vector unsigned long long);
vector bool short vec_reve (vector bool short);
vector signed short vec_reve (vector signed short);
vector double vec_reve (vector double);
vector float vec_reve (vector float);
* config/rs6000/rs6000-builtin.def (VREVE_V2DI, VREVE_V4SI,
VREVE_V8HI, VREVE_V16QI, VREVE_V2DF, VREVE_V4SF, VREVE): New builtin.
* config/rs6000/altivec.md (UNSPEC_VREVEV): New UNSPEC.
(altivec_vreve): New pattern.
* config/rs6000/altivec.h (vec_reve): New define.
* doc/extend.texi (vec_rev): Update the built-in documentation file
for the new built-in functions.

gcc/testsuite/ChangeLog:

2017-06-23  Carl Love  

* gcc.target/powerpc/builtins-3-vec_reve-runnable.c:
Add new runnable test file for the vec_rev built-ins.
---
 gcc/config/rs6000/altivec.h|   1 +
 gcc/config/rs6000/altivec.md   |  26 ++
 gcc/config/rs6000/rs6000-builtin.def   |   9 +
 gcc/config/rs6000/rs6000-c.c   |  29 ++
 gcc/doc/extend.texi|  13 +
 .../powerpc/builtins-3-vec_reve-runnable.c | 394 +
 6 files changed, 472 insertions(+)
 create mode 100644 
gcc/testsuite/gcc.target/powerpc/builtins-3-vec_reve-runnable.c

diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index d542315..dd68ae1 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -142,6 +142,7 @@
 #define vec_madd __builtin_vec_madd
 #define vec_madds __builtin_vec_madds
 #define vec_mtvscr __builtin_vec_mtvscr
+#define vec_reve __builtin_vec_vreve
 #define vec_vmaxfp __builtin_vec_vmaxfp
 #define vec_vmaxsw __builtin_vec_vmaxsw
 #define vec_vmaxsh __builtin_vec_vmaxsh
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 25b2768..ac86e43 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -46,6 +46,7 @@
UNSPEC_VPACK_UNS_UNS_SAT
UNSPEC_VPACK_UNS_UNS_MOD
UNSPEC_VPACK_UNS_UNS_MOD_DIRECT
+   UNSPEC_VREVEV
UNSPEC_VSLV4SI
UNSPEC_VSLO
UNSPEC_VSR
@@ -3727,6 +3728,31 @@
   DONE;
 }")
 
+;; Vector reverse elements
+(define_expand "altivec_vreve2"
+  [(set (match_operand:VEC_A 0 "register_operand" "=v")
+   (unspec:VEC_A [(match_operand:VEC_A 1 "register_operand" "v")]
+ UNSPEC_VREVEV))]
+  "TARGET_ALTIVEC"
+{
+  int i, j, size, num_elements;
+  rtvec v = rtvec_alloc (16);
+  rtx mask = gen_reg_rtx (V16QImode);
+
+  size = GET_MODE_UNIT_SIZE (mode);
+  num_elements = GET_MODE_NUNITS (mode);
+
+  for (j = num_elements - 1; j >= 0; j--)
+for (i = 0; i < size; i++)
+  RTVEC_ELT (v, i + j * size)
+   =  gen_rtx_CONST_INT (QImode, i + (num_elements - 1 - j) * size);
+
+  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
+  emit_insn (gen_altivec_vperm_ (operands[0], operands[1],
+operands[1], mask));
+  DONE;
+})
+
 ;; Vector SIMD PEM v2.06c defines LVLX, LVLXL, LVRX, LVRXL,
 ;; STVLX, STVLXL, STVVRX, STVRXL are available only on Cell.
 (define_insn "altivec_lvlx"
diff --git a/gcc/config/rs6000/rs6000-builtin.def 
b/gcc/config/rs6000/rs6000-builtin.def
index 4682628..20974b4 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -1130,6 +1130,13 @@ BU_ALTIVEC_1 (VUPKLSB, "vupklsb",CONST,  
altivec_vupklsb)
 BU_ALTIVEC_1 (VUPKLPX,   "vupklpx",CONST,  altivec_vupklpx)
 BU_ALTIVEC_1 (VUPKLSH,   "vupklsh",CONST,  altivec_vupklsh)
 
+BU_ALTIVEC_1 (VREVE_V2DI,  "vreve_v2di", CONST,  altivec_vrevev2di2)
+BU_ALTIVEC_1 (VREVE_V4SI,  "vreve_v4si", CONST,  altivec_vrevev4si2)
+BU_ALTIVEC_1 (VREVE_V8HI,  "vreve_v8hi", CONST,