| Message ID | 54841e0e6324ff041ff9c2c383ec29640dd3f048.1627562851.git.wschmidt@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace the Power target-specific builtin machinery | expand |
On Thu, 2021-07-29 at 08:30 -0500, Bill Schmidt wrote: > 2021-06-07 Bill Schmidt <wschmidt@linux.ibm.com> > > gcc/ > * config/rs6000/rs6000-builtin-new.def: Add always, power5, and > power6 stanzas. > --- > gcc/config/rs6000/rs6000-builtin-new.def | 72 ++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/gcc/config/rs6000/rs6000-builtin-new.def b/gcc/config/rs6000/rs6000-builtin-new.def > index 974cdc8c37c..ca694be1ac3 100644 > --- a/gcc/config/rs6000/rs6000-builtin-new.def > +++ b/gcc/config/rs6000/rs6000-builtin-new.def > @@ -184,6 +184,78 @@ > > > > +; Builtins that have been around since time immemorial or are just > +; considered available everywhere. > +[always] > + void __builtin_cpu_init (); > + CPU_INIT nothing {cpu} > + > + bool __builtin_cpu_is (string); > + CPU_IS nothing {cpu} > + > + bool __builtin_cpu_supports (string); > + CPU_SUPPORTS nothing {cpu} > + > + unsigned long long __builtin_ppc_get_timebase (); > + GET_TB rs6000_get_timebase {} > + > + double __builtin_mffs (); > + MFFS rs6000_mffs {} > + > +; This will break for long double == _Float128. libgcc history. Add a few more words to provide bigger hints for future archeological digs? (This is perhaps an obvious issue, but I'd need to do some spelunking) I see similar comments below, maybe just a wordier comment for the first occurance. Unsure... > + const long double __builtin_pack_longdouble (double, double); > + PACK_TF packtf {} > + > + unsigned long __builtin_ppc_mftb (); > + MFTB rs6000_mftb_di {32bit} > + > + void __builtin_mtfsb0 (const int<5>); > + MTFSB0 rs6000_mtfsb0 {} > + > + void __builtin_mtfsb1 (const int<5>); > + MTFSB1 rs6000_mtfsb1 {} > + > + void __builtin_mtfsf (const int<8>, double); > + MTFSF rs6000_mtfsf {} > + > + const __ibm128 __builtin_pack_ibm128 (double, double); > + PACK_IF packif {} > + > + void __builtin_set_fpscr_rn (const int[0,3]); > + SET_FPSCR_RN rs6000_set_fpscr_rn {} > + > + const double __builtin_unpack_ibm128 (__ibm128, const int<1>); > + UNPACK_IF unpackif {} > + > +; This will break for long double == _Float128. libgcc history. > + const double __builtin_unpack_longdouble (long double, const int<1>); > + UNPACK_TF unpacktf {} > + > + > +; Builtins that have been around just about forever, but not quite. > +[power5] > + fpmath double __builtin_recipdiv (double, double); > + RECIP recipdf3 {} > + > + fpmath float __builtin_recipdivf (float, float); > + RECIPF recipsf3 {} > + > + fpmath double __builtin_rsqrt (double); > + RSQRT rsqrtdf2 {} > + > + fpmath float __builtin_rsqrtf (float); > + RSQRTF rsqrtsf2 {} > + > + > +; Power6 builtins. I see in subsequent patches you also call out the ISA version in the comment. so perhaps ; Power6 builtins (ISA 2.05). Similar comment for Power5 reference above. > +[power6] > + const signed long __builtin_p6_cmpb (signed long, signed long); > + CMPB cmpbdi3 {} > + > + const signed int __builtin_p6_cmpb_32 (signed int, signed int); > + CMPB_32 cmpbsi3 {} > + > + ok. > ; AltiVec builtins. > [altivec] > const vsc __builtin_altivec_abs_v16qi (vsc);
On Tue, Aug 10, 2021 at 11:17:05AM -0500, will schmidt wrote: > On Thu, 2021-07-29 at 08:30 -0500, Bill Schmidt wrote: > > +; This will break for long double == _Float128. libgcc history. > > + const long double __builtin_pack_longdouble (double, double); > > + PACK_TF packtf {} > > Add a few more words to provide bigger hints for future archeological > digs? (This is perhaps an obvious issue, but I'd need to do some > spelunking) It is for __ibm128 only, not for other long double formats (we have three: plain double, double double, IEEE QP). So maybe the return type should be changed? The name of the builtin of course is unfortunate, but it is too late to change :-) Segher
On Thu, Jul 29, 2021 at 08:30:52AM -0500, Bill Schmidt wrote: > * config/rs6000/rs6000-builtin-new.def: Add always, power5, and > power6 stanzas. > + unsigned long __builtin_ppc_mftb (); > + MFTB rs6000_mftb_di {32bit} I'm not sure what {32bit} means... The builtin exists on both 32-bit and on 64-bit, and returns what is a "long" in both cases. The point is that it is just a single "mfspr 268" always, which is fast, and importantly has fixed and low latency. Modulo perhaps that, okay for trunk. Thanks! Segher
Hi Segher, On 8/10/21 1:38 PM, Segher Boessenkool wrote: > On Thu, Jul 29, 2021 at 08:30:52AM -0500, Bill Schmidt wrote: >> * config/rs6000/rs6000-builtin-new.def: Add always, power5, and >> power6 stanzas. >> + unsigned long __builtin_ppc_mftb (); >> + MFTB rs6000_mftb_di {32bit} > I'm not sure what {32bit} means... The builtin exists on both 32-bit > and on 64-bit, and returns what is a "long" in both cases. The point > is that it is just a single "mfspr 268" always, which is fast, and > importantly has fixed and low latency. Right. The implementation of this thing is that we have two different patterns in the machine description that get invoked depending on whether the target is 32-bit or 64-bit. The syntax in the built-ins file only allows for one pattern. So the {32bit} flag allows us to perform special processing for TARGET_32BIT, in this case to override the pattern. Later in the patch series you'll find: if (bif_is_32bit (*bifaddr) && TARGET_32BIT) { if (fcode == RS6000_BIF_MFTB) icode = CODE_FOR_rs6000_mftb_si; else gcc_unreachable (); } This is the only {32bit} built-in for now, and probably ever... I'm sure there's a better way of dealing with the mode dependency on TARGET_32BIT, but for now this matches the old behavior as closely as possible. I'm happy to take suggestions on this. Thanks for the review! Bill > > Modulo perhaps that, okay for trunk. Thanks! > > > Segher
On Tue, Aug 10, 2021 at 01:56:58PM -0500, Bill Schmidt wrote: > On 8/10/21 1:38 PM, Segher Boessenkool wrote: > >On Thu, Jul 29, 2021 at 08:30:52AM -0500, Bill Schmidt wrote: > >> * config/rs6000/rs6000-builtin-new.def: Add always, power5, and > >> power6 stanzas. > >>+ unsigned long __builtin_ppc_mftb (); > >>+ MFTB rs6000_mftb_di {32bit} > >I'm not sure what {32bit} means... The builtin exists on both 32-bit > >and on 64-bit, and returns what is a "long" in both cases. The point > >is that it is just a single "mfspr 268" always, which is fast, and > >importantly has fixed and low latency. > > Right. The implementation of this thing is that we have two different > patterns in the machine description that get invoked depending on > whether the target is 32-bit or 64-bit. The syntax in the built-ins > file only allows for one pattern. So the {32bit} flag allows us to > perform special processing for TARGET_32BIT, in this case to override > the pattern. Later in the patch series you'll find: [ snip ] Ah ok. > I'm sure there's a better way of dealing with the mode dependency on > TARGET_32BIT, but for now this matches the old behavior as closely as > possible. I'm happy to take suggestions on this. You could try to use something with Pmode, but it's not going to be pretty in any case. You also might have to deal with -m32 -mpowerpc64, depending on if the original did. Segher
Hi Segher, On 8/10/21 12:34 PM, Segher Boessenkool wrote: > On Tue, Aug 10, 2021 at 11:17:05AM -0500, will schmidt wrote: >> On Thu, 2021-07-29 at 08:30 -0500, Bill Schmidt wrote: >>> +; This will break for long double == _Float128. libgcc history. >>> + const long double __builtin_pack_longdouble (double, double); >>> + PACK_TF packtf {} >> Add a few more words to provide bigger hints for future archeological >> digs? (This is perhaps an obvious issue, but I'd need to do some >> spelunking) > It is for __ibm128 only, not for other long double formats (we have > three: plain double, double double, IEEE QP). So maybe the return type > should be changed? The name of the builtin of course is unfortunate, > but it is too late to change :-) Yeah...I'm not sure how much flexibility we have here to avoid breaking code in the field, but it's not a big break because whoever may be using it has to be assuming long double = __ibm128, and probably has work to do anyway. Perhaps I should commit as is for now, and then prepare a separate patch to change this builtin? There may be test suite fallout, not sure offhand. Thanks! Bill > > > Segher
On Tue, Aug 10, 2021 at 04:29:10PM -0500, Bill Schmidt wrote: > On 8/10/21 12:34 PM, Segher Boessenkool wrote: > >On Tue, Aug 10, 2021 at 11:17:05AM -0500, will schmidt wrote: > >>On Thu, 2021-07-29 at 08:30 -0500, Bill Schmidt wrote: > >>>+; This will break for long double == _Float128. libgcc history. > >>>+ const long double __builtin_pack_longdouble (double, double); > >>>+ PACK_TF packtf {} > >>Add a few more words to provide bigger hints for future archeological > >>digs? (This is perhaps an obvious issue, but I'd need to do some > >>spelunking) > >It is for __ibm128 only, not for other long double formats (we have > >three: plain double, double double, IEEE QP). So maybe the return type > >should be changed? The name of the builtin of course is unfortunate, > >but it is too late to change :-) > > Yeah...I'm not sure how much flexibility we have here to avoid breaking > code in the field, but it's not a big break because whoever may be using > it has to be assuming long double = __ibm128, and probably has work to > do anyway. We do have an __ibm128 __builtin_pack_ibm128 (double, double); already, so we just should get people to use that one, make it more prominent in the documentation? Or we can also make __builtin_pack_longdouble warn (or even error) if used when long double is not double-double. Maybe an attribute (or what is it called, a {thing} I mean) in the new description files to say "warn (or error) if long double is not ibm128"? > Perhaps I should commit as is for now, and then prepare a separate patch > to change this builtin? There may be test suite fallout, not sure offhand. Yes, I did approve it already, right? Reviewing these patches I notice things that should be improved, but that does not have to be done *now*, or by you for that matter :-) Cheers, Segher
diff --git a/gcc/config/rs6000/rs6000-builtin-new.def b/gcc/config/rs6000/rs6000-builtin-new.def index 974cdc8c37c..ca694be1ac3 100644 --- a/gcc/config/rs6000/rs6000-builtin-new.def +++ b/gcc/config/rs6000/rs6000-builtin-new.def @@ -184,6 +184,78 @@ +; Builtins that have been around since time immemorial or are just +; considered available everywhere. +[always] + void __builtin_cpu_init (); + CPU_INIT nothing {cpu} + + bool __builtin_cpu_is (string); + CPU_IS nothing {cpu} + + bool __builtin_cpu_supports (string); + CPU_SUPPORTS nothing {cpu} + + unsigned long long __builtin_ppc_get_timebase (); + GET_TB rs6000_get_timebase {} + + double __builtin_mffs (); + MFFS rs6000_mffs {} + +; This will break for long double == _Float128. libgcc history. + const long double __builtin_pack_longdouble (double, double); + PACK_TF packtf {} + + unsigned long __builtin_ppc_mftb (); + MFTB rs6000_mftb_di {32bit} + + void __builtin_mtfsb0 (const int<5>); + MTFSB0 rs6000_mtfsb0 {} + + void __builtin_mtfsb1 (const int<5>); + MTFSB1 rs6000_mtfsb1 {} + + void __builtin_mtfsf (const int<8>, double); + MTFSF rs6000_mtfsf {} + + const __ibm128 __builtin_pack_ibm128 (double, double); + PACK_IF packif {} + + void __builtin_set_fpscr_rn (const int[0,3]); + SET_FPSCR_RN rs6000_set_fpscr_rn {} + + const double __builtin_unpack_ibm128 (__ibm128, const int<1>); + UNPACK_IF unpackif {} + +; This will break for long double == _Float128. libgcc history. + const double __builtin_unpack_longdouble (long double, const int<1>); + UNPACK_TF unpacktf {} + + +; Builtins that have been around just about forever, but not quite. +[power5] + fpmath double __builtin_recipdiv (double, double); + RECIP recipdf3 {} + + fpmath float __builtin_recipdivf (float, float); + RECIPF recipsf3 {} + + fpmath double __builtin_rsqrt (double); + RSQRT rsqrtdf2 {} + + fpmath float __builtin_rsqrtf (float); + RSQRTF rsqrtsf2 {} + + +; Power6 builtins. +[power6] + const signed long __builtin_p6_cmpb (signed long, signed long); + CMPB cmpbdi3 {} + + const signed int __builtin_p6_cmpb_32 (signed int, signed int); + CMPB_32 cmpbsi3 {} + + ; AltiVec builtins. [altivec] const vsc __builtin_altivec_abs_v16qi (vsc);