diff mbox series

rs6000: Fix MMA API - Add support for compatibility built-ins

Message ID 12ebce7f-29c0-3d98-c2f2-e6d3135ff043@linux.ibm.com
State New
Headers show
Series rs6000: Fix MMA API - Add support for compatibility built-ins | expand

Commit Message

Peter Bergner Feb. 4, 2021, 8:40 p.m. UTC
The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
__builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
__builtin_vsx_disassemble_pair respectively.  It's too late to remove the
old names, so this patch adds support for creating compatibility built-ins
(ie, multiple built-in functions generate the same code) and then creates
compatibility built-ins using the new names.

This passed bootstrap and regtesting on powerpc64le-linux with no regressions.
Ok for mainline?

This will need backporting to GCC 10.  Ok there too once it's baked on
trunk for a little while?

Peter


gcc/
	* gcc/config/rs6000/rs6000-builtin.def (BU_COMPAT): Add support macro
	for defining compatibility built-ins.
	(vsx_assemble_pair): Add compatibility built-in.
	* gcc/config/rs6000/rs6000-call.c (struct builtin_compatibility): New.
	(bdesc_compat): New.
	(RS6000_BUILTIN_COMPAT): Define.
	(rs6000_init_builtins): Register compatibility built-ins.
	* gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c: Add tests for
	__builtin_vsx_assemble_pair and __builtin_vsx_disassemble_pair.
	Update expected instruction counts.

Comments

Segher Boessenkool Feb. 4, 2021, 9:16 p.m. UTC | #1
Hi!

On Thu, Feb 04, 2021 at 02:40:20PM -0600, Peter Bergner wrote:
> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
> old names, so this patch adds support for creating compatibility built-ins
> (ie, multiple built-in functions generate the same code) and then creates
> compatibility built-ins using the new names.

This needs a documentation update.  You can just delete the old names
there (i.e. rename everything there).

> This passed bootstrap and regtesting on powerpc64le-linux with no regressions.
> Ok for mainline?

Some comments:

> +#ifndef RS6000_BUILTIN_COMPAT
> +  #undef BU_COMPAT
> +  #define BU_COMPAT(ENUM, COMPAT_NAME)

Please do not do #undef unless necessary: it hides bugs (and that causes
more bugs).

>  BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
> +BU_COMPAT (MMA_BUILTIN_ASSEMBLE_PAIR, "vsx_assemble_pair")

You should do those the other way around (the mma_ one is the
compatibility one).  This matters, because if you disable the
compatibility builtins the vsx_ one should still be there, but not the
old name.  (It also makes more sense of course).

Okay for trunk with that fixed.  Also okay for gcc-10 after watching
a week or so for fallout.  Thanks!


Segher
Peter Bergner Feb. 5, 2021, 4:05 a.m. UTC | #2
On 2/4/21 3:16 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Feb 04, 2021 at 02:40:20PM -0600, Peter Bergner wrote:
>> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
>> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
>> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
>> old names, so this patch adds support for creating compatibility built-ins
>> (ie, multiple built-in functions generate the same code) and then creates
>> compatibility built-ins using the new names.
> 
> This needs a documentation update.  You can just delete the old names
> there (i.e. rename everything there).

Good catch!  I had meant to do that and totally forgot! :-(


>> +#ifndef RS6000_BUILTIN_COMPAT
>> +  #undef BU_COMPAT
>> +  #define BU_COMPAT(ENUM, COMPAT_NAME)
> 
> Please do not do #undef unless necessary: it hides bugs (and that causes
> more bugs).

I thought it was needed, since rs6000-builtin.def is #included into
rs6000-call.c and rs6000.h multiple times.  The first 11 times, we must
make sure it expands to nothing/empty string and on the 12th time, it
expands to create the bdesc_compat array.  However, it looks like if you
redefine it to be the same value each time, then you do not need to
#undef it, so it looks like just the last usage when we create the
bdesc_compat array will we need to #undef it.  I'll give that a try
and see how it works out.



>>  BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
>> +BU_COMPAT (MMA_BUILTIN_ASSEMBLE_PAIR, "vsx_assemble_pair")
> 
> You should do those the other way around (the mma_ one is the
> compatibility one).  This matters, because if you disable the
> compatibility builtins the vsx_ one should still be there, but not the
> old name.  (It also makes more sense of course).

I actually did it that way initially, but decided to do it this was
just to make the patch smaller.  You are correct that it's "more"
correct to rename it though. 

Peter
Florian Weimer Feb. 5, 2021, 10:28 a.m. UTC | #3
* Peter Bergner via Gcc-patches:

> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
> old names, so this patch adds support for creating compatibility built-ins
> (ie, multiple built-in functions generate the same code) and then creates
> compatibility built-ins using the new names.

Maybe add a check that the compatibility builtins are flagged as
availble using __has_builtin?

Thanks,
Florian
Peter Bergner Feb. 5, 2021, 3:09 p.m. UTC | #4
On 2/5/21 4:28 AM, Florian Weimer wrote:
> * Peter Bergner via Gcc-patches:
> 
>> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
>> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
>> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
>> old names, so this patch adds support for creating compatibility built-ins
>> (ie, multiple built-in functions generate the same code) and then creates
>> compatibility built-ins using the new names.
> 
> Maybe add a check that the compatibility builtins are flagged as
> availble using __has_builtin?

Do you mean add a test in the testsuite for this?  I can check on
adding that to the test case.

Peter
Florian Weimer Feb. 5, 2021, 3:11 p.m. UTC | #5
* Peter Bergner:

> On 2/5/21 4:28 AM, Florian Weimer wrote:
>> * Peter Bergner via Gcc-patches:
>> 
>>> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
>>> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
>>> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
>>> old names, so this patch adds support for creating compatibility built-ins
>>> (ie, multiple built-in functions generate the same code) and then creates
>>> compatibility built-ins using the new names.
>> 
>> Maybe add a check that the compatibility builtins are flagged as
>> availble using __has_builtin?
>
> Do you mean add a test in the testsuite for this?  I can check on
> adding that to the test case.

Right, in the test case.  Given that it's a new kind of built-in.
(Not sure if it makes sense.)

Thanks,
Florian
Segher Boessenkool Feb. 5, 2021, 6:25 p.m. UTC | #6
On Thu, Feb 04, 2021 at 10:05:19PM -0600, Peter Bergner wrote:
> On 2/4/21 3:16 PM, Segher Boessenkool wrote:
> > On Thu, Feb 04, 2021 at 02:40:20PM -0600, Peter Bergner wrote:
> >> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
> >> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
> >> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
> >> old names, so this patch adds support for creating compatibility built-ins
> >> (ie, multiple built-in functions generate the same code) and then creates
> >> compatibility built-ins using the new names.

> >> +#ifndef RS6000_BUILTIN_COMPAT
> >> +  #undef BU_COMPAT
> >> +  #define BU_COMPAT(ENUM, COMPAT_NAME)
> > 
> > Please do not do #undef unless necessary: it hides bugs (and that causes
> > more bugs).
> 
> I thought it was needed, since rs6000-builtin.def is #included into
> rs6000-call.c and rs6000.h multiple times.

Ah yes, that is a good enough reason.  Bill's rewrite will clean all
this (and much more) up anyway, so it is fine for now.

> >>  BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
> >> +BU_COMPAT (MMA_BUILTIN_ASSEMBLE_PAIR, "vsx_assemble_pair")
> > 
> > You should do those the other way around (the mma_ one is the
> > compatibility one).  This matters, because if you disable the
> > compatibility builtins the vsx_ one should still be there, but not the
> > old name.  (It also makes more sense of course).
> 
> I actually did it that way initially, but decided to do it this was
> just to make the patch smaller.  You are correct that it's "more"
> correct to rename it though. 

It is less work (less confusion) in the future, and the number of lines
isn't important anyway :-)


Segher
Segher Boessenkool Feb. 5, 2021, 6:28 p.m. UTC | #7
On Fri, Feb 05, 2021 at 04:11:30PM +0100, Florian Weimer wrote:
> * Peter Bergner:
> > On 2/5/21 4:28 AM, Florian Weimer wrote:
> >> Maybe add a check that the compatibility builtins are flagged as
> >> availble using __has_builtin?
> >
> > Do you mean add a test in the testsuite for this?  I can check on
> > adding that to the test case.
> 
> Right, in the test case.  Given that it's a new kind of built-in.
> (Not sure if it makes sense.)

There aren't many such tests yet, so it will be helpful just because of
that.  But __has_builtin should work for any builtin function
whatsoever, and there is nothing special about these compatibility
builtins (it is just a name, it is defined as any other).


Segher
Peter Bergner Feb. 23, 2021, 10 p.m. UTC | #8
On 2/5/21 12:28 PM, Segher Boessenkool wrote:
> On Fri, Feb 05, 2021 at 04:11:30PM +0100, Florian Weimer wrote:
>> * Peter Bergner:
>>> On 2/5/21 4:28 AM, Florian Weimer wrote:
>>>> Maybe add a check that the compatibility builtins are flagged as
>>>> availble using __has_builtin?
>>>
>>> Do you mean add a test in the testsuite for this?  I can check on
>>> adding that to the test case.
>>
>> Right, in the test case.  Given that it's a new kind of built-in.
>> (Not sure if it makes sense.)
> 
> There aren't many such tests yet, so it will be helpful just because of
> that.  But __has_builtin should work for any builtin function
> whatsoever, and there is nothing special about these compatibility
> builtins (it is just a name, it is defined as any other).

__has_builtin does work for the compat builtins, so I added tests using it.
Here is the updated patch given the review comments:



rs6000: Add support for compatibility built-ins

The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
__builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
__builtin_vsx_disassemble_pair respectively.  It's too late to remove the
old names, so this patch renames the built-ins to the new names and then
adds support for creating compatibility built-ins (ie, multiple built-in
functions generate the same code) and then creates compatibility built-ins
using the old names.

This passed bootstrap and regtesting on powerpc64le-linux with no regressions.
Ok for mainline?

This will need backporting to GCC 10.  Ok there too once it's baked on
trunk for a little while?

Peter

2021-02-23  Peter Bergner  <bergner@linux.ibm.com>

gcc/
	* config/rs6000/mma.md (mma_assemble_pair): Rename from this...
	(vsx_assemble_pair): ...to this.
	(*mma_assemble_pair): Rename from this...
	(*vsx_assemble_pair): ...to this.
	(mma_disassemble_pair): Rename from this...
	(vsx_disassemble_pair): ...to this.
	(*mma_disassemble_pair): Rename from this...
	(*vsx_disassemble_pair): ...to this.
	* gcc/config/rs6000/rs6000-builtin.def (BU_MMA_V2, BU_MMA_V3,
	BU_COMPAT): New macros.
	(mma_assemble_pair): Rename from this...
	(vsx_assemble_pair): ...to this.
	(mma_disassemble_pair): Rename from this...
	(vsx_disassemble_pair): ...to this.
	(mma_assemble_pair): Add compatibility built-in.
	(mma_disassemble_pair): Likewise.
	* config/rs6000/rs6000-call.c (struct builtin_compatibility): New.
	(RS6000_BUILTIN_COMPAT): Define.
	(bdesc_compat): New.
	(mma_expand_builtin): Use VSX_BUILTIN_DISASSEMBLE_PAIR_INTERNAL.
	(rs6000_gimple_fold_mma_builtin): Use MMA_BUILTIN_DISASSEMBLE_PAIR
	and VSX_BUILTIN_ASSEMBLE_PAIR.
	(rs6000_init_builtins): Register compatibility built-ins.
	(mma_init_builtins): USE VSX_BUILTIN_ASSEMBLE_PAIR,
	VSX_BUILTIN_ASSEMBLE_PAIR_INTERNAL, VSX_BUILTIN_DISASSEMBLE_PAIR and
	VSX_BUILTIN_DISASSEMBLE_PAIR_INTERNAL.
	* doc/extend.texi (__builtin_mma_assemble_pair): Rename from this...
	(__builtin_vsx_assemble_pair): ...to this.
	(__builtin_mma_disassemble_pair): Rename from this...
	(__builtin_vsx_disassemble_pair): ...to this.

gcc/testsuite/
	* gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c: Add tests for
	__builtin_vsx_assemble_pair and __builtin_vsx_disassemble_pair.
	Add __has_builtin tests for built-ins.
	Update expected instruction counts.

diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
index 87569f1c31d..c40501f2e09 100644
--- a/gcc/config/rs6000/mma.md
+++ b/gcc/config/rs6000/mma.md
@@ -321,7 +321,7 @@
    (set_attr "length" "*,*,16")
    (set_attr "max_prefixed_insns" "2,2,*")])
 
-(define_expand "mma_assemble_pair"
+(define_expand "vsx_assemble_pair"
   [(match_operand:OO 0 "vsx_register_operand")
    (match_operand:V16QI 1 "mma_assemble_input_operand")
    (match_operand:V16QI 2 "mma_assemble_input_operand")]
@@ -334,7 +334,7 @@
   DONE;
 })
 
-(define_insn_and_split "*mma_assemble_pair"
+(define_insn_and_split "*vsx_assemble_pair"
   [(set (match_operand:OO 0 "vsx_register_operand" "=wa")
 	(unspec:OO [(match_operand:V16QI 1 "mma_assemble_input_operand" "mwa")
 		    (match_operand:V16QI 2 "mma_assemble_input_operand" "mwa")]
@@ -351,7 +351,7 @@
   DONE;
 })
 
-(define_expand "mma_disassemble_pair"
+(define_expand "vsx_disassemble_pair"
   [(match_operand:V16QI 0 "mma_disassemble_output_operand")
    (match_operand:OO 1 "vsx_register_operand")
    (match_operand 2 "const_0_to_1_operand")]
@@ -366,7 +366,7 @@
   DONE;
 })
 
-(define_insn_and_split "*mma_disassemble_pair"
+(define_insn_and_split "*vsx_disassemble_pair"
   [(set (match_operand:V16QI 0 "mma_disassemble_output_operand" "=mwa")
        (unspec:V16QI [(match_operand:OO 1 "vsx_register_operand" "wa")
 		      (match_operand 2 "const_0_to_1_operand")]
diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 058a32abf4c..609bebdfd74 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -43,6 +43,10 @@
 	ATTR	builtin attribute information.
 	ICODE	Insn code of the function that implements the builtin.  */
 
+#ifndef RS6000_BUILTIN_COMPAT
+  #undef BU_COMPAT
+  #define BU_COMPAT(ENUM, COMPAT_NAME)
+
 #ifndef RS6000_BUILTIN_0
   #error "RS6000_BUILTIN_0 is not defined."
 #endif
@@ -87,6 +91,36 @@
   #error "RS6000_BUILTIN_X is not defined."
 #endif
 
+#else
+  /* Compatibility builtins.  These builtins are simply mapped into
+     their compatible builtin function identified by ENUM.  */
+  #undef BU_COMPAT
+  #define BU_COMPAT(ENUM, COMPAT_NAME) { ENUM, "__builtin_" COMPAT_NAME },
+
+  #undef RS6000_BUILTIN_0
+  #undef RS6000_BUILTIN_1
+  #undef RS6000_BUILTIN_2
+  #undef RS6000_BUILTIN_3
+  #undef RS6000_BUILTIN_4
+  #undef RS6000_BUILTIN_A
+  #undef RS6000_BUILTIN_D
+  #undef RS6000_BUILTIN_H
+  #undef RS6000_BUILTIN_M
+  #undef RS6000_BUILTIN_P
+  #undef RS6000_BUILTIN_X
+  #define RS6000_BUILTIN_0(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_1(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_2(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_3(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_4(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_A(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_D(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_H(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_M(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_P(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_X(ENUM, NAME, MASK, ATTR, ICODE)
+#endif
+
 #ifndef BU_AV_1
 /* Define convenience macros using token pasting to allow fitting everything in
    one line.  */
@@ -368,6 +402,23 @@
 		     | RS6000_BTC_BINARY),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
+/* Like BU_MMA_2, but uses "vsx" rather than "mma" naming.  */
+#define BU_MMA_V2(ENUM, NAME, ATTR, ICODE)				\
+  RS6000_BUILTIN_M (VSX_BUILTIN_ ## ENUM,		/* ENUM */	\
+		    "__builtin_vsx_" NAME,		/* NAME */	\
+		    RS6000_BTM_MMA,			/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_BINARY				\
+		     | RS6000_BTC_VOID					\
+		     | RS6000_BTC_GIMPLE),				\
+		    CODE_FOR_nothing)			/* ICODE */	\
+  RS6000_BUILTIN_M (VSX_BUILTIN_ ## ENUM ## _INTERNAL,	/* ENUM */	\
+		    "__builtin_vsx_" NAME "_internal",	/* NAME */	\
+		    RS6000_BTM_MMA,			/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_BINARY),				\
+		    CODE_FOR_ ## ICODE)			/* ICODE */
+
 #define BU_MMA_3(ENUM, NAME, ATTR, ICODE)				\
   RS6000_BUILTIN_M (MMA_BUILTIN_ ## ENUM,		/* ENUM */	\
 		    "__builtin_mma_" NAME,		/* NAME */	\
@@ -384,6 +435,23 @@
 		     | RS6000_BTC_TERNARY),				\
 		    CODE_FOR_ ## ICODE)			/* ICODE */
 
+/* Like BU_MMA_3, but uses "vsx" rather than "mma" naming.  */
+#define BU_MMA_V3(ENUM, NAME, ATTR, ICODE)				\
+  RS6000_BUILTIN_M (VSX_BUILTIN_ ## ENUM,		/* ENUM */	\
+		    "__builtin_vsx_" NAME,		/* NAME */	\
+		    RS6000_BTM_MMA,			/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_TERNARY				\
+		     | RS6000_BTC_VOID					\
+		     | RS6000_BTC_GIMPLE),				\
+		    CODE_FOR_nothing)			/* ICODE */	\
+  RS6000_BUILTIN_M (VSX_BUILTIN_ ## ENUM ## _INTERNAL,	/* ENUM */	\
+		    "__builtin_vsx_" NAME "_internal",	/* NAME */	\
+		    RS6000_BTM_MMA,			/* MASK */	\
+		    (RS6000_BTC_ ## ATTR		/* ATTR */	\
+		     | RS6000_BTC_TERNARY),				\
+		    CODE_FOR_ ## ICODE)			/* ICODE */
+
 #define BU_MMA_5(ENUM, NAME, ATTR, ICODE)				\
   RS6000_BUILTIN_M (MMA_BUILTIN_ ## ENUM,		/* ENUM */	\
 		    "__builtin_mma_" NAME,		/* NAME */	\
@@ -3136,9 +3204,11 @@ BU_MMA_1 (XXMTACC,	    "xxmtacc",		QUAD, mma_xxmtacc)
 BU_MMA_1 (XXSETACCZ,	    "xxsetaccz",	MISC, mma_xxsetaccz)
 
 BU_MMA_2 (DISASSEMBLE_ACC, "disassemble_acc",	QUAD, mma_disassemble_acc)
-BU_MMA_2 (DISASSEMBLE_PAIR,"disassemble_pair",	PAIR, mma_disassemble_pair)
+BU_MMA_V2 (DISASSEMBLE_PAIR, "disassemble_pair", PAIR, vsx_disassemble_pair)
+BU_COMPAT (VSX_BUILTIN_DISASSEMBLE_PAIR, "mma_disassemble_pair")
 
-BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
+BU_MMA_V3 (ASSEMBLE_PAIR,   "assemble_pair",	MISC, vsx_assemble_pair)
+BU_COMPAT (VSX_BUILTIN_ASSEMBLE_PAIR, "mma_assemble_pair")
 BU_MMA_3 (XVBF16GER2,	    "xvbf16ger2",	MISC, mma_xvbf16ger2)
 BU_MMA_3 (XVF16GER2,	    "xvf16ger2",	MISC, mma_xvf16ger2)
 BU_MMA_3 (XVF32GER,	    "xvf32ger",		MISC, mma_xvf32ger)
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index de0ce50796d..d2bd03e7a62 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -89,6 +89,12 @@
 #define TARGET_NO_PROTOTYPE 0
 #endif
 
+struct builtin_compatibility
+{
+  const enum rs6000_builtins code;
+  const char *const name;
+};
+
 struct builtin_description
 {
   const HOST_WIDE_INT mask;
@@ -8839,6 +8845,13 @@ def_builtin (const char *name, tree type, enum rs6000_builtins code)
 	     (int)code, name, attr_string);
 }
 
+static const struct builtin_compatibility bdesc_compat[] =
+{
+#define RS6000_BUILTIN_COMPAT
+#include "rs6000-builtin.def"
+};
+#undef RS6000_BUILTIN_COMPAT
+
 /* Simple ternary operations: VECd = foo (VECa, VECb, VECc).  */
 
 #undef RS6000_BUILTIN_0
@@ -10115,7 +10128,7 @@ mma_expand_builtin (tree exp, rtx target, bool *expandedp)
 
   unsigned attr_args = attr & RS6000_BTC_OPND_MASK;
   if (attr & RS6000_BTC_QUAD
-      || fcode == MMA_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
+      || fcode == VSX_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
     attr_args++;
 
   gcc_assert (nopnds == attr_args);
@@ -11730,7 +11743,7 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
   tree new_decl;
 
   if (fncode == MMA_BUILTIN_DISASSEMBLE_ACC
-      || fncode == MMA_BUILTIN_DISASSEMBLE_PAIR)
+      || fncode == VSX_BUILTIN_DISASSEMBLE_PAIR)
     {
       /* This is an MMA disassemble built-in function.  */
       push_gimplify_context (true);
@@ -11745,7 +11758,7 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
 	 another accumulator/pair, then just copy the entire thing as is.  */
       if ((fncode == MMA_BUILTIN_DISASSEMBLE_ACC
 	   && TREE_TYPE (TREE_TYPE (dst_ptr)) == vector_quad_type_node)
-	  || (fncode == MMA_BUILTIN_DISASSEMBLE_PAIR
+	  || (fncode == VSX_BUILTIN_DISASSEMBLE_PAIR
 	      && TREE_TYPE (TREE_TYPE (dst_ptr)) == vector_pair_type_node))
 	{
 	  tree dst = build_simple_mem_ref (build1 (VIEW_CONVERT_EXPR,
@@ -11847,7 +11860,7 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi)
       gcc_unreachable ();
     }
 
-  if (fncode == MMA_BUILTIN_ASSEMBLE_PAIR)
+  if (fncode == VSX_BUILTIN_ASSEMBLE_PAIR)
     lhs = make_ssa_name (vector_pair_type_node);
   else
     lhs = make_ssa_name (vector_quad_type_node);
@@ -13447,6 +13460,18 @@ rs6000_init_builtins (void)
 #ifdef SUBTARGET_INIT_BUILTINS
   SUBTARGET_INIT_BUILTINS;
 #endif
+
+  /* Register the compatibility builtins after all of the normal
+     builtins have been defined.  */
+  const struct builtin_compatibility *d = bdesc_compat;
+  unsigned i;
+  for (i = 0; i < ARRAY_SIZE (bdesc_compat); i++, d++)
+    {
+      tree decl = rs6000_builtin_decls[(int)d->code];
+      gcc_assert (decl != NULL);
+      add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code,
+			    BUILT_IN_MD, NULL, NULL_TREE);
+    }
 }
 
 /* Returns the rs6000 builtin decl for CODE.  */
@@ -14119,7 +14144,7 @@ mma_init_builtins (void)
       else
 	{
 	  if (!(d->code == MMA_BUILTIN_DISASSEMBLE_ACC_INTERNAL
-		 || d->code == MMA_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
+		 || d->code == VSX_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
 	       && (attr & RS6000_BTC_QUAD) == 0)
 	    attr_args--;
 
@@ -14129,7 +14154,7 @@ mma_init_builtins (void)
 
       /* This is a disassemble pair/acc function. */
       if (d->code == MMA_BUILTIN_DISASSEMBLE_ACC
-	  || d->code == MMA_BUILTIN_DISASSEMBLE_PAIR)
+	  || d->code == VSX_BUILTIN_DISASSEMBLE_PAIR)
 	{
 	  op[nopnds++] = build_pointer_type (void_type_node);
 	  if (d->code == MMA_BUILTIN_DISASSEMBLE_ACC)
@@ -14143,7 +14168,7 @@ mma_init_builtins (void)
 	  unsigned j = 0;
 	  if (attr & RS6000_BTC_QUAD
 	      && d->code != MMA_BUILTIN_DISASSEMBLE_ACC_INTERNAL
-	      && d->code != MMA_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
+	      && d->code != VSX_BUILTIN_DISASSEMBLE_PAIR_INTERNAL)
 	    j = 1;
 	  for (; j < (unsigned) insn_data[icode].n_operands; j++)
 	    {
@@ -14151,7 +14176,7 @@ mma_init_builtins (void)
 	      if (gimple_func && mode == XOmode)
 		op[nopnds++] = build_pointer_type (vector_quad_type_node);
 	      else if (gimple_func && mode == OOmode
-		       && d->code == MMA_BUILTIN_ASSEMBLE_PAIR)
+		       && d->code == VSX_BUILTIN_ASSEMBLE_PAIR)
 		op[nopnds++] = build_pointer_type (vector_pair_type_node);
 	      else
 		/* MMA uses unsigned types.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e110cb01061..f89d1d8e61e 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -22208,8 +22208,8 @@ void __builtin_mma_xxsetaccz (__vector_quad *);
 void __builtin_mma_assemble_acc (__vector_quad *, vec_t, vec_t, vec_t, vec_t);
 void __builtin_mma_disassemble_acc (void *, __vector_quad *);
 
-void __builtin_mma_assemble_pair (__vector_pair *, vec_t, vec_t);
-void __builtin_mma_disassemble_pair (void *, __vector_pair *);
+void __builtin_vsx_assemble_pair (__vector_pair *, vec_t, vec_t);
+void __builtin_vsx_disassemble_pair (void *, __vector_pair *);
 
 vec_t __builtin_vsx_xvcvspbf16 (vec_t);
 vec_t __builtin_vsx_xvcvbf16spn (vec_t);
diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c b/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
index f3a857bb8c1..3bedf531de0 100644
--- a/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
@@ -12,6 +12,14 @@ foo (__vector_pair *dst, vec_t *src)
   *dst = pair;
 }
 
+void
+foo2 (__vector_pair *dst, vec_t *src)
+{
+  __vector_pair pair;
+  __builtin_vsx_assemble_pair (&pair, src[0], src[4]);
+  *dst = pair;
+}
+
 void
 bar (vec_t *dst, __vector_pair *src)
 {
@@ -21,8 +29,33 @@ bar (vec_t *dst, __vector_pair *src)
   dst[4] = res[1];
 }
 
-/* { dg-final { scan-assembler-times {\mlxv\M} 2 } } */
-/* { dg-final { scan-assembler-times {\mlxvp\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mstxv\M} 2 } } */
-/* { dg-final { scan-assembler-times {\mstxvp\M} 1 } } */
+void
+bar2 (vec_t *dst, __vector_pair *src)
+{
+  vec_t res[2];
+  __builtin_vsx_disassemble_pair (res, src);
+  dst[0] = res[0];
+  dst[4] = res[1];
+}
+
+#if !__has_builtin (__builtin_vsx_assemble_pair)
+#  error "__has_builtin (__builtin_vsx_assemble_pair) failed"
+#endif
+
+#if !__has_builtin (__builtin_vsx_disassemble_pair)
+#  error "__has_builtin (__builtin_vsx_disassemble_pair) failed"
+#endif
+
+#if !__has_builtin (__builtin_mma_assemble_pair)
+#  error "__has_builtin (__builtin_mma_assemble_pair) failed"
+#endif
+
+#if !__has_builtin (__builtin_mma_disassemble_pair)
+#  error "__has_builtin (__builtin_mma_disassemble_pair) failed"
+#endif
+
+/* { dg-final { scan-assembler-times {\mlxv\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mlxvp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mstxv\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mstxvp\M} 2 } } */
Segher Boessenkool Feb. 23, 2021, 10:53 p.m. UTC | #9
Hi!

On Tue, Feb 23, 2021 at 04:00:42PM -0600, Peter Bergner wrote:
> 	(mma_assemble_pair): Add compatibility built-in.
s/Add/New/ is better (it makes clear you do not add something to the
(already existing) mma_assemble_pair, that it is in fact new here).

> 	(mma_init_builtins): USE VSX_BUILTIN_ASSEMBLE_PAIR,

s/USE/Use/

The patch is okay for trunk and for 10.  Thank you!


Segher
Peter Bergner Feb. 23, 2021, 11:18 p.m. UTC | #10
On 2/23/21 4:53 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Feb 23, 2021 at 04:00:42PM -0600, Peter Bergner wrote:
>> 	(mma_assemble_pair): Add compatibility built-in.
> s/Add/New/ is better (it makes clear you do not add something to the
> (already existing) mma_assemble_pair, that it is in fact new here).
> 
>> 	(mma_init_builtins): USE VSX_BUILTIN_ASSEMBLE_PAIR,
> 
> s/USE/Use/
> 
> The patch is okay for trunk and for 10.  Thank you!

Fixed and pushed to trunk.  I'll push the backport after a day or two
of burn in on trunk.  Thanks!

Peter
Peter Bergner March 11, 2021, 12:21 a.m. UTC | #11
On 2/23/21 5:18 PM, Peter Bergner wrote:
> On 2/23/21 4:53 PM, Segher Boessenkool wrote:
>> The patch is okay for trunk and for 10.  Thank you!
> 
> Fixed and pushed to trunk.  I'll push the backport after a day or two
> of burn in on trunk.  Thanks!

Pushed to gcc10, so fixed everywhere.

Peter
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def
index 058a32abf4c..268f5d5b52b 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -43,6 +43,10 @@ 
 	ATTR	builtin attribute information.
 	ICODE	Insn code of the function that implements the builtin.  */
 
+#ifndef RS6000_BUILTIN_COMPAT
+  #undef BU_COMPAT
+  #define BU_COMPAT(ENUM, COMPAT_NAME)
+
 #ifndef RS6000_BUILTIN_0
   #error "RS6000_BUILTIN_0 is not defined."
 #endif
@@ -87,6 +91,36 @@ 
   #error "RS6000_BUILTIN_X is not defined."
 #endif
 
+#else
+  /* Compatibility builtins.  These builtins are simply mapped into
+     their compatible builtin function identified by ENUM.  */
+  #undef BU_COMPAT
+  #define BU_COMPAT(ENUM, COMPAT_NAME) { ENUM, "__builtin_" COMPAT_NAME },
+
+  #undef RS6000_BUILTIN_0
+  #undef RS6000_BUILTIN_1
+  #undef RS6000_BUILTIN_2
+  #undef RS6000_BUILTIN_3
+  #undef RS6000_BUILTIN_4
+  #undef RS6000_BUILTIN_A
+  #undef RS6000_BUILTIN_D
+  #undef RS6000_BUILTIN_H
+  #undef RS6000_BUILTIN_M
+  #undef RS6000_BUILTIN_P
+  #undef RS6000_BUILTIN_X
+  #define RS6000_BUILTIN_0(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_1(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_2(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_3(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_4(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_A(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_D(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_H(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_M(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_P(ENUM, NAME, MASK, ATTR, ICODE)
+  #define RS6000_BUILTIN_X(ENUM, NAME, MASK, ATTR, ICODE)
+#endif
+
 #ifndef BU_AV_1
 /* Define convenience macros using token pasting to allow fitting everything in
    one line.  */
@@ -3137,8 +3171,10 @@  BU_MMA_1 (XXSETACCZ,	    "xxsetaccz",	MISC, mma_xxsetaccz)
 
 BU_MMA_2 (DISASSEMBLE_ACC, "disassemble_acc",	QUAD, mma_disassemble_acc)
 BU_MMA_2 (DISASSEMBLE_PAIR,"disassemble_pair",	PAIR, mma_disassemble_pair)
+BU_COMPAT (MMA_BUILTIN_DISASSEMBLE_PAIR, "vsx_disassemble_pair")
 
 BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
+BU_COMPAT (MMA_BUILTIN_ASSEMBLE_PAIR, "vsx_assemble_pair")
 BU_MMA_3 (XVBF16GER2,	    "xvbf16ger2",	MISC, mma_xvbf16ger2)
 BU_MMA_3 (XVF16GER2,	    "xvf16ger2",	MISC, mma_xvf16ger2)
 BU_MMA_3 (XVF32GER,	    "xvf32ger",		MISC, mma_xvf32ger)
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index ae0c761f0a4..240533dec55 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -89,6 +89,12 @@ 
 #define TARGET_NO_PROTOTYPE 0
 #endif
 
+struct builtin_compatibility
+{
+  const enum rs6000_builtins code;
+  const char *const name;
+};
+
 struct builtin_description
 {
   const HOST_WIDE_INT mask;
@@ -8839,6 +8845,13 @@  def_builtin (const char *name, tree type, enum rs6000_builtins code)
 	     (int)code, name, attr_string);
 }
 
+static const struct builtin_compatibility bdesc_compat[] =
+{
+#define RS6000_BUILTIN_COMPAT
+#include "rs6000-builtin.def"
+};
+#undef RS6000_BUILTIN_COMPAT
+
 /* Simple ternary operations: VECd = foo (VECa, VECb, VECc).  */
 
 #undef RS6000_BUILTIN_0
@@ -13445,6 +13458,18 @@  rs6000_init_builtins (void)
 #ifdef SUBTARGET_INIT_BUILTINS
   SUBTARGET_INIT_BUILTINS;
 #endif
+
+  /* Register the compatibility builtins after all of the normal
+     builtins have been defined.  */
+  const struct builtin_compatibility *d = bdesc_compat;
+  unsigned i;
+  for (i = 0; i < ARRAY_SIZE (bdesc_compat); i++, d++)
+    {
+      tree decl = rs6000_builtin_decls[(int)d->code];
+      gcc_assert (decl != NULL);
+      add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code,
+			    BUILT_IN_MD, NULL, NULL_TREE);
+    }
 }
 
 /* Returns the rs6000 builtin decl for CODE.  */
diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c b/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
index f3a857bb8c1..4b1ba3148e2 100644
--- a/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-4.c
@@ -12,6 +12,14 @@  foo (__vector_pair *dst, vec_t *src)
   *dst = pair;
 }
 
+void
+foo2 (__vector_pair *dst, vec_t *src)
+{
+  __vector_pair pair;
+  __builtin_vsx_assemble_pair (&pair, src[0], src[4]);
+  *dst = pair;
+}
+
 void
 bar (vec_t *dst, __vector_pair *src)
 {
@@ -21,8 +29,17 @@  bar (vec_t *dst, __vector_pair *src)
   dst[4] = res[1];
 }
 
-/* { dg-final { scan-assembler-times {\mlxv\M} 2 } } */
-/* { dg-final { scan-assembler-times {\mlxvp\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mstxv\M} 2 } } */
-/* { dg-final { scan-assembler-times {\mstxvp\M} 1 } } */
+void
+bar2 (vec_t *dst, __vector_pair *src)
+{
+  vec_t res[2];
+  __builtin_vsx_disassemble_pair (res, src);
+  dst[0] = res[0];
+  dst[4] = res[1];
+}
+
+/* { dg-final { scan-assembler-times {\mlxv\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mlxvp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mstxv\M} 4 } } */
+/* { dg-final { scan-assembler-times {\mstxvp\M} 2 } } */