* [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define @ 2020-09-24 20:35 will schmidt 2020-09-25 17:36 ` Segher Boessenkool 0 siblings, 1 reply; 7+ messages in thread From: will schmidt @ 2020-09-24 20:35 UTC (permalink / raw) To: Gcc-patches Cc: Segher Boessenkool, David Edelsohn, Bill Schmidt, Peter Bergner [PATCH, rs6000] correct an erroneous blip in the BU_P10_MISC define Hi, We have extraneous BTM entry (RS6000_BTM_POWERPC64) in the define for our P10 MISC 2 builtin definition. This does not exist for the '0', '1' or '3' definitions. It appears to me that this was erroneously copied from the P7 version of the define which contains a version of the BU macro both with and without that element. Removing the RS6000_BTM_POWERPC64 portion of the define does not introduce any obvious failures, I believe this extra line can be safely removed. OK for trunk? Thanks -Will diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index e91a48ddf5fe..62c9b77cb76d 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -1112,12 +1112,11 @@ CODE_FOR_ ## ICODE) /* ICODE */ #define BU_P10_MISC_2(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_2 (P10_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ - RS6000_BTM_P10 \ - | RS6000_BTM_POWERPC64, /* MASK */ \ + RS6000_BTM_P10, /* MASK */ \ (RS6000_BTC_ ## ATTR /* ATTR */ \ | RS6000_BTC_BINARY), \ CODE_FOR_ ## ICODE) /* ICODE */ #define BU_P10_MISC_3(ENUM, NAME, ATTR, ICODE) \ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define 2020-09-24 20:35 [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define will schmidt @ 2020-09-25 17:36 ` Segher Boessenkool 2020-09-25 20:34 ` will schmidt 0 siblings, 1 reply; 7+ messages in thread From: Segher Boessenkool @ 2020-09-25 17:36 UTC (permalink / raw) To: will schmidt; +Cc: Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner Hi! On Thu, Sep 24, 2020 at 03:35:24PM -0500, will schmidt wrote: > We have extraneous BTM entry (RS6000_BTM_POWERPC64) in the define for > our P10 MISC 2 builtin definition. This does not exist for the '0', > '1' or '3' definitions. It appears to me that this was erroneously > copied from the P7 version of the define which contains a version of the > BU macro both with and without that element. Removing the > RS6000_BTM_POWERPC64 portion of the define does not introduce any obvious > failures, I believe this extra line can be safely removed. No, it cannot. This is used for pdepd/pextd/cntlzdm/cnttzdm/cfuged, all of which do need 64-bit registers to do anything sane. This should really have defined some new builtin class, and I thought we could just be tricky and take a massive shortcut. Bill has been hit by this already as well, sigh :-( Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define 2020-09-25 17:36 ` Segher Boessenkool @ 2020-09-25 20:34 ` will schmidt 2020-09-25 23:50 ` Segher Boessenkool 0 siblings, 1 reply; 7+ messages in thread From: will schmidt @ 2020-09-25 20:34 UTC (permalink / raw) To: Segher Boessenkool Cc: Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner On Fri, 2020-09-25 at 12:36 -0500, Segher Boessenkool wrote: > Hi! > > On Thu, Sep 24, 2020 at 03:35:24PM -0500, will schmidt wrote: > > We have extraneous BTM entry (RS6000_BTM_POWERPC64) in the > > define for > > our P10 MISC 2 builtin definition. This does not exist for the > > '0', > > '1' or '3' definitions. It appears to me that this was erroneously > > copied from the P7 version of the define which contains a version > > of the > > BU macro both with and without that element. Removing the > > RS6000_BTM_POWERPC64 portion of the define does not introduce any > > obvious > > failures, I believe this extra line can be safely removed. > > No, it cannot. > > This is used for pdepd/pextd/cntlzdm/cnttzdm/cfuged, all of which do > need 64-bit registers to do anything sane. > > This should really have defined some new builtin class, and I thought > we > could just be tricky and take a massive shortcut. Bill has been hit > by > this already as well, sigh :-( Ok. The usage of that macro seems to be limited to those that you have referenced. i.e. /* 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) BU_P10_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) BU_P10_MISC_2 (PDEPD, "pdepd", CONST, pdepd) BU_P10_MISC_2 (PEXTD, "pextd", CONST, pextd) So looking at the power7 entries that have the BTM_POWERPC64 entry.. BU_P7_MISC_2 (DIVWE, "divwe", CONST, dive_si) BU_P7_MISC_2 (DIVWEU, "divweu", CONST, diveu_si) BU_P7_POWERPC64_MISC_2 (DIVDE, "divde", CONST, dive_di) BU_P7_POWERPC64_MISC_2 (DIVDEU, "divdeu", CONST, diveu_di) Would it be suitable to rename the P10 macro to BU_P10_POWERPC64_MISC_2 ? I'd then debate whether to add a unused macro to fill the gap between BU_P10_MISC_1 and BU_P10_MISC_2 If you've got schemes for a deeper fix, i'd need another hint. :-) thanks -Will > > > Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define 2020-09-25 20:34 ` will schmidt @ 2020-09-25 23:50 ` Segher Boessenkool 2020-09-27 13:55 ` Bill Schmidt 0 siblings, 1 reply; 7+ messages in thread From: Segher Boessenkool @ 2020-09-25 23:50 UTC (permalink / raw) To: will schmidt; +Cc: Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner On Fri, Sep 25, 2020 at 03:34:49PM -0500, will schmidt wrote: > On Fri, 2020-09-25 at 12:36 -0500, Segher Boessenkool wrote: > > No, it cannot. > > > > This is used for pdepd/pextd/cntlzdm/cnttzdm/cfuged, all of which do > > need 64-bit registers to do anything sane. > > > > This should really have defined some new builtin class, and I thought > > we > > could just be tricky and take a massive shortcut. Bill has been hit > > by > > this already as well, sigh :-( > > Ok. > > The usage of that macro seems to be limited to those that you have > referenced. i.e. > > /* 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) > BU_P10_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) > BU_P10_MISC_2 (PDEPD, "pdepd", CONST, pdepd) > BU_P10_MISC_2 (PEXTD, "pextd", CONST, pextd) > > So looking at the power7 entries that have the BTM_POWERPC64 entry.. > > BU_P7_MISC_2 (DIVWE, "divwe", CONST, dive_si) > BU_P7_MISC_2 (DIVWEU, "divweu", CONST, diveu_si) > BU_P7_POWERPC64_MISC_2 (DIVDE, "divde", CONST, dive_di) > BU_P7_POWERPC64_MISC_2 (DIVDEU, "divdeu", CONST, diveu_di) > > Would it be suitable to rename the P10 macro to > BU_P10_POWERPC64_MISC_2 ? Yes. But that requires some more infrastructure I thought... Maybe not though? And we can do that anyway of course, it's not like we do not have way way way too much there already. > I'd then debate whether to add a unused macro to fill the gap between > BU_P10_MISC_1 and BU_P10_MISC_2 Nah, don't bother, those are just names, the numbers are meaningless :-) > If you've got schemes for a deeper fix, i'd need another hint. :-) Talk with Bill if this makes things easier for him / harder / no difference? Thanks, Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define 2020-09-25 23:50 ` Segher Boessenkool @ 2020-09-27 13:55 ` Bill Schmidt 2020-10-07 17:44 ` [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2 will schmidt 0 siblings, 1 reply; 7+ messages in thread From: Bill Schmidt @ 2020-09-27 13:55 UTC (permalink / raw) To: Segher Boessenkool, will schmidt Cc: Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner On 9/25/20 6:50 PM, Segher Boessenkool wrote: > On Fri, Sep 25, 2020 at 03:34:49PM -0500, will schmidt wrote: >> On Fri, 2020-09-25 at 12:36 -0500, Segher Boessenkool wrote: >>> No, it cannot. >>> >>> This is used for pdepd/pextd/cntlzdm/cnttzdm/cfuged, all of which do >>> need 64-bit registers to do anything sane. >>> >>> This should really have defined some new builtin class, and I thought >>> we >>> could just be tricky and take a massive shortcut. Bill has been hit >>> by >>> this already as well, sigh :-( >> Ok. >> >> The usage of that macro seems to be limited to those that you have >> referenced. i.e. >> >> /* 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) >> BU_P10_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) >> BU_P10_MISC_2 (PDEPD, "pdepd", CONST, pdepd) >> BU_P10_MISC_2 (PEXTD, "pextd", CONST, pextd) >> >> So looking at the power7 entries that have the BTM_POWERPC64 entry.. >> >> BU_P7_MISC_2 (DIVWE, "divwe", CONST, dive_si) >> BU_P7_MISC_2 (DIVWEU, "divweu", CONST, diveu_si) >> BU_P7_POWERPC64_MISC_2 (DIVDE, "divde", CONST, dive_di) >> BU_P7_POWERPC64_MISC_2 (DIVDEU, "divdeu", CONST, diveu_di) >> >> Would it be suitable to rename the P10 macro to >> BU_P10_POWERPC64_MISC_2 ? > Yes. But that requires some more infrastructure I thought... Maybe not > though? And we can do that anyway of course, it's not like we do not > have way way way too much there already. > >> I'd then debate whether to add a unused macro to fill the gap between >> BU_P10_MISC_1 and BU_P10_MISC_2 > Nah, don't bother, those are just names, the numbers are meaningless :-) > >> If you've got schemes for a deeper fix, i'd need another hint. :-) > Talk with Bill if this makes things easier for him / harder / no > difference? What Will has in mind is what I would prefer. I identified this as a naming problem above all else. The only issue for me is that I nearly missed it when converting things to use the new builtin methodology, because it wasn't made obvious by the naming. Thanks, Bill > > Thanks, > > > Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2 2020-09-27 13:55 ` Bill Schmidt @ 2020-10-07 17:44 ` will schmidt 2020-10-08 0:48 ` Segher Boessenkool 0 siblings, 1 reply; 7+ messages in thread From: will schmidt @ 2020-10-07 17:44 UTC (permalink / raw) To: wschmidt, Segher Boessenkool Cc: Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner Hi, Rename our BU_P10_MISC_2 built-in define macro to be BU_P10_POWERPC64_MISC_2. This more accurately reflects that the macro includes the RS6000_BTM_POWERPC64 entry that is not present in the other BU_P10_MISC macros, and matches the style we used for the P7 equivalent. Should be entirely cosmetic, no codegen changes. A regtest is underway just in case. OK for trunk? Thanks, -Will gcc/ChangeLog: * gcc/config/rs6000/rs6000-builtin.def (BU_P10_MISC_2): Rename to BU_P10_POWERPC64_MISC_2. (CFUGED,CNTLZDM,CNTTZDM,PDEPD,PEXTD): Call renamed macro. diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index e91a48ddf5fe..3eb55f0ae434 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -1109,11 +1109,11 @@ RS6000_BTM_P10, /* MASK */ \ (RS6000_BTC_ ## ATTR /* ATTR */ \ | RS6000_BTC_UNARY), \ CODE_FOR_ ## ICODE) /* ICODE */ -#define BU_P10_MISC_2(ENUM, NAME, ATTR, ICODE) \ +#define BU_P10_POWERPC64_MISC_2(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_2 (P10_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ RS6000_BTM_P10 \ | RS6000_BTM_POWERPC64, /* MASK */ \ (RS6000_BTC_ ## ATTR /* ATTR */ \ @@ -2725,15 +2725,15 @@ BU_P9_64BIT_2 (CMPEQB, "byte_in_set", CONST, cmpeqb) 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") \f /* 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) -BU_P10_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) -BU_P10_MISC_2 (PDEPD, "pdepd", CONST, pdepd) -BU_P10_MISC_2 (PEXTD, "pextd", CONST, pextd) +BU_P10_POWERPC64_MISC_2 (CFUGED, "cfuged", CONST, cfuged) +BU_P10_POWERPC64_MISC_2 (CNTLZDM, "cntlzdm", CONST, cntlzdm) +BU_P10_POWERPC64_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) +BU_P10_POWERPC64_MISC_2 (PDEPD, "pdepd", CONST, pdepd) +BU_P10_POWERPC64_MISC_2 (PEXTD, "pextd", CONST, pextd) /* Builtins for vector instructions added in ISA 3.1 (power10). */ BU_P10V_AV_2 (VCLRLB, "vclrlb", CONST, vclrlb) BU_P10V_AV_2 (VCLRRB, "vclrrb", CONST, vclrrb) BU_P10V_AV_2 (VCFUGED, "vcfuged", CONST, vcfuged) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2 2020-10-07 17:44 ` [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2 will schmidt @ 2020-10-08 0:48 ` Segher Boessenkool 0 siblings, 0 replies; 7+ messages in thread From: Segher Boessenkool @ 2020-10-08 0:48 UTC (permalink / raw) To: will schmidt Cc: wschmidt, Gcc-patches, David Edelsohn, Bill Schmidt, Peter Bergner Hi! On Wed, Oct 07, 2020 at 12:44:04PM -0500, will schmidt wrote: > Rename our BU_P10_MISC_2 built-in define macro to be > BU_P10_POWERPC64_MISC_2. This more accurately reflects > that the macro includes the RS6000_BTM_POWERPC64 entry > that is not present in the other BU_P10_MISC macros, > and matches the style we used for the P7 equivalent. > > Should be entirely cosmetic, no codegen changes. > A regtest is underway just in case. This is okay for trunk. Thank you! > * gcc/config/rs6000/rs6000-builtin.def (BU_P10_MISC_2): Rename > to BU_P10_POWERPC64_MISC_2. > (CFUGED,CNTLZDM,CNTTZDM,PDEPD,PEXTD): Call renamed macro. (space after comma?) Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-10-08 0:49 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-24 20:35 [PATCH, rs6000] correct an erroneous BTM value in the BU_P10_MISC define will schmidt 2020-09-25 17:36 ` Segher Boessenkool 2020-09-25 20:34 ` will schmidt 2020-09-25 23:50 ` Segher Boessenkool 2020-09-27 13:55 ` Bill Schmidt 2020-10-07 17:44 ` [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2 will schmidt 2020-10-08 0:48 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).