diff mbox series

[1/5] rs6000, Add 128-bit sign extension support

Message ID 4ebaa81cef8cd6fe8caf8c33d6a4fc8a946c591f.camel@us.ibm.com
State New
Headers show
Series rs6000, 128-bit Binary Integer Operations | expand

Commit Message

Carl Love Aug. 11, 2020, 7:22 p.m. UTC
Segher, Will:

Patch 1, adds the sign extension instruction support and corresponding
builtins.

             Carl Love

---------------------------------------------------------------------
RS6000 Add 128-bit sign extension support

gcc/ChangeLog

2020-08-10  Carl Love  <cel@us.ibm.com>
	* config/rs6000/altivec.h (vec_signextll, vec_signexti): Add define
	for new builtins.
	* config/rs6000/rs6000-builtin.def (VSIGNEXTI, VSIGNEXTLL):  Add
	overloaded builtin definitions.
	(VSIGNEXTSB2W, VSIGNEXTSB2D, VSIGNEXTSH2D,VSIGNEXTSW2D): Add builtin
	expansions.
	* config/rs6000-call.c (P9V_BUILTIN_VEC_VSIGNEXTI,
	P9V_BUILTIN_VEC_VSIGNEXTLL): Add overloaded argument definitions.
	* config/rs6000/vsx.md: Make define_insn vsx_sign_extend_si_v2di
	visible.
	* doc/extend.texi:  Add documentation for the vec_signexti and
	vec_signextll builtins.

gcc/testsuite/ChangeLog

2020-08-10  Carl Love  <cel@us.ibm.com>
	* gcc.target/powerpc/p9-sign_extend-runnable.c:  New test case.
---
 gcc/config/rs6000/altivec.h                   |   3 +
 gcc/config/rs6000/rs6000-builtin.def          |   9 ++
 gcc/config/rs6000/rs6000-call.c               |  13 ++
 gcc/config/rs6000/vsx.md                      |   2 +-
 gcc/doc/extend.texi                           |  15 ++
 .../powerpc/p9-sign_extend-runnable.c         | 128 ++++++++++++++++++
 6 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c

Comments

Segher Boessenkool Aug. 13, 2020, 5:36 p.m. UTC | #1
Hi!

On Tue, Aug 11, 2020 at 12:22:37PM -0700, Carl Love wrote:
> +/* Sign extend builtins that work on ISA 3.0, but not defined until ISA 3.1.  */

What does this mean?  Not defined in GCC before now?  Does it need
backporting?  Not defined in older versions of the ELFv2 ABI (or vector
doc) and we do not want a backport?

> +  /* Sign extend builtins that work work on ISA 3.0, not added until ISA 3.1 */

Same (also "work work").

> +uThe following sign extension builtins are provided.

(stray "u")

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
> @@ -0,0 +1,128 @@
> +/* { dg-do run { target { powerpc*-*-linux* && { lp64 && p9vector_hw } } } } */

/* { dg-do run { target { lp64 && p9vector_hw } } } */

or such; or do you require Linux actually?

> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -save-temps" } */

Is -save-temps needed?  Not for the scan-assembler at least.

Okay for trunk with those details take care of.  Thanks!


Segher
Carl Love Aug. 13, 2020, 6:09 p.m. UTC | #2
Segher:

On Thu, 2020-08-13 at 12:36 -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Aug 11, 2020 at 12:22:37PM -0700, Carl Love wrote:
> > +/* Sign extend builtins that work on ISA 3.0, but not defined
> > until ISA 3.1.  */
> 
> What does this mean?  Not defined in GCC before now?  Does it need
> backporting?  Not defined in older versions of the ELFv2 ABI (or
> vector
> doc) and we do not want a backport?
> 
> > +  /* Sign extend builtins that work work on ISA 3.0, not added
> > until ISA 3.1 */

The builtins

vector signed int vec_signexti (vector signed char a)
vector signed long long vec_signextll (vector signed char a)
vector signed int vec_signexti (vector signed short a)
vector signed long long vec_signextll (vector signed short a)
vector signed long long vec_signextll (vector signed int a)

were defined in the function prototypes directory in box called "RFC
2608 - 128-bit Binary Integer Operations".  The document the new P10
builtins.  However, this subset of the newly defined builtins for P10
can be implemented with existing Power 9 instructions.  That was the
point of the comment.  That is probably a level of detail that is not
really needed in the GCC code comment.  Probably best to just change
the comment to read something like "ISA 3.0 sign extend builtins". 

My thought for calling it out is that they could be back ported to an
earlier GCC version since they use Power 9 instructions but it is
probably not worth the effort unless there is an explicit request for
them. 

                 Carl
Segher Boessenkool Aug. 13, 2020, 6:29 p.m. UTC | #3
On Thu, Aug 13, 2020 at 11:09:10AM -0700, Carl Love wrote:
> The builtins
> 
> vector signed int vec_signexti (vector signed char a)
> vector signed long long vec_signextll (vector signed char a)
> vector signed int vec_signexti (vector signed short a)
> vector signed long long vec_signextll (vector signed short a)
> vector signed long long vec_signextll (vector signed int a)
> 
> were defined in the function prototypes directory in box called "RFC
> 2608 - 128-bit Binary Integer Operations".  The document the new P10
> builtins.  However, this subset of the newly defined builtins for P10
> can be implemented with existing Power 9 instructions.  That was the
> point of the comment.

Ah, I see :-)

> That is probably a level of detail that is not
> really needed in the GCC code comment.  Probably best to just change
> the comment to read something like "ISA 3.0 sign extend builtins". 

Sounds good.

> My thought for calling it out is that they could be back ported to an
> earlier GCC version since they use Power 9 instructions but it is
> probably not worth the effort unless there is an explicit request for
> them. 

Yeah.  Thanks for the explanation!


Segher
will schmidt Aug. 13, 2020, 10:11 p.m. UTC | #4
On Thu, 2020-08-13 at 13:29 -0500, Segher Boessenkool wrote:
> On Thu, Aug 13, 2020 at 11:09:10AM -0700, Carl Love wrote:
> > The builtins
> > 
> > vector signed int vec_signexti (vector signed char a)
> > vector signed long long vec_signextll (vector signed char a)
> > vector signed int vec_signexti (vector signed short a)
> > vector signed long long vec_signextll (vector signed short a)
> > vector signed long long vec_signextll (vector signed int a)
> > 
> > were defined in the function prototypes directory in box called
> > "RFC
> > 2608 - 128-bit Binary Integer Operations".  The document the new
> > P10
> > builtins.  However, this subset of the newly defined builtins for
> > P10
> > can be implemented with existing Power 9 instructions.  That was
> > the
> > point of the comment.
> 
> Ah, I see :-)
> 
> > That is probably a level of detail that is not
> > really needed in the GCC code comment.  Probably best to just
> > change
> > the comment to read something like "ISA 3.0 sign extend builtins". 
> 
> Sounds good.

As long as there are no issues defining the builtins for 3.0 here.
AFAIK they are not documented in ISA 3.0.  This is a happy accident
that these ISA 3.1 builtins can be implemented with existing support.

> 
> > My thought for calling it out is that they could be back ported to
> > an
> > earlier GCC version since they use Power 9 instructions but it is
> > probably not worth the effort unless there is an explicit request
> > for
> > them. 
> 
> Yeah.  Thanks for the explanation!
> 
> 
> Segher
Segher Boessenkool Aug. 13, 2020, 10:55 p.m. UTC | #5
Hi!

On Thu, Aug 13, 2020 at 05:11:11PM -0500, will schmidt wrote:
> > > That is probably a level of detail that is not
> > > really needed in the GCC code comment.  Probably best to just
> > > change
> > > the comment to read something like "ISA 3.0 sign extend builtins". 
> > 
> > Sounds good.
> 
> As long as there are no issues defining the builtins for 3.0 here.
> AFAIK they are not documented in ISA 3.0.  This is a happy accident
> that these ISA 3.1 builtins can be implemented with existing support.

There are *no* builtins defined in the ISA!  The insns are just ISA 3.0
instructions.


Segher
will schmidt Aug. 13, 2020, 11:53 p.m. UTC | #6
On Thu, 2020-08-13 at 17:55 -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Aug 13, 2020 at 05:11:11PM -0500, will schmidt wrote:
> > > > That is probably a level of detail that is not
> > > > really needed in the GCC code comment.  Probably best to just
> > > > change
> > > > the comment to read something like "ISA 3.0 sign extend
> > > > builtins". 
> > > 
> > > Sounds good.
> > 
> > As long as there are no issues defining the builtins for 3.0 here.
> > AFAIK they are not documented in ISA 3.0.  This is a happy accident
> > that these ISA 3.1 builtins can be implemented with existing
> > support.
> 
> There are *no* builtins defined in the ISA!  The insns are just ISA
> 3.0
> instructions.
> 

Ok. 

So then maybe just "Sign extend builtins" and leave off the ISA
reference all together.   

:-)

thanks
-WIll

> 
> Segher
Segher Boessenkool Aug. 18, 2020, 9:50 p.m. UTC | #7
On Thu, Aug 13, 2020 at 06:53:56PM -0500, will schmidt wrote:
> On Thu, 2020-08-13 at 17:55 -0500, Segher Boessenkool wrote:
> > > As long as there are no issues defining the builtins for 3.0 here.
> > > AFAIK they are not documented in ISA 3.0.  This is a happy accident
> > > that these ISA 3.1 builtins can be implemented with existing
> > > support.
> > 
> > There are *no* builtins defined in the ISA!  The insns are just ISA
> > 3.0
> > instructions.
> 
> Ok. 
> 
> So then maybe just "Sign extend builtins" and leave off the ISA
> reference all together.   

Sure.  Or you can say "builtins for the instructions introduced in
Power ISA 3.1" or such.

If we ever get the builtins documentation updated quickly (and updated),
it should go on https://gcc.gnu.org/readings.html , and live will be
good.


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index bf2240f16a2..09320df14ca 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -498,6 +498,9 @@ 
 
 #define vec_xlx __builtin_vec_vextulx
 #define vec_xrx __builtin_vec_vexturx
+#define vec_signexti  __builtin_vec_vsignexti
+#define vec_signextll __builtin_vec_vsignextll
+
 #endif
 
 /* Predicates.
diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index f9f0fece549..667c2450d41 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2691,6 +2691,8 @@  BU_P9V_OVERLOAD_1 (VPRTYBD,	"vprtybd")
 BU_P9V_OVERLOAD_1 (VPRTYBQ,	"vprtybq")
 BU_P9V_OVERLOAD_1 (VPRTYBW,	"vprtybw")
 BU_P9V_OVERLOAD_1 (VPARITY_LSBB,	"vparity_lsbb")
+BU_P9V_OVERLOAD_1 (VSIGNEXTI,	"vsignexti")
+BU_P9V_OVERLOAD_1 (VSIGNEXTLL,	"vsignextll")
 
 /* 2 argument functions added in ISA 3.0 (power9).  */
 BU_P9_2 (CMPRB,	"byte_in_range",	CONST,	cmprb)
@@ -2702,6 +2704,13 @@  BU_P9_OVERLOAD_2 (CMPRB,	"byte_in_range")
 BU_P9_OVERLOAD_2 (CMPRB2,	"byte_in_either_range")
 BU_P9_OVERLOAD_2 (CMPEQB,	"byte_in_set")
 
+/* Sign extend builtins that work on ISA 3.0, but not defined until ISA 3.1.  */
+BU_P9V_AV_1 (VSIGNEXTSB2W,	"vsignextsb2w",		CONST,  vsx_sign_extend_qi_v4si)
+BU_P9V_AV_1 (VSIGNEXTSH2W,	"vsignextsh2w",		CONST,  vsx_sign_extend_hi_v4si)
+BU_P9V_AV_1 (VSIGNEXTSB2D,	"vsignextsb2d",		CONST,  vsx_sign_extend_qi_v2di)
+BU_P9V_AV_1 (VSIGNEXTSH2D,	"vsignextsh2d",		CONST,  vsx_sign_extend_hi_v2di)
+BU_P9V_AV_1 (VSIGNEXTSW2D,	"vsignextsw2d",		CONST,  vsx_sign_extend_si_v2di)
+
 /* Builtins for scalar instructions added in ISA 3.1 (power10).  */
 BU_P10_MISC_2 (CFUGED, "cfuged", CONST, cfuged)
 BU_P10_MISC_2 (CNTLZDM, "cntlzdm", CONST, cntlzdm)
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 189497efb45..87699be8a07 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -5527,6 +5527,19 @@  const struct altivec_builtin_types altivec_overloaded_builtins[] = {
     RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI,
     RS6000_BTI_INTSI, RS6000_BTI_INTSI },
 
+  /* Sign extend builtins that work work on ISA 3.0, not added until ISA 3.1 */
+  { P9V_BUILTIN_VEC_VSIGNEXTI, P9V_BUILTIN_VSIGNEXTSB2W,
+    RS6000_BTI_V4SI, RS6000_BTI_V16QI, 0, 0 },
+  { P9V_BUILTIN_VEC_VSIGNEXTI, P9V_BUILTIN_VSIGNEXTSH2W,
+    RS6000_BTI_V4SI, RS6000_BTI_V8HI, 0, 0 },
+
+  { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSB2D,
+    RS6000_BTI_V2DI, RS6000_BTI_V16QI, 0, 0 },
+  { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSH2D,
+    RS6000_BTI_V2DI, RS6000_BTI_V8HI, 0, 0 },
+  { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSW2D,
+    RS6000_BTI_V2DI, RS6000_BTI_V4SI, 0, 0 },
+
   /* Overloaded built-in functions for ISA3.1 (power10). */
   { P10_BUILTIN_VEC_CLRL, P10_BUILTIN_VCLRLB,
     RS6000_BTI_V16QI, RS6000_BTI_V16QI, RS6000_BTI_UINTSI, 0 },
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index dd750210758..1153a01b4ef 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -4787,7 +4787,7 @@ 
   "vextsh2<wd> %0,%1"
   [(set_attr "type" "vecexts")])
 
-(define_insn "*vsx_sign_extend_si_v2di"
+(define_insn "vsx_sign_extend_si_v2di"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=v")
 	(unspec:V2DI [(match_operand:V4SI 1 "vsx_register_operand" "v")]
 		     UNSPEC_VSX_SIGN_EXTEND))]
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 79833171c5a..cb501ab2d75 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -20754,6 +20754,21 @@  void vec_xst (vector unsigned char, int, vector unsigned char *);
 void vec_xst (vector unsigned char, int, unsigned char *);
 @end smallexample
 
+uThe following sign extension builtins are provided.
+
+@smallexample
+vector signed int vec_signexti (vector signed char a)
+vector signed long long vec_signextll (vector signed char a)
+vector signed int vec_signexti (vector signed short a)
+vector signed long long vec_signextll (vector signed short a)
+vector signed long long vec_signextll (vector signed int a)
+@end smallexample
+
+Each element of the result is produced by sign-extending the element of the
+input vector that would fall in the least significant portion of the result
+element. For example, a sign-extension of a vector signed char to a vector
+signed long long will sign extend the rightmost byte of each doubleword.
+
 @node PowerPC AltiVec Built-in Functions Available on ISA 3.1
 @subsubsection PowerPC AltiVec Built-in Functions Available on ISA 3.1
 
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c b/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
new file mode 100644
index 00000000000..7bf979c6fd4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
@@ -0,0 +1,128 @@ 
+/* { dg-do run { target { powerpc*-*-linux* && { lp64 && p9vector_hw } } } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power9 -save-temps" } */
+
+/* These builtins were not defined until ISA 3.1 but only require ISA 3.0
+   support.  */
+
+/* { dg-final { scan-assembler-times {\mvextsb2w\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvextsb2d\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvextsh2w\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvextsh2d\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mvextsw2d\M} 1 } } */
+
+#include <altivec.h>
+
+#define DEBUG 0
+
+#if DEBUG
+#include <stdio.h>
+#include <stdlib.h>
+#endif
+
+void abort (void);
+
+int main ()
+{
+  int i;
+
+  vector signed char vec_arg_qi, vec_result_qi;
+  vector signed short int vec_arg_hi, vec_result_hi, vec_expected_hi;
+  vector signed int vec_arg_wi, vec_result_wi, vec_expected_wi;
+  vector signed long long vec_result_di, vec_expected_di;
+
+  /* test sign extend byte to word */
+  vec_arg_qi = (vector signed char) {1, 2, 3, 4, 5, 6, 7, 8,
+				     -1, -2, -3, -4, -5, -6, -7, -8};
+  vec_expected_wi = (vector signed int) {1, 5, -1, -5};
+
+  vec_result_wi = vec_signexti (vec_arg_qi);
+
+  for (i = 0; i < 4; i++)
+    if (vec_result_wi[i] != vec_expected_wi[i]) {
+#if DEBUG
+      printf("ERROR: vec_signexti(char, int):  ");
+      printf("vec_result_wi[%d] != vec_expected_wi[%d]\n",
+	     i, i);
+      printf("vec_result_wi[%d] = %d\n", i, vec_result_wi[i]);
+      printf("vec_expected_wi[%d] = %d\n", i, vec_expected_wi[i]);
+#else
+      abort();
+#endif
+    }
+
+  /* test sign extend byte to double */
+  vec_arg_qi = (vector signed char){1, 2, 3, 4, 5, 6, 7, 8,
+				    -1, -2, -3, -4, -5, -6, -7, -8};
+  vec_expected_di = (vector signed long long int){1, -1};
+
+  vec_result_di = vec_signextll(vec_arg_qi);
+
+  for (i = 0; i < 2; i++)
+    if (vec_result_di[i] != vec_expected_di[i]) {
+#if DEBUG
+      printf("ERROR: vec_signextll(byte, long long int):  ");
+      printf("vec_result_di[%d] != vec_expected_di[%d]\n", i, i);
+      printf("vec_result_di[%d] = %lld\n", i, vec_result_di[i]);
+      printf("vec_expected_di[%d] = %lld\n", i, vec_expected_di[i]);
+#else
+      abort();
+#endif
+    }
+
+  /* test sign extend short to word */
+  vec_arg_hi = (vector signed short int){1, 2, 3, 4, -1, -2, -3, -4};
+  vec_expected_wi = (vector signed int){1, 3, -1, -3};
+
+  vec_result_wi = vec_signexti(vec_arg_hi);
+
+  for (i = 0; i < 4; i++)
+    if (vec_result_wi[i] != vec_expected_wi[i]) {
+#if DEBUG
+      printf("ERROR: vec_signexti(short, int):  ");
+      printf("vec_result_wi[%d] != vec_expected_wi[%d]\n", i, i);
+      printf("vec_result_wi[%d] = %d\n", i, vec_result_wi[i]);
+      printf("vec_expected_wi[%d] = %d\n", i, vec_expected_wi[i]);
+#else
+      abort();
+#endif
+    }
+
+  /* test sign extend short to double word */
+  vec_arg_hi = (vector signed short int ){1, 3, 5, 7,  -1, -3, -5, -7};
+  vec_expected_di = (vector signed long long int){1, -1};
+
+  vec_result_di = vec_signextll(vec_arg_hi);
+
+  for (i = 0; i < 2; i++)
+    if (vec_result_di[i] != vec_expected_di[i]) {
+#if DEBUG
+      printf("ERROR: vec_signextll(short, double):  ");
+      printf("vec_result_di[%d] != vec_expected_di[%d]\n", i, i);
+      printf("vec_result_di[%d] = %lld\n", i, vec_result_di[i]);
+      printf("vec_expected_di[%d] = %lld\n", i, vec_expected_di[i]);
+#else
+      abort();
+#endif
+    }
+
+  /* test sign extend word to double word */
+  vec_arg_wi = (vector signed int ){1, 3, -1, -3};
+  vec_expected_di = (vector signed long long int){1, -1};
+
+  vec_result_di = vec_signextll(vec_arg_wi);
+
+  for (i = 0; i < 2; i++)
+    if (vec_result_di[i] != vec_expected_di[i]) {
+#if DEBUG
+      printf("ERROR: vec_signextll(word, double):  ");
+      printf("vec_result_di[%d] != vec_expected_di[%d]\n", i, i);
+      printf("vec_result_di[%d] = %lld\n", i, vec_result_di[i]);
+      printf("vec_expected_di[%d] = %lld\n", i, vec_expected_di[i]);
+#else
+      abort();
+#endif
+    }
+
+  return 0;
+}