[PATCH, rs6000] Use hardware support for vector character multiply

[PATCH, rs6000] Use hardware support for vector character multiply

[Bill Schmidt]

Hi,

It was pointed out to me recently that multiplying two vector chars is
performed using scalarization, even though we have hardware support for
byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
to correct this.

The expansion is pretty simple.  We do a multiply-even and multiply-odd
to create halfword results, and then use a permute to extract the
low-order bytes of each result.  This particular form of a permute uses
a different set of input/output vector modes than have been used before,
so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
source operands are vector halfword types, while the target operand is a
vector char type.)

I've added two test variants, one to test the code generation, and one
executable test to check correctness.  One other test failed with this
change.  This turned out to be because PowerPC was excluded from the
check_effective_target_vect_char_mult target support test.  I resolved
this by adding check_effective_target_powerpc_altivec to that test.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?

Thanks,
Bill


[gcc]

2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>

	* config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
	define_insn.
	(mulv16qi3): New define_expand.

[gcc/testsuite]

2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>

	* gcc.target/powerpc/vec-mult-char-1.c: New test.
	* gcc.target/powerpc/vec-mult-char-2.c: New test.


Index: gcc/config/rs6000/altivec.md
===================================================================
--- gcc/config/rs6000/altivec.md	(revision 227416)
+++ gcc/config/rs6000/altivec.md	(working copy)
@@ -1957,6 +1957,16 @@
   "vperm %0,%1,%2,%3"
   [(set_attr "type" "vecperm")])
 
+(define_insn "altivec_vperm_v8hiv16qi"
+  [(set (match_operand:V16QI 0 "register_operand" "=v")
+	(unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
+   	               (match_operand:V8HI 2 "register_operand" "v")
+		       (match_operand:V16QI 3 "register_operand" "v")]
+		   UNSPEC_VPERM))]
+  "TARGET_ALTIVEC"
+  "vperm %0,%1,%2,%3"
+  [(set_attr "type" "vecperm")])
+
 (define_expand "altivec_vperm_<mode>_uns"
   [(set (match_operand:VM 0 "register_operand" "=v")
 	(unspec:VM [(match_operand:VM 1 "register_operand" "v")
@@ -3161,6 +3171,34 @@
   "<VI_unit>"
   "")
 
+(define_expand "mulv16qi3"
+  [(set (match_operand:V16QI 0 "register_operand" "=v")
+        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
+                    (match_operand:V16QI 2 "register_operand" "v")))]
+  "TARGET_ALTIVEC"
+  "
+{
+  rtx even = gen_reg_rtx (V8HImode);
+  rtx odd = gen_reg_rtx (V8HImode);
+  rtx mask = gen_reg_rtx (V16QImode);
+  rtvec v = rtvec_alloc (16);
+  bool be = BYTES_BIG_ENDIAN;
+  int i;
+
+  for (i = 0; i < 8; ++i) {
+    RTVEC_ELT (v, 2 * i)
+     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
+    RTVEC_ELT (v, 2 * i + 1)
+     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
+  }
+
+  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
+  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
+  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
+  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
+  DONE;
+}")
+
 (define_expand "altivec_negv4sf2"
   [(use (match_operand:V4SF 0 "register_operand" ""))
    (use (match_operand:V4SF 1 "register_operand" ""))]
Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c	(working copy)
@@ -0,0 +1,53 @@
+/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec" } */
+
+#include <altivec.h>
+
+extern void abort (void);
+
+vector unsigned char vmului(vector unsigned char v,
+			    vector unsigned char i)
+{
+	return v * i;
+}
+
+vector signed char vmulsi(vector signed char v,
+			  vector signed char i)
+{
+	return v * i;
+}
+
+int main ()
+{
+  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
+			    18, 20, 22, 24, 26, 28, 30, 32};
+  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
+			    27, 30, 33, 36, 39, 42, 45, 48};
+  vector unsigned char c = vmului (a, b);
+  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
+				   230, 88, 214, 96, 246, 152, 70, 0};
+
+  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
+			  18, -20, 22, -24, 26, -28, 30, -32};
+  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
+			  27, 30, -33, -36, 39, 42, -45, -48};
+  vector signed char f = vmulsi (d, e);
+  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
+				 -26, -88, 42, 96, -10, 104, -70, 0};
+
+  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
+			  123, -123, 122, -122, 121, -121,  120, -120};
+  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
+			    10,  10, -10,  -10,  64,   65,  -64,  -65};
+  vector signed char i = vmulsi (g, h);
+  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
+				 -50, 50, 60, -60, 64, 71, 0, 120};
+
+  if (!vec_all_eq (c, expect_c))
+    abort ();
+  if (!vec_all_eq (f, expect_f))
+    abort ();
+  if (!vec_all_eq (i, expect_i))
+    abort ();
+}
Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c	(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec" } */
+
+#include <altivec.h>
+
+vector unsigned char vmului(vector unsigned char v,
+			    vector unsigned char i)
+{
+	return v * i;
+}
+
+vector signed char vmulsi(vector signed char v,
+			  vector signed char i)
+{
+	return v * i;
+}
+
+/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
+/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
+/* { dg-final { scan-assembler-times "vperm" 2 } } */
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 227416)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
 	if { [istarget aarch64*-*-*]
 	     || [istarget ia64-*-*]
 	     || [istarget i?86-*-*] || [istarget x86_64-*-*]
-            || [check_effective_target_arm32] } {
+             || [check_effective_target_arm32]
+	     || [check_effective_target_powerpc_altivec] } {
 	   set et_vect_char_mult_saved 1
 	}
     }

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Andrew Pinski]

On Thu, Sep 3, 2015 at 11:20 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> It was pointed out to me recently that multiplying two vector chars is
> performed using scalarization, even though we have hardware support for
> byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
> to correct this.
>
> The expansion is pretty simple.  We do a multiply-even and multiply-odd
> to create halfword results, and then use a permute to extract the
> low-order bytes of each result.  This particular form of a permute uses
> a different set of input/output vector modes than have been used before,
> so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
> source operands are vector halfword types, while the target operand is a
> vector char type.)

This seems like something which should be done in vector generic
rather than the back-end.  I am not blocking this patch but just
suggesting an alternative way of doing this instead of a target
specific patch.

Thanks,
Andrew

>
> I've added two test variants, one to test the code generation, and one
> executable test to check correctness.  One other test failed with this
> change.  This turned out to be because PowerPC was excluded from the
> check_effective_target_vect_char_mult target support test.  I resolved
> this by adding check_effective_target_powerpc_altivec to that test.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>
>         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
>         define_insn.
>         (mulv16qi3): New define_expand.
>
> [gcc/testsuite]
>
> 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>
>         * gcc.target/powerpc/vec-mult-char-1.c: New test.
>         * gcc.target/powerpc/vec-mult-char-2.c: New test.
>
>
> Index: gcc/config/rs6000/altivec.md
> ===================================================================
> --- gcc/config/rs6000/altivec.md        (revision 227416)
> +++ gcc/config/rs6000/altivec.md        (working copy)
> @@ -1957,6 +1957,16 @@
>    "vperm %0,%1,%2,%3"
>    [(set_attr "type" "vecperm")])
>
> +(define_insn "altivec_vperm_v8hiv16qi"
> +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> +       (unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
> +                      (match_operand:V8HI 2 "register_operand" "v")
> +                      (match_operand:V16QI 3 "register_operand" "v")]
> +                  UNSPEC_VPERM))]
> +  "TARGET_ALTIVEC"
> +  "vperm %0,%1,%2,%3"
> +  [(set_attr "type" "vecperm")])
> +
>  (define_expand "altivec_vperm_<mode>_uns"
>    [(set (match_operand:VM 0 "register_operand" "=v")
>         (unspec:VM [(match_operand:VM 1 "register_operand" "v")
> @@ -3161,6 +3171,34 @@
>    "<VI_unit>"
>    "")
>
> +(define_expand "mulv16qi3"
> +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> +        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
> +                    (match_operand:V16QI 2 "register_operand" "v")))]
> +  "TARGET_ALTIVEC"
> +  "
> +{
> +  rtx even = gen_reg_rtx (V8HImode);
> +  rtx odd = gen_reg_rtx (V8HImode);
> +  rtx mask = gen_reg_rtx (V16QImode);
> +  rtvec v = rtvec_alloc (16);
> +  bool be = BYTES_BIG_ENDIAN;
> +  int i;
> +
> +  for (i = 0; i < 8; ++i) {
> +    RTVEC_ELT (v, 2 * i)
> +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
> +    RTVEC_ELT (v, 2 * i + 1)
> +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
> +  }
> +
> +  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
> +  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
> +  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
> +  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
> +  DONE;
> +}")
> +
>  (define_expand "altivec_negv4sf2"
>    [(use (match_operand:V4SF 0 "register_operand" ""))
>     (use (match_operand:V4SF 1 "register_operand" ""))]
> Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (working copy)
> @@ -0,0 +1,53 @@
> +/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-maltivec" } */
> +
> +#include <altivec.h>
> +
> +extern void abort (void);
> +
> +vector unsigned char vmului(vector unsigned char v,
> +                           vector unsigned char i)
> +{
> +       return v * i;
> +}
> +
> +vector signed char vmulsi(vector signed char v,
> +                         vector signed char i)
> +{
> +       return v * i;
> +}
> +
> +int main ()
> +{
> +  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
> +                           18, 20, 22, 24, 26, 28, 30, 32};
> +  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
> +                           27, 30, 33, 36, 39, 42, 45, 48};
> +  vector unsigned char c = vmului (a, b);
> +  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
> +                                  230, 88, 214, 96, 246, 152, 70, 0};
> +
> +  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
> +                         18, -20, 22, -24, 26, -28, 30, -32};
> +  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
> +                         27, 30, -33, -36, 39, 42, -45, -48};
> +  vector signed char f = vmulsi (d, e);
> +  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
> +                                -26, -88, 42, 96, -10, 104, -70, 0};
> +
> +  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
> +                         123, -123, 122, -122, 121, -121,  120, -120};
> +  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
> +                           10,  10, -10,  -10,  64,   65,  -64,  -65};
> +  vector signed char i = vmulsi (g, h);
> +  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
> +                                -50, 50, 60, -60, 64, 71, 0, 120};
> +
> +  if (!vec_all_eq (c, expect_c))
> +    abort ();
> +  if (!vec_all_eq (f, expect_f))
> +    abort ();
> +  if (!vec_all_eq (i, expect_i))
> +    abort ();
> +}
> Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-maltivec" } */
> +
> +#include <altivec.h>
> +
> +vector unsigned char vmului(vector unsigned char v,
> +                           vector unsigned char i)
> +{
> +       return v * i;
> +}
> +
> +vector signed char vmulsi(vector signed char v,
> +                         vector signed char i)
> +{
> +       return v * i;
> +}
> +
> +/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
> +/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
> +/* { dg-final { scan-assembler-times "vperm" 2 } } */
> Index: gcc/testsuite/lib/target-supports.exp
> ===================================================================
> --- gcc/testsuite/lib/target-supports.exp       (revision 227416)
> +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> @@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
>         if { [istarget aarch64*-*-*]
>              || [istarget ia64-*-*]
>              || [istarget i?86-*-*] || [istarget x86_64-*-*]
> -            || [check_effective_target_arm32] } {
> +             || [check_effective_target_arm32]
> +            || [check_effective_target_powerpc_altivec] } {
>            set et_vect_char_mult_saved 1
>         }
>      }
>
>

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Bill Schmidt]

On Thu, 2015-09-03 at 23:26 +0800, Andrew Pinski wrote:
> On Thu, Sep 3, 2015 at 11:20 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > It was pointed out to me recently that multiplying two vector chars is
> > performed using scalarization, even though we have hardware support for
> > byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
> > to correct this.
> >
> > The expansion is pretty simple.  We do a multiply-even and multiply-odd
> > to create halfword results, and then use a permute to extract the
> > low-order bytes of each result.  This particular form of a permute uses
> > a different set of input/output vector modes than have been used before,
> > so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
> > source operands are vector halfword types, while the target operand is a
> > vector char type.)
> 
> This seems like something which should be done in vector generic
> rather than the back-end.  I am not blocking this patch but just
> suggesting an alternative way of doing this instead of a target
> specific patch.

Currently vector-generic checks whether the back end implements the
smul_optab for the specific vector type; if not, it scalarizes the code.
I'm not sure what else it should do.  Targets might implement the
character multiply in several different ways (directly, using
mult-even/mult-odd, using mult-hi/mult-lo), so anything other than
leaving the multiply in place could be wrong for some targets.  Am I
misunderstanding your point?

Thanks,
Bill

> 
> Thanks,
> Andrew
> 
> >
> > I've added two test variants, one to test the code generation, and one
> > executable test to check correctness.  One other test failed with this
> > change.  This turned out to be because PowerPC was excluded from the
> > check_effective_target_vect_char_mult target support test.  I resolved
> > this by adding check_effective_target_powerpc_altivec to that test.
> >
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> > regressions.  Is this ok for trunk?
> >
> > Thanks,
> > Bill
> >
> >
> > [gcc]
> >
> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >
> >         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
> >         define_insn.
> >         (mulv16qi3): New define_expand.
> >
> > [gcc/testsuite]
> >
> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >
> >         * gcc.target/powerpc/vec-mult-char-1.c: New test.
> >         * gcc.target/powerpc/vec-mult-char-2.c: New test.
> >
> >
> > Index: gcc/config/rs6000/altivec.md
> > ===================================================================
> > --- gcc/config/rs6000/altivec.md        (revision 227416)
> > +++ gcc/config/rs6000/altivec.md        (working copy)
> > @@ -1957,6 +1957,16 @@
> >    "vperm %0,%1,%2,%3"
> >    [(set_attr "type" "vecperm")])
> >
> > +(define_insn "altivec_vperm_v8hiv16qi"
> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> > +       (unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
> > +                      (match_operand:V8HI 2 "register_operand" "v")
> > +                      (match_operand:V16QI 3 "register_operand" "v")]
> > +                  UNSPEC_VPERM))]
> > +  "TARGET_ALTIVEC"
> > +  "vperm %0,%1,%2,%3"
> > +  [(set_attr "type" "vecperm")])
> > +
> >  (define_expand "altivec_vperm_<mode>_uns"
> >    [(set (match_operand:VM 0 "register_operand" "=v")
> >         (unspec:VM [(match_operand:VM 1 "register_operand" "v")
> > @@ -3161,6 +3171,34 @@
> >    "<VI_unit>"
> >    "")
> >
> > +(define_expand "mulv16qi3"
> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> > +        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
> > +                    (match_operand:V16QI 2 "register_operand" "v")))]
> > +  "TARGET_ALTIVEC"
> > +  "
> > +{
> > +  rtx even = gen_reg_rtx (V8HImode);
> > +  rtx odd = gen_reg_rtx (V8HImode);
> > +  rtx mask = gen_reg_rtx (V16QImode);
> > +  rtvec v = rtvec_alloc (16);
> > +  bool be = BYTES_BIG_ENDIAN;
> > +  int i;
> > +
> > +  for (i = 0; i < 8; ++i) {
> > +    RTVEC_ELT (v, 2 * i)
> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
> > +    RTVEC_ELT (v, 2 * i + 1)
> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
> > +  }
> > +
> > +  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
> > +  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
> > +  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
> > +  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
> > +  DONE;
> > +}")
> > +
> >  (define_expand "altivec_negv4sf2"
> >    [(use (match_operand:V4SF 0 "register_operand" ""))
> >     (use (match_operand:V4SF 1 "register_operand" ""))]
> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
> > ===================================================================
> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (working copy)
> > @@ -0,0 +1,53 @@
> > +/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec" } */
> > +
> > +#include <altivec.h>
> > +
> > +extern void abort (void);
> > +
> > +vector unsigned char vmului(vector unsigned char v,
> > +                           vector unsigned char i)
> > +{
> > +       return v * i;
> > +}
> > +
> > +vector signed char vmulsi(vector signed char v,
> > +                         vector signed char i)
> > +{
> > +       return v * i;
> > +}
> > +
> > +int main ()
> > +{
> > +  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
> > +                           18, 20, 22, 24, 26, 28, 30, 32};
> > +  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
> > +                           27, 30, 33, 36, 39, 42, 45, 48};
> > +  vector unsigned char c = vmului (a, b);
> > +  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
> > +                                  230, 88, 214, 96, 246, 152, 70, 0};
> > +
> > +  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
> > +                         18, -20, 22, -24, 26, -28, 30, -32};
> > +  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
> > +                         27, 30, -33, -36, 39, 42, -45, -48};
> > +  vector signed char f = vmulsi (d, e);
> > +  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
> > +                                -26, -88, 42, 96, -10, 104, -70, 0};
> > +
> > +  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
> > +                         123, -123, 122, -122, 121, -121,  120, -120};
> > +  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
> > +                           10,  10, -10,  -10,  64,   65,  -64,  -65};
> > +  vector signed char i = vmulsi (g, h);
> > +  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
> > +                                -50, 50, 60, -60, 64, 71, 0, 120};
> > +
> > +  if (!vec_all_eq (c, expect_c))
> > +    abort ();
> > +  if (!vec_all_eq (f, expect_f))
> > +    abort ();
> > +  if (!vec_all_eq (i, expect_i))
> > +    abort ();
> > +}
> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
> > ===================================================================
> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (working copy)
> > @@ -0,0 +1,21 @@
> > +/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> > +/* { dg-options "-maltivec" } */
> > +
> > +#include <altivec.h>
> > +
> > +vector unsigned char vmului(vector unsigned char v,
> > +                           vector unsigned char i)
> > +{
> > +       return v * i;
> > +}
> > +
> > +vector signed char vmulsi(vector signed char v,
> > +                         vector signed char i)
> > +{
> > +       return v * i;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
> > +/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
> > +/* { dg-final { scan-assembler-times "vperm" 2 } } */
> > Index: gcc/testsuite/lib/target-supports.exp
> > ===================================================================
> > --- gcc/testsuite/lib/target-supports.exp       (revision 227416)
> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> > @@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
> >         if { [istarget aarch64*-*-*]
> >              || [istarget ia64-*-*]
> >              || [istarget i?86-*-*] || [istarget x86_64-*-*]
> > -            || [check_effective_target_arm32] } {
> > +             || [check_effective_target_arm32]
> > +            || [check_effective_target_powerpc_altivec] } {
> >            set et_vect_char_mult_saved 1
> >         }
> >      }
> >
> >
> 

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Richard Biener]

On Thu, Sep 3, 2015 at 5:32 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> On Thu, 2015-09-03 at 23:26 +0800, Andrew Pinski wrote:
>> On Thu, Sep 3, 2015 at 11:20 PM, Bill Schmidt
>> <wschmidt@linux.vnet.ibm.com> wrote:
>> > Hi,
>> >
>> > It was pointed out to me recently that multiplying two vector chars is
>> > performed using scalarization, even though we have hardware support for
>> > byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
>> > to correct this.
>> >
>> > The expansion is pretty simple.  We do a multiply-even and multiply-odd
>> > to create halfword results, and then use a permute to extract the
>> > low-order bytes of each result.  This particular form of a permute uses
>> > a different set of input/output vector modes than have been used before,
>> > so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
>> > source operands are vector halfword types, while the target operand is a
>> > vector char type.)
>>
>> This seems like something which should be done in vector generic
>> rather than the back-end.  I am not blocking this patch but just
>> suggesting an alternative way of doing this instead of a target
>> specific patch.
>
> Currently vector-generic checks whether the back end implements the
> smul_optab for the specific vector type; if not, it scalarizes the code.
> I'm not sure what else it should do.  Targets might implement the
> character multiply in several different ways (directly, using
> mult-even/mult-odd, using mult-hi/mult-lo), so anything other than
> leaving the multiply in place could be wrong for some targets.  Am I
> misunderstanding your point?

I think Andrews point is the vectorizer could try different ways to vectorize
the char multiply if the target does not support it directly, like effectively
doing what you are now doing in the target.

That's certainly possible but IMHO will be a bit awkward with the current
vectorizer structure.  So as a medium-term solution I prefer Bills.

Richard.

> Thanks,
> Bill
>
>>
>> Thanks,
>> Andrew
>>
>> >
>> > I've added two test variants, one to test the code generation, and one
>> > executable test to check correctness.  One other test failed with this
>> > change.  This turned out to be because PowerPC was excluded from the
>> > check_effective_target_vect_char_mult target support test.  I resolved
>> > this by adding check_effective_target_powerpc_altivec to that test.
>> >
>> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
>> > regressions.  Is this ok for trunk?
>> >
>> > Thanks,
>> > Bill
>> >
>> >
>> > [gcc]
>> >
>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>> >
>> >         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
>> >         define_insn.
>> >         (mulv16qi3): New define_expand.
>> >
>> > [gcc/testsuite]
>> >
>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>> >
>> >         * gcc.target/powerpc/vec-mult-char-1.c: New test.
>> >         * gcc.target/powerpc/vec-mult-char-2.c: New test.
>> >
>> >
>> > Index: gcc/config/rs6000/altivec.md
>> > ===================================================================
>> > --- gcc/config/rs6000/altivec.md        (revision 227416)
>> > +++ gcc/config/rs6000/altivec.md        (working copy)
>> > @@ -1957,6 +1957,16 @@
>> >    "vperm %0,%1,%2,%3"
>> >    [(set_attr "type" "vecperm")])
>> >
>> > +(define_insn "altivec_vperm_v8hiv16qi"
>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
>> > +       (unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
>> > +                      (match_operand:V8HI 2 "register_operand" "v")
>> > +                      (match_operand:V16QI 3 "register_operand" "v")]
>> > +                  UNSPEC_VPERM))]
>> > +  "TARGET_ALTIVEC"
>> > +  "vperm %0,%1,%2,%3"
>> > +  [(set_attr "type" "vecperm")])
>> > +
>> >  (define_expand "altivec_vperm_<mode>_uns"
>> >    [(set (match_operand:VM 0 "register_operand" "=v")
>> >         (unspec:VM [(match_operand:VM 1 "register_operand" "v")
>> > @@ -3161,6 +3171,34 @@
>> >    "<VI_unit>"
>> >    "")
>> >
>> > +(define_expand "mulv16qi3"
>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
>> > +        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
>> > +                    (match_operand:V16QI 2 "register_operand" "v")))]
>> > +  "TARGET_ALTIVEC"
>> > +  "
>> > +{
>> > +  rtx even = gen_reg_rtx (V8HImode);
>> > +  rtx odd = gen_reg_rtx (V8HImode);
>> > +  rtx mask = gen_reg_rtx (V16QImode);
>> > +  rtvec v = rtvec_alloc (16);
>> > +  bool be = BYTES_BIG_ENDIAN;
>> > +  int i;
>> > +
>> > +  for (i = 0; i < 8; ++i) {
>> > +    RTVEC_ELT (v, 2 * i)
>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
>> > +    RTVEC_ELT (v, 2 * i + 1)
>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
>> > +  }
>> > +
>> > +  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
>> > +  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
>> > +  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
>> > +  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
>> > +  DONE;
>> > +}")
>> > +
>> >  (define_expand "altivec_negv4sf2"
>> >    [(use (match_operand:V4SF 0 "register_operand" ""))
>> >     (use (match_operand:V4SF 1 "register_operand" ""))]
>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
>> > ===================================================================
>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (revision 0)
>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (working copy)
>> > @@ -0,0 +1,53 @@
>> > +/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
>> > +/* { dg-options "-maltivec" } */
>> > +
>> > +#include <altivec.h>
>> > +
>> > +extern void abort (void);
>> > +
>> > +vector unsigned char vmului(vector unsigned char v,
>> > +                           vector unsigned char i)
>> > +{
>> > +       return v * i;
>> > +}
>> > +
>> > +vector signed char vmulsi(vector signed char v,
>> > +                         vector signed char i)
>> > +{
>> > +       return v * i;
>> > +}
>> > +
>> > +int main ()
>> > +{
>> > +  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
>> > +                           18, 20, 22, 24, 26, 28, 30, 32};
>> > +  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
>> > +                           27, 30, 33, 36, 39, 42, 45, 48};
>> > +  vector unsigned char c = vmului (a, b);
>> > +  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
>> > +                                  230, 88, 214, 96, 246, 152, 70, 0};
>> > +
>> > +  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
>> > +                         18, -20, 22, -24, 26, -28, 30, -32};
>> > +  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
>> > +                         27, 30, -33, -36, 39, 42, -45, -48};
>> > +  vector signed char f = vmulsi (d, e);
>> > +  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
>> > +                                -26, -88, 42, 96, -10, 104, -70, 0};
>> > +
>> > +  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
>> > +                         123, -123, 122, -122, 121, -121,  120, -120};
>> > +  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
>> > +                           10,  10, -10,  -10,  64,   65,  -64,  -65};
>> > +  vector signed char i = vmulsi (g, h);
>> > +  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
>> > +                                -50, 50, 60, -60, 64, 71, 0, 120};
>> > +
>> > +  if (!vec_all_eq (c, expect_c))
>> > +    abort ();
>> > +  if (!vec_all_eq (f, expect_f))
>> > +    abort ();
>> > +  if (!vec_all_eq (i, expect_i))
>> > +    abort ();
>> > +}
>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
>> > ===================================================================
>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (revision 0)
>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (working copy)
>> > @@ -0,0 +1,21 @@
>> > +/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
>> > +/* { dg-options "-maltivec" } */
>> > +
>> > +#include <altivec.h>
>> > +
>> > +vector unsigned char vmului(vector unsigned char v,
>> > +                           vector unsigned char i)
>> > +{
>> > +       return v * i;
>> > +}
>> > +
>> > +vector signed char vmulsi(vector signed char v,
>> > +                         vector signed char i)
>> > +{
>> > +       return v * i;
>> > +}
>> > +
>> > +/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
>> > +/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
>> > +/* { dg-final { scan-assembler-times "vperm" 2 } } */
>> > Index: gcc/testsuite/lib/target-supports.exp
>> > ===================================================================
>> > --- gcc/testsuite/lib/target-supports.exp       (revision 227416)
>> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
>> > @@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
>> >         if { [istarget aarch64*-*-*]
>> >              || [istarget ia64-*-*]
>> >              || [istarget i?86-*-*] || [istarget x86_64-*-*]
>> > -            || [check_effective_target_arm32] } {
>> > +             || [check_effective_target_arm32]
>> > +            || [check_effective_target_powerpc_altivec] } {
>> >            set et_vect_char_mult_saved 1
>> >         }
>> >      }
>> >
>> >
>>
>
>

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Andrew Pinski]

On Tue, Sep 15, 2015 at 6:58 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Sep 3, 2015 at 5:32 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>> On Thu, 2015-09-03 at 23:26 +0800, Andrew Pinski wrote:
>>> On Thu, Sep 3, 2015 at 11:20 PM, Bill Schmidt
>>> <wschmidt@linux.vnet.ibm.com> wrote:
>>> > Hi,
>>> >
>>> > It was pointed out to me recently that multiplying two vector chars is
>>> > performed using scalarization, even though we have hardware support for
>>> > byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
>>> > to correct this.
>>> >
>>> > The expansion is pretty simple.  We do a multiply-even and multiply-odd
>>> > to create halfword results, and then use a permute to extract the
>>> > low-order bytes of each result.  This particular form of a permute uses
>>> > a different set of input/output vector modes than have been used before,
>>> > so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
>>> > source operands are vector halfword types, while the target operand is a
>>> > vector char type.)
>>>
>>> This seems like something which should be done in vector generic
>>> rather than the back-end.  I am not blocking this patch but just
>>> suggesting an alternative way of doing this instead of a target
>>> specific patch.
>>
>> Currently vector-generic checks whether the back end implements the
>> smul_optab for the specific vector type; if not, it scalarizes the code.
>> I'm not sure what else it should do.  Targets might implement the
>> character multiply in several different ways (directly, using
>> mult-even/mult-odd, using mult-hi/mult-lo), so anything other than
>> leaving the multiply in place could be wrong for some targets.  Am I
>> misunderstanding your point?
>
> I think Andrews point is the vectorizer could try different ways to vectorize
> the char multiply if the target does not support it directly, like effectively
> doing what you are now doing in the target.
>
> That's certainly possible but IMHO will be a bit awkward with the current
> vectorizer structure.  So as a medium-term solution I prefer Bills.


Yes that is correct.  That is why I said I am not blocking this patch
for a generic fix but I just want to make sure the long-term solution
is the generic solution.

Thanks,
Andrew

>
> Richard.
>
>> Thanks,
>> Bill
>>
>>>
>>> Thanks,
>>> Andrew
>>>
>>> >
>>> > I've added two test variants, one to test the code generation, and one
>>> > executable test to check correctness.  One other test failed with this
>>> > change.  This turned out to be because PowerPC was excluded from the
>>> > check_effective_target_vect_char_mult target support test.  I resolved
>>> > this by adding check_effective_target_powerpc_altivec to that test.
>>> >
>>> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
>>> > regressions.  Is this ok for trunk?
>>> >
>>> > Thanks,
>>> > Bill
>>> >
>>> >
>>> > [gcc]
>>> >
>>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>>> >
>>> >         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
>>> >         define_insn.
>>> >         (mulv16qi3): New define_expand.
>>> >
>>> > [gcc/testsuite]
>>> >
>>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>>> >
>>> >         * gcc.target/powerpc/vec-mult-char-1.c: New test.
>>> >         * gcc.target/powerpc/vec-mult-char-2.c: New test.
>>> >
>>> >
>>> > Index: gcc/config/rs6000/altivec.md
>>> > ===================================================================
>>> > --- gcc/config/rs6000/altivec.md        (revision 227416)
>>> > +++ gcc/config/rs6000/altivec.md        (working copy)
>>> > @@ -1957,6 +1957,16 @@
>>> >    "vperm %0,%1,%2,%3"
>>> >    [(set_attr "type" "vecperm")])
>>> >
>>> > +(define_insn "altivec_vperm_v8hiv16qi"
>>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
>>> > +       (unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
>>> > +                      (match_operand:V8HI 2 "register_operand" "v")
>>> > +                      (match_operand:V16QI 3 "register_operand" "v")]
>>> > +                  UNSPEC_VPERM))]
>>> > +  "TARGET_ALTIVEC"
>>> > +  "vperm %0,%1,%2,%3"
>>> > +  [(set_attr "type" "vecperm")])
>>> > +
>>> >  (define_expand "altivec_vperm_<mode>_uns"
>>> >    [(set (match_operand:VM 0 "register_operand" "=v")
>>> >         (unspec:VM [(match_operand:VM 1 "register_operand" "v")
>>> > @@ -3161,6 +3171,34 @@
>>> >    "<VI_unit>"
>>> >    "")
>>> >
>>> > +(define_expand "mulv16qi3"
>>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
>>> > +        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
>>> > +                    (match_operand:V16QI 2 "register_operand" "v")))]
>>> > +  "TARGET_ALTIVEC"
>>> > +  "
>>> > +{
>>> > +  rtx even = gen_reg_rtx (V8HImode);
>>> > +  rtx odd = gen_reg_rtx (V8HImode);
>>> > +  rtx mask = gen_reg_rtx (V16QImode);
>>> > +  rtvec v = rtvec_alloc (16);
>>> > +  bool be = BYTES_BIG_ENDIAN;
>>> > +  int i;
>>> > +
>>> > +  for (i = 0; i < 8; ++i) {
>>> > +    RTVEC_ELT (v, 2 * i)
>>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
>>> > +    RTVEC_ELT (v, 2 * i + 1)
>>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
>>> > +  }
>>> > +
>>> > +  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
>>> > +  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
>>> > +  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
>>> > +  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
>>> > +  DONE;
>>> > +}")
>>> > +
>>> >  (define_expand "altivec_negv4sf2"
>>> >    [(use (match_operand:V4SF 0 "register_operand" ""))
>>> >     (use (match_operand:V4SF 1 "register_operand" ""))]
>>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
>>> > ===================================================================
>>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (revision 0)
>>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (working copy)
>>> > @@ -0,0 +1,53 @@
>>> > +/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
>>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
>>> > +/* { dg-options "-maltivec" } */
>>> > +
>>> > +#include <altivec.h>
>>> > +
>>> > +extern void abort (void);
>>> > +
>>> > +vector unsigned char vmului(vector unsigned char v,
>>> > +                           vector unsigned char i)
>>> > +{
>>> > +       return v * i;
>>> > +}
>>> > +
>>> > +vector signed char vmulsi(vector signed char v,
>>> > +                         vector signed char i)
>>> > +{
>>> > +       return v * i;
>>> > +}
>>> > +
>>> > +int main ()
>>> > +{
>>> > +  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
>>> > +                           18, 20, 22, 24, 26, 28, 30, 32};
>>> > +  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
>>> > +                           27, 30, 33, 36, 39, 42, 45, 48};
>>> > +  vector unsigned char c = vmului (a, b);
>>> > +  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
>>> > +                                  230, 88, 214, 96, 246, 152, 70, 0};
>>> > +
>>> > +  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
>>> > +                         18, -20, 22, -24, 26, -28, 30, -32};
>>> > +  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
>>> > +                         27, 30, -33, -36, 39, 42, -45, -48};
>>> > +  vector signed char f = vmulsi (d, e);
>>> > +  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
>>> > +                                -26, -88, 42, 96, -10, 104, -70, 0};
>>> > +
>>> > +  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
>>> > +                         123, -123, 122, -122, 121, -121,  120, -120};
>>> > +  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
>>> > +                           10,  10, -10,  -10,  64,   65,  -64,  -65};
>>> > +  vector signed char i = vmulsi (g, h);
>>> > +  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
>>> > +                                -50, 50, 60, -60, 64, 71, 0, 120};
>>> > +
>>> > +  if (!vec_all_eq (c, expect_c))
>>> > +    abort ();
>>> > +  if (!vec_all_eq (f, expect_f))
>>> > +    abort ();
>>> > +  if (!vec_all_eq (i, expect_i))
>>> > +    abort ();
>>> > +}
>>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
>>> > ===================================================================
>>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (revision 0)
>>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (working copy)
>>> > @@ -0,0 +1,21 @@
>>> > +/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
>>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
>>> > +/* { dg-options "-maltivec" } */
>>> > +
>>> > +#include <altivec.h>
>>> > +
>>> > +vector unsigned char vmului(vector unsigned char v,
>>> > +                           vector unsigned char i)
>>> > +{
>>> > +       return v * i;
>>> > +}
>>> > +
>>> > +vector signed char vmulsi(vector signed char v,
>>> > +                         vector signed char i)
>>> > +{
>>> > +       return v * i;
>>> > +}
>>> > +
>>> > +/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
>>> > +/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
>>> > +/* { dg-final { scan-assembler-times "vperm" 2 } } */
>>> > Index: gcc/testsuite/lib/target-supports.exp
>>> > ===================================================================
>>> > --- gcc/testsuite/lib/target-supports.exp       (revision 227416)
>>> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
>>> > @@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
>>> >         if { [istarget aarch64*-*-*]
>>> >              || [istarget ia64-*-*]
>>> >              || [istarget i?86-*-*] || [istarget x86_64-*-*]
>>> > -            || [check_effective_target_arm32] } {
>>> > +             || [check_effective_target_arm32]
>>> > +            || [check_effective_target_powerpc_altivec] } {
>>> >            set et_vect_char_mult_saved 1
>>> >         }
>>> >      }
>>> >
>>> >
>>>
>>
>>

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Bill Schmidt]

On Tue, 2015-09-15 at 13:29 -0700, Andrew Pinski wrote:
> On Tue, Sep 15, 2015 at 6:58 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
> > On Thu, Sep 3, 2015 at 5:32 PM, Bill Schmidt
> > <wschmidt@linux.vnet.ibm.com> wrote:
> >> On Thu, 2015-09-03 at 23:26 +0800, Andrew Pinski wrote:
> >>> On Thu, Sep 3, 2015 at 11:20 PM, Bill Schmidt
> >>> <wschmidt@linux.vnet.ibm.com> wrote:
> >>> > Hi,
> >>> >
> >>> > It was pointed out to me recently that multiplying two vector chars is
> >>> > performed using scalarization, even though we have hardware support for
> >>> > byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
> >>> > to correct this.
> >>> >
> >>> > The expansion is pretty simple.  We do a multiply-even and multiply-odd
> >>> > to create halfword results, and then use a permute to extract the
> >>> > low-order bytes of each result.  This particular form of a permute uses
> >>> > a different set of input/output vector modes than have been used before,
> >>> > so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
> >>> > source operands are vector halfword types, while the target operand is a
> >>> > vector char type.)
> >>>
> >>> This seems like something which should be done in vector generic
> >>> rather than the back-end.  I am not blocking this patch but just
> >>> suggesting an alternative way of doing this instead of a target
> >>> specific patch.
> >>
> >> Currently vector-generic checks whether the back end implements the
> >> smul_optab for the specific vector type; if not, it scalarizes the code.
> >> I'm not sure what else it should do.  Targets might implement the
> >> character multiply in several different ways (directly, using
> >> mult-even/mult-odd, using mult-hi/mult-lo), so anything other than
> >> leaving the multiply in place could be wrong for some targets.  Am I
> >> misunderstanding your point?
> >
> > I think Andrews point is the vectorizer could try different ways to vectorize
> > the char multiply if the target does not support it directly, like effectively
> > doing what you are now doing in the target.
> >
> > That's certainly possible but IMHO will be a bit awkward with the current
> > vectorizer structure.  So as a medium-term solution I prefer Bills.
> 
> 
> Yes that is correct.  That is why I said I am not blocking this patch
> for a generic fix but I just want to make sure the long-term solution
> is the generic solution.

Ok, I see.  So the vectorizer could try various known patterns for the
multiply and see if the target supports them instead, thus exposing
multiply-even/odd or what-have-you to more optimization.  That's
reasonable.  Thanks for the explanations!

Bill

> 
> Thanks,
> Andrew
> 
> >
> > Richard.
> >
> >> Thanks,
> >> Bill
> >>
> >>>
> >>> Thanks,
> >>> Andrew
> >>>
> >>> >
> >>> > I've added two test variants, one to test the code generation, and one
> >>> > executable test to check correctness.  One other test failed with this
> >>> > change.  This turned out to be because PowerPC was excluded from the
> >>> > check_effective_target_vect_char_mult target support test.  I resolved
> >>> > this by adding check_effective_target_powerpc_altivec to that test.
> >>> >
> >>> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> >>> > regressions.  Is this ok for trunk?
> >>> >
> >>> > Thanks,
> >>> > Bill
> >>> >
> >>> >
> >>> > [gcc]
> >>> >
> >>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >>> >
> >>> >         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
> >>> >         define_insn.
> >>> >         (mulv16qi3): New define_expand.
> >>> >
> >>> > [gcc/testsuite]
> >>> >
> >>> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >>> >
> >>> >         * gcc.target/powerpc/vec-mult-char-1.c: New test.
> >>> >         * gcc.target/powerpc/vec-mult-char-2.c: New test.
> >>> >
> >>> >
> >>> > Index: gcc/config/rs6000/altivec.md
> >>> > ===================================================================
> >>> > --- gcc/config/rs6000/altivec.md        (revision 227416)
> >>> > +++ gcc/config/rs6000/altivec.md        (working copy)
> >>> > @@ -1957,6 +1957,16 @@
> >>> >    "vperm %0,%1,%2,%3"
> >>> >    [(set_attr "type" "vecperm")])
> >>> >
> >>> > +(define_insn "altivec_vperm_v8hiv16qi"
> >>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> >>> > +       (unspec:V16QI [(match_operand:V8HI 1 "register_operand" "v")
> >>> > +                      (match_operand:V8HI 2 "register_operand" "v")
> >>> > +                      (match_operand:V16QI 3 "register_operand" "v")]
> >>> > +                  UNSPEC_VPERM))]
> >>> > +  "TARGET_ALTIVEC"
> >>> > +  "vperm %0,%1,%2,%3"
> >>> > +  [(set_attr "type" "vecperm")])
> >>> > +
> >>> >  (define_expand "altivec_vperm_<mode>_uns"
> >>> >    [(set (match_operand:VM 0 "register_operand" "=v")
> >>> >         (unspec:VM [(match_operand:VM 1 "register_operand" "v")
> >>> > @@ -3161,6 +3171,34 @@
> >>> >    "<VI_unit>"
> >>> >    "")
> >>> >
> >>> > +(define_expand "mulv16qi3"
> >>> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> >>> > +        (mult:V16QI (match_operand:V16QI 1 "register_operand" "v")
> >>> > +                    (match_operand:V16QI 2 "register_operand" "v")))]
> >>> > +  "TARGET_ALTIVEC"
> >>> > +  "
> >>> > +{
> >>> > +  rtx even = gen_reg_rtx (V8HImode);
> >>> > +  rtx odd = gen_reg_rtx (V8HImode);
> >>> > +  rtx mask = gen_reg_rtx (V16QImode);
> >>> > +  rtvec v = rtvec_alloc (16);
> >>> > +  bool be = BYTES_BIG_ENDIAN;
> >>> > +  int i;
> >>> > +
> >>> > +  for (i = 0; i < 8; ++i) {
> >>> > +    RTVEC_ELT (v, 2 * i)
> >>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 1 : 31 - 2 * i);
> >>> > +    RTVEC_ELT (v, 2 * i + 1)
> >>> > +     = gen_rtx_CONST_INT (QImode, be ? 2 * i + 17 : 15 - 2 * i);
> >>> > +  }
> >>> > +
> >>> > +  emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v)));
> >>> > +  emit_insn (gen_altivec_vmulesb (even, operands[1], operands[2]));
> >>> > +  emit_insn (gen_altivec_vmulosb (odd, operands[1], operands[2]));
> >>> > +  emit_insn (gen_altivec_vperm_v8hiv16qi (operands[0], even, odd, mask));
> >>> > +  DONE;
> >>> > +}")
> >>> > +
> >>> >  (define_expand "altivec_negv4sf2"
> >>> >    [(use (match_operand:V4SF 0 "register_operand" ""))
> >>> >     (use (match_operand:V4SF 1 "register_operand" ""))]
> >>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c
> >>> > ===================================================================
> >>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (revision 0)
> >>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-1.c  (working copy)
> >>> > @@ -0,0 +1,53 @@
> >>> > +/* { dg-do run { target { powerpc*-*-* && vmx_hw } } } */
> >>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> >>> > +/* { dg-options "-maltivec" } */
> >>> > +
> >>> > +#include <altivec.h>
> >>> > +
> >>> > +extern void abort (void);
> >>> > +
> >>> > +vector unsigned char vmului(vector unsigned char v,
> >>> > +                           vector unsigned char i)
> >>> > +{
> >>> > +       return v * i;
> >>> > +}
> >>> > +
> >>> > +vector signed char vmulsi(vector signed char v,
> >>> > +                         vector signed char i)
> >>> > +{
> >>> > +       return v * i;
> >>> > +}
> >>> > +
> >>> > +int main ()
> >>> > +{
> >>> > +  vector unsigned char a = {2, 4, 6, 8, 10, 12, 14, 16,
> >>> > +                           18, 20, 22, 24, 26, 28, 30, 32};
> >>> > +  vector unsigned char b = {3, 6, 9, 12, 15, 18, 21, 24,
> >>> > +                           27, 30, 33, 36, 39, 42, 45, 48};
> >>> > +  vector unsigned char c = vmului (a, b);
> >>> > +  vector unsigned char expect_c = {6, 24, 54, 96, 150, 216, 38, 128,
> >>> > +                                  230, 88, 214, 96, 246, 152, 70, 0};
> >>> > +
> >>> > +  vector signed char d = {2, -4, 6, -8, 10, -12, 14, -16,
> >>> > +                         18, -20, 22, -24, 26, -28, 30, -32};
> >>> > +  vector signed char e = {3, 6, -9, -12, 15, 18, -21, -24,
> >>> > +                         27, 30, -33, -36, 39, 42, -45, -48};
> >>> > +  vector signed char f = vmulsi (d, e);
> >>> > +  vector signed char expect_f = {6, -24, -54, 96, -106, 40, -38, -128,
> >>> > +                                -26, -88, 42, 96, -10, 104, -70, 0};
> >>> > +
> >>> > +  vector signed char g = {127, -128, 126, -126, 125, -125,  124, -124,
> >>> > +                         123, -123, 122, -122, 121, -121,  120, -120};
> >>> > +  vector signed char h = {  2,    2,  -2,   -2, 127,  127, -128, -128,
> >>> > +                           10,  10, -10,  -10,  64,   65,  -64,  -65};
> >>> > +  vector signed char i = vmulsi (g, h);
> >>> > +  vector signed char expect_i = {-2, 0, 4, -4, 3, -3, 0, 0,
> >>> > +                                -50, 50, 60, -60, 64, 71, 0, 120};
> >>> > +
> >>> > +  if (!vec_all_eq (c, expect_c))
> >>> > +    abort ();
> >>> > +  if (!vec_all_eq (f, expect_f))
> >>> > +    abort ();
> >>> > +  if (!vec_all_eq (i, expect_i))
> >>> > +    abort ();
> >>> > +}
> >>> > Index: gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c
> >>> > ===================================================================
> >>> > --- gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (revision 0)
> >>> > +++ gcc/testsuite/gcc.target/powerpc/vec-mult-char-2.c  (working copy)
> >>> > @@ -0,0 +1,21 @@
> >>> > +/* { dg-do compile { target { powerpc*-*-* && vmx_hw } } } */
> >>> > +/* { dg-require-effective-target powerpc_altivec_ok } */
> >>> > +/* { dg-options "-maltivec" } */
> >>> > +
> >>> > +#include <altivec.h>
> >>> > +
> >>> > +vector unsigned char vmului(vector unsigned char v,
> >>> > +                           vector unsigned char i)
> >>> > +{
> >>> > +       return v * i;
> >>> > +}
> >>> > +
> >>> > +vector signed char vmulsi(vector signed char v,
> >>> > +                         vector signed char i)
> >>> > +{
> >>> > +       return v * i;
> >>> > +}
> >>> > +
> >>> > +/* { dg-final { scan-assembler-times "vmulesb" 2 } } */
> >>> > +/* { dg-final { scan-assembler-times "vmulosb" 2 } } */
> >>> > +/* { dg-final { scan-assembler-times "vperm" 2 } } */
> >>> > Index: gcc/testsuite/lib/target-supports.exp
> >>> > ===================================================================
> >>> > --- gcc/testsuite/lib/target-supports.exp       (revision 227416)
> >>> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> >>> > @@ -4577,7 +4577,8 @@ proc check_effective_target_vect_char_mult { } {
> >>> >         if { [istarget aarch64*-*-*]
> >>> >              || [istarget ia64-*-*]
> >>> >              || [istarget i?86-*-*] || [istarget x86_64-*-*]
> >>> > -            || [check_effective_target_arm32] } {
> >>> > +             || [check_effective_target_arm32]
> >>> > +            || [check_effective_target_powerpc_altivec] } {
> >>> >            set et_vect_char_mult_saved 1
> >>> >         }
> >>> >      }
> >>> >
> >>> >
> >>>
> >>
> >>
> 

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[David Edelsohn]

On Thu, Sep 3, 2015 at 11:20 AM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> It was pointed out to me recently that multiplying two vector chars is
> performed using scalarization, even though we have hardware support for
> byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
> to correct this.
>
> The expansion is pretty simple.  We do a multiply-even and multiply-odd
> to create halfword results, and then use a permute to extract the
> low-order bytes of each result.  This particular form of a permute uses
> a different set of input/output vector modes than have been used before,
> so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
> source operands are vector halfword types, while the target operand is a
> vector char type.)
>
> I've added two test variants, one to test the code generation, and one
> executable test to check correctness.  One other test failed with this
> change.  This turned out to be because PowerPC was excluded from the
> check_effective_target_vect_char_mult target support test.  I resolved
> this by adding check_effective_target_powerpc_altivec to that test.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>
>         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
>         define_insn.
>         (mulv16qi3): New define_expand.
>
> [gcc/testsuite]
>
> 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
>
>         * gcc.target/powerpc/vec-mult-char-1.c: New test.
>         * gcc.target/powerpc/vec-mult-char-2.c: New test.

This is okay.

The "bool be = BYTES_BIG_ENDIAN" and use of "be" is not a common style in GCC.

Thanks, David

Re: [PATCH, rs6000] Use hardware support for vector character multiply

[Bill Schmidt]

On Thu, 2015-09-03 at 11:36 -0400, David Edelsohn wrote:
> On Thu, Sep 3, 2015 at 11:20 AM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > It was pointed out to me recently that multiplying two vector chars is
> > performed using scalarization, even though we have hardware support for
> > byte multiplies in vectors.  This patch adds an expansion for mulv16qi3
> > to correct this.
> >
> > The expansion is pretty simple.  We do a multiply-even and multiply-odd
> > to create halfword results, and then use a permute to extract the
> > low-order bytes of each result.  This particular form of a permute uses
> > a different set of input/output vector modes than have been used before,
> > so I added the altivec_vperm_v8hiv16qi insn to represent this.  (The two
> > source operands are vector halfword types, while the target operand is a
> > vector char type.)
> >
> > I've added two test variants, one to test the code generation, and one
> > executable test to check correctness.  One other test failed with this
> > change.  This turned out to be because PowerPC was excluded from the
> > check_effective_target_vect_char_mult target support test.  I resolved
> > this by adding check_effective_target_powerpc_altivec to that test.
> >
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> > regressions.  Is this ok for trunk?
> >
> > Thanks,
> > Bill
> >
> >
> > [gcc]
> >
> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >
> >         * config/rs6000/altivec.md (altivec_vperm_v8hiv16qi): New
> >         define_insn.
> >         (mulv16qi3): New define_expand.
> >
> > [gcc/testsuite]
> >
> > 2015-09-03  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
> >
> >         * gcc.target/powerpc/vec-mult-char-1.c: New test.
> >         * gcc.target/powerpc/vec-mult-char-2.c: New test.
> 
> This is okay.
> 
> The "bool be = BYTES_BIG_ENDIAN" and use of "be" is not a common style in GCC.

OK.  I thought that helped with readability, but I will revise this to
use BYTES_BIG_ENDIAN directly in the expressions.

Thanks,
Bill

> 
> Thanks, David
>