Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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 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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
* 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 -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
* 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 -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
Re: [PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins
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
[PATCH] rs6000: Fix MMA API - 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 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.
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 @@
ATTRbuiltin 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 =
