On Mon, 2020-08-24 at 14:39 -0700, Carl Love wrote: > Segher: > > On Wed, 2020-08-19 at 15:16 -0500, Segher Boessenkool wrote: > > On Wed, Aug 19, 2020 at 02:19:12PM -0500, Peter Bergner wrote: > > > On 8/14/20 7:42 PM, Segher Boessenkool wrote: > > > > I think your current code is fine; I hadn't considered Bill's > > > > upcoming > > > > rewrite. It is more important to make that go smoother than to > > > > fix some > > > > aesthetics right now. > > > > > > > > Please check if the names for the builtins match the (future) > > > > documentation, and then, approved for trunk. Thank you! > > > > > > This is also a bug in GCC 10, so this should be backported too. > > > > > > Segher, is this ok for Carl to backport to GCC 10 after it has sat > > > on > > > trunk for a few days? > > > > Yes. Thanks guys.
Hi, > > I attempted to do the backport however the patch doesn't even come > close to applying. The names XVCVBF16SPN and XVCVSPBF16 are the only > two builtin names that exist in GCC 10. The other issue is there is no > Power 10 builtin macro definitions in GCC 10. So basically, I can fix > the issue with XVCVBF16SPN and XVCVSPBF16 to be restricted to Power 10 > by adding the needed Power 10 macro definition. > > This is a whole new patch so I figure it needs to be reviewed to make > sure we want to make this change to GCC 10. I did run the regression > tests again using a Power 9 machine to verify it complies and there are > no regression test failures. > I recommend a pure and clean description of what the patch is doing in a paragraph, separate from the history. " This patch corrects the built-in definitions for cvcvspbf16 and xvcvbf16spn to restrict their use to P10 and beyond in a way that is consistent with the P8 and P9 builtin naming conventions. This is a subset of changes made to trunk ... " > Please let me know if this is OK for the GCC 10 tree. Thanks. > > Carl Love > > ------------------------------------------------------------ > [PATCH] rs6000, restrict bfloat convert intrinsic to Power 10. Back port to > GCC 10. > > gcc/ChangeLog > > 2020-08-24 Carl Love <c...@us.ibm.com> whitespace/indentation > * config/rs6000/rs6000-builtin.def: (BU_P10V_VSX_1) New builtin macro > expansion. > (XVCVBF16SPN, XVCVSPBF16): Replace macro expansion BU_VSX_1 with > BU_P10V_VSX_1. OK as long as it's clear 'replace' means the define being used, versus the macro expansions themselves being replaced. > * config/rs6000/rs6000-call.c: (VSX_BUILTIN_XVCVSPBF16, > VSX_BUILTIN_XVCVBF16SPN): > Replace with P10V_BUILTIN_XVCVSPBF16, P10V_BUILTIN_XVCVBF16SPN > respectively. s/Replace with/Rename to/ ? > --- > gcc/config/rs6000/rs6000-builtin.def | 14 ++++++++++++-- > gcc/config/rs6000/rs6000-call.c | 4 ++-- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000-builtin.def > b/gcc/config/rs6000/rs6000-builtin.def > index 88f78cb0a15..512b7629a46 100644 > --- a/gcc/config/rs6000/rs6000-builtin.def > +++ b/gcc/config/rs6000/rs6000-builtin.def > @@ -1014,6 +1014,16 @@ > | RS6000_BTC_BINARY), \ > CODE_FOR_ ## ICODE) /* ICODE */ > > +/* For builtins for power10 vector instructions that are encoded as altivec > + instructions, use __builtin_altivec_ as the builtin name. */ That comment doesn't seem to apply to this change, update or drop ? > + > +#define BU_P10V_VSX_1(ENUM, NAME, ATTR, ICODE)\ > + RS6000_BUILTIN_1 (P10_BUILTIN_ ## ENUM, /* ENUM */ \ > + "__builtin_vsx_" NAME, /* NAME */ \ > + RS6000_BTM_P10, /* MASK */ \ > + (RS6000_BTC_ ## ATTR /* ATTR */ \ > + | RS6000_BTC_UNARY), \ > + CODE_FOR_ ## ICODE) /* ICODE */ > #endif > > > @@ -2698,8 +2708,8 @@ BU_SPECIAL_X (RS6000_BUILTIN_CFSTRING, > "__builtin_cfstring", RS6000_BTM_ALWAYS, > RS6000_BTC_MISC) > > /* POWER10 MMA builtins. */ > -BU_VSX_1 (XVCVBF16SPN, "xvcvbf16spn", MISC, vsx_xvcvbf16spn) > -BU_VSX_1 (XVCVSPBF16, "xvcvspbf16", MISC, vsx_xvcvspbf16) > +BU_P10V_VSX_1 (XVCVBF16SPN, "xvcvbf16spn", MISC, vsx_xvcvbf16spn) > +BU_P10V_VSX_1 (XVCVSPBF16, "xvcvspbf16", MISC, vsx_xvcvspbf16) > > BU_MMA_1 (XXMFACC, "xxmfacc", QUAD, mma_xxmfacc) > BU_MMA_1 (XXMTACC, "xxmtacc", QUAD, mma_xxmtacc) > diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c > index 2cf78dfa5fe..fc1671e1bb2 100644 > --- a/gcc/config/rs6000/rs6000-call.c > +++ b/gcc/config/rs6000/rs6000-call.c > @@ -13383,8 +13383,8 @@ builtin_function_type (machine_mode mode_ret, > machine_mode mode_arg0, > case P8V_BUILTIN_VGBBD: > case MISC_BUILTIN_CDTBCD: > case MISC_BUILTIN_CBCDTD: > - case VSX_BUILTIN_XVCVSPBF16: > - case VSX_BUILTIN_XVCVBF16SPN: > + case P10V_BUILTIN_XVCVSPBF16: > + case P10V_BUILTIN_XVCVBF16SPN: > h.uns_p[0] = 1; > h.uns_p[1] = 1; > break; ok. thanks, -WIll
