Re: [PATCH 00/28] rs6000: Auto-generate builtins from descriptions
Gentle ping to keep this in your inbox. :) There are still many more important things ahead of this. Bill On 6/17/20 2:46 PM, Bill Schmidt via Gcc-patches wrote: I posted a version of these patches back in stage 4 (February), but we agreed that holding off until stage 1 was a better idea. Since then I've made more progress and reorganized the patches accordingly. This group of patches lays groundwork, but does not actually change GCC's behavior yet, other than to generate the new initialization information and ignore it. The current built-in support in the rs6000 back end requires at least a master's degree in spelunking to comprehend. It's full of cruft, redundancy, and unused bits of code, and long overdue for a replacement. This is the first part of my project to do that. My intent is to make adding new built-in functions as simple as adding a few lines to a couple of files, and automatically generating as much of the initialization, overload resolution, and expansion logic as possible. This patch series establishes the format of the input files and creates a new program (rs6000-gen-builtins) to: * Parse the input files into an internal representation; * Generate a file of #defines (rs6000-vecdefines.h) for eventual inclusion into altivec.h; and * Generate an initialization file to create and initialize tables of built-in functions and overloads. Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen-builtins. Patch 8 provides balanced tree search support for parsing scalability. Patches 2 and 21-27 provide a first cut at the input files. Patch 20 incorporates the new code into the GCC build. Patch 28 adds comments to some existing files that will help during the transition from the previous builtin mechanism. The patch series is constructed so that any prefix set of the patches can be upstreamed without breaking anything, so we can take the reviews slowly. There's still plenty of work left, but I think it will be helpful to get this big chunk of patches upstream to make further progress easier. Thanks in advance for your reviews! Bill Schmidt (28): rs6000: Initial create of rs6000-gen-builtins.c rs6000: Add initial input files rs6000: Add file support and functions for diagnostic support rs6000: Add helper functions for parsing rs6000: Add functions for matching types, part 1 of 3 rs6000: Add functions for matching types, part 2 of 3 rs6000: Add functions for matching types, part 3 of 3 rs6000: Red-black tree implementation for balanced tree search rs6000: Main function with stubs for parsing and output rs6000: Parsing built-in input file, part 1 of 3 rs6000: Parsing built-in input file, part 2 of 3 rs6000: Parsing built-in input file, part 3 of 3 rs6000: Parsing of overload input file rs6000: Build and store function type identifiers rs6000: Write output to the vector definition include file rs6000: Write output to the builtins header file rs6000: Write output to the builtins init file, part 1 of 3 rs6000: Write output to the builtins init file, part 2 of 3 rs6000: Write output to the builtins init file, part 3 of 3 rs6000: Incorporate new builtins code into the build machinery rs6000: Add remaining MASK_ALTIVEC builtins rs6000: Add MASK_VSX builtins rs6000: Add available-everywhere and ancient builtins rs6000: Add Power7 builtins rs6000: Add MASK_P8_VECTOR builtins rs6000: Add MASK_P9_VECTOR and MASK_P9_MISC builtins rs6000: Add remaining builtins rs6000: Add comments to help with transition gcc/config.gcc |3 +- gcc/config/rs6000/rbtree.c | 233 ++ gcc/config/rs6000/rbtree.h | 51 + gcc/config/rs6000/rs6000-builtin-new.def | 2965 ++ gcc/config/rs6000/rs6000-builtin.def | 15 + gcc/config/rs6000/rs6000-call.c | 166 ++ gcc/config/rs6000/rs6000-gen-builtins.c | 2586 +++ gcc/config/rs6000/rs6000-overload.def| 57 + gcc/config/rs6000/t-rs6000 | 25 +- 9 files changed, 6099 insertions(+), 2 deletions(-) create mode 100644 gcc/config/rs6000/rbtree.c create mode 100644 gcc/config/rs6000/rbtree.h create mode 100644 gcc/config/rs6000/rs6000-builtin-new.def create mode 100644 gcc/config/rs6000/rs6000-gen-builtins.c create mode 100644 gcc/config/rs6000/rs6000-overload.def
Re: [PATCH 00/28] rs6000: Auto-generate builtins from descriptions
On 6/18/20 5:48 PM, will schmidt wrote: On Thu, 2020-06-18 at 17:01 -0500, Bill Schmidt wrote: Thanks for the review, Will! Responses below... On 6/18/20 11:08 AM, will schmidt wrote: On Wed, 2020-06-17 at 14:46 -0500, Bill Schmidt wrote: I posted a version of these patches back in stage 4 (February), but we agreed that holding off until stage 1 was a better idea. Since then I've made more progress and reorganized the patches accordingly. This group of patches lays groundwork, but does not actually change GCC's behavior yet, other than to generate the new initialization information and ignore it. The current built-in support in the rs6000 back end requires at least a master's degree in spelunking to comprehend. It's full of cruft, redundancy, and unused bits of code, and long overdue for a replacement. This is the first part of my project to do that. My intent is to make adding new built-in functions as simple as adding a few lines to a couple of files, and automatically generating as much of the initialization, overload resolution, and expansion logic as possible. This patch series establishes the format of the input files and creates a new program (rs6000-gen-builtins) to: * Parse the input files into an internal representation; * Generate a file of #defines (rs6000-vecdefines.h) for eventual inclusion into altivec.h; and * Generate an initialization file to create and initialize tables of built-in functions and overloads. Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen- builtins. Patch 8 provides balanced tree search support for parsing scalability. Patches 2 and 21-27 provide a first cut at the input files. Patch 20 incorporates the new code into the GCC build. Patch 28 adds comments to some existing files that will help during the transition from the previous builtin mechanism. The patch series is constructed so that any prefix set of the patches can be upstreamed without breaking anything, so we can take the reviews slowly. There's still plenty of work left, but I think it will be helpful to get this big chunk of patches upstream to make further progress easier. Thanks in advance for your reviews! I've read through the series. Nothing significant, just a few cosmetic nits, i've called them out below here, versus replying to the individual emails. generally lgtm. thanks -Will Bill Schmidt (28): rs6000: Initial create of rs6000-gen-builtins.c ok rs6000: Add initial input files Whitespace/tabs in "Legal values of " blurb. otherwise ok Urk. Will fix. rs6000: Add file support and functions for diagnostic support ok rs6000: Add helper functions for parsing ok rs6000: Add functions for matching types, part 1 of 3 ok rs6000: Add functions for matching types, part 2 of 3 ok rs6000: Add functions for matching types, part 3 of 3 ok rs6000: Red-black tree implementation for balanced tree search ok rs6000: Main function with stubs for parsing and output ok rs6000: Parsing built-in input file, part 1 of 3 ok rs6000: Parsing built-in input file, part 2 of 3 ok rs6000: Parsing built-in input file, part 3 of 3 ok rs6000: Parsing of overload input file use enums or consts instead of hardcoding values ? Is this specifically about MAXOVLDSTANZAS, MAXOVLDS, or something else? If the former, I guess I can define these in const decls instead of using #define if that's preferred. No issue with those. I was noting the constants used as the return values in the parse_ovld_entry() function. You have them clearly documented there. +/* Parse one two-line entry in the overload file. Return 0 for EOF, 1 for + success, 2 for end-of-stanza, and 6 for a parsing failure. */ So just a suggestion to use other defined values for that. I didn't notice those numbers used in the other patches, so maybe this is already fixed up elsewhere. I see, thanks for the explanation. I agree, it's not just in this patch; the return values used in parsing are a mess. I'll create an enum and use it consistently. Thanks! Bill Thanks -Will
Re: [PATCH 00/28] rs6000: Auto-generate builtins from descriptions
On Thu, 2020-06-18 at 17:01 -0500, Bill Schmidt wrote: > Thanks for the review, Will! Responses below... > > On 6/18/20 11:08 AM, will schmidt wrote: > > On Wed, 2020-06-17 at 14:46 -0500, Bill Schmidt wrote: > > > I posted a version of these patches back in stage 4 (February), > > > but we agreed that holding off until stage 1 was a better idea. > > > Since then I've made more progress and reorganized the patches > > > accordingly. This group of patches lays groundwork, but does not > > > actually change GCC's behavior yet, other than to generate the > > > new initialization information and ignore it. > > > > > > The current built-in support in the rs6000 back end requires at > > > least > > > a master's degree in spelunking to comprehend. It's full of > > > cruft, > > > redundancy, and unused bits of code, and long overdue for a > > > replacement. This is the first part of my project to do that. > > > > > > My intent is to make adding new built-in functions as simple as > > > adding > > > a few lines to a couple of files, and automatically generating as > > > much > > > of the initialization, overload resolution, and expansion logic > > > as > > > possible. This patch series establishes the format of the input > > > files > > > and creates a new program (rs6000-gen-builtins) to: > > > > > > * Parse the input files into an internal representation; > > > * Generate a file of #defines (rs6000-vecdefines.h) for > > > eventual > > > inclusion into altivec.h; and > > > * Generate an initialization file to create and initialize > > > tables of > > > built-in functions and overloads. > > > > > > Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen- > > > builtins. > > > Patch 8 provides balanced tree search support for parsing > > > scalability. > > > Patches 2 and 21-27 provide a first cut at the input files. > > > Patch 20 incorporates the new code into the GCC build. > > > Patch 28 adds comments to some existing files that will help > > > during the transition from the previous builtin mechanism. > > > > > > The patch series is constructed so that any prefix set of the > > > patches > > > can be upstreamed without breaking anything, so we can take the > > > reviews slowly. There's still plenty of work left, but I think > > > it > > > will be helpful to get this big chunk of patches upstream to make > > > further progress easier. > > > > > > Thanks in advance for your reviews! > > > > > > > I've read through the series. Nothing significant, just a few > > cosmetic > > nits, i've called them out below here, versus replying to the > > individual emails. > > > > generally lgtm. > > thanks > > -Will > > > > > > > Bill Schmidt (28): > > >rs6000: Initial create of rs6000-gen-builtins.c > > > > ok > > >rs6000: Add initial input files > > > > Whitespace/tabs in "Legal values of " blurb. > > otherwise ok > > Urk. Will fix. > > >rs6000: Add file support and functions for diagnostic support > > > > ok > > >rs6000: Add helper functions for parsing > > > > ok > > >rs6000: Add functions for matching types, part 1 of 3 > > > > ok > > > > >rs6000: Add functions for matching types, part 2 of 3 > > > > ok > > > > >rs6000: Add functions for matching types, part 3 of 3 > > > > ok > > > > >rs6000: Red-black tree implementation for balanced tree search > > > > ok > > > > >rs6000: Main function with stubs for parsing and output > > > > ok > > > > >rs6000: Parsing built-in input file, part 1 of 3 > > > > ok > > >rs6000: Parsing built-in input file, part 2 of 3 > > > > ok > > >rs6000: Parsing built-in input file, part 3 of 3 > > > > ok > > >rs6000: Parsing of overload input file > > > > use enums or consts instead of hardcoding values ? > > Is this specifically about MAXOVLDSTANZAS, MAXOVLDS, or something > else? > If the former, I guess I can define these in const decls instead of > using #define if that's preferred. No issue with those. I was noting the constants used as the return values in the parse_ovld_entry() function. You have them clearly documented there. +/* Parse one two-line entry in the overload file. Return 0 for EOF, 1 for + success, 2 for end-of-stanza, and 6 for a parsing failure. */ So just a suggestion to use other defined values for that. I didn't notice those numbers used in the other patches, so maybe this is already fixed up elsewhere. Thanks -Will
Re: [PATCH 00/28] rs6000: Auto-generate builtins from descriptions
Thanks for the review, Will! Responses below...
On 6/18/20 11:08 AM, will schmidt wrote:
On Wed, 2020-06-17 at 14:46 -0500, Bill Schmidt wrote:
I posted a version of these patches back in stage 4 (February),
but we agreed that holding off until stage 1 was a better idea.
Since then I've made more progress and reorganized the patches
accordingly. This group of patches lays groundwork, but does not
actually change GCC's behavior yet, other than to generate the
new initialization information and ignore it.
The current built-in support in the rs6000 back end requires at least
a master's degree in spelunking to comprehend. It's full of cruft,
redundancy, and unused bits of code, and long overdue for a
replacement. This is the first part of my project to do that.
My intent is to make adding new built-in functions as simple as
adding
a few lines to a couple of files, and automatically generating as
much
of the initialization, overload resolution, and expansion logic as
possible. This patch series establishes the format of the input
files
and creates a new program (rs6000-gen-builtins) to:
* Parse the input files into an internal representation;
* Generate a file of #defines (rs6000-vecdefines.h) for eventual
inclusion into altivec.h; and
* Generate an initialization file to create and initialize tables of
built-in functions and overloads.
Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen-builtins.
Patch 8 provides balanced tree search support for parsing
scalability.
Patches 2 and 21-27 provide a first cut at the input files.
Patch 20 incorporates the new code into the GCC build.
Patch 28 adds comments to some existing files that will help
during the transition from the previous builtin mechanism.
The patch series is constructed so that any prefix set of the patches
can be upstreamed without breaking anything, so we can take the
reviews slowly. There's still plenty of work left, but I think it
will be helpful to get this big chunk of patches upstream to make
further progress easier.
Thanks in advance for your reviews!
I've read through the series. Nothing significant, just a few cosmetic
nits, i've called them out below here, versus replying to the
individual emails.
generally lgtm.
thanks
-Will
Bill Schmidt (28):
rs6000: Initial create of rs6000-gen-builtins.c
ok
rs6000: Add initial input files
Whitespace/tabs in "Legal values of " blurb.
otherwise ok
Urk. Will fix.
rs6000: Add file support and functions for diagnostic support
ok
rs6000: Add helper functions for parsing
ok
rs6000: Add functions for matching types, part 1 of 3
ok
rs6000: Add functions for matching types, part 2 of 3
ok
rs6000: Add functions for matching types, part 3 of 3
ok
rs6000: Red-black tree implementation for balanced tree search
ok
rs6000: Main function with stubs for parsing and output
ok
rs6000: Parsing built-in input file, part 1 of 3
ok
rs6000: Parsing built-in input file, part 2 of 3
ok
rs6000: Parsing built-in input file, part 3 of 3
ok
rs6000: Parsing of overload input file
use enums or consts instead of hardcoding values ?
Is this specifically about MAXOVLDSTANZAS, MAXOVLDS, or something else?
If the former, I guess I can define these in const decls instead of
using #define if that's preferred.
rs6000: Build and store function type identifiers
ok
rs6000: Write output to the vector definition include file
ok
rs6000: Write output to the builtins header file
Nit, i'd probably add a 'FIXME' keyword near the "_x" conflict
avoidance so this is easy to find later on. That said, this is rather
obvious, so nbd. :-)
That's fair, will add.
ok.
rs6000: Write output to the builtins init file, part 1 of 3
BILLDEBUG reference should probably be culled. :-)
Thanks, good catch. Needs to be a separate patch on my test branch for
the ongoing work...
ok
rs6000: Write output to the builtins init file, part 2 of 3
ok
rs6000: Write output to the builtins init file, part 3 of 3
ok
rs6000: Incorporate new builtins code into the build machinery
ok
rs6000: Add remaining MASK_ALTIVEC builtins
A few very long lines (__builtin_vec_init_v16qi, etc) ; but I think
those are fine.
Yeah, they have to be, with the simplistic parsing strategy I'm using.
So long as there aren't many of them, I feel it's not too unreadable.
ok
rs6000: Add MASK_VSX builtins
For the pick-one-or-the-other, i'd tend to the generic looking one.
i.e.
const vf __builtin_vsx_xvcvuxwsp(vui);
CVCVUXWSP_V4SF vsx_xvcvuxwsp {}
.. looks better to me. Though there may be subtlety that I don't
understand.
Yeah, for now let me add a FIXME to such things. This is just an
acknowledgment that currently we have defined two names and patterns
where we only need one...
ok
rs6000: Add available-everywhere and ancient builtins
No opinion or thoughts on the packtf or unpacktf entries.
Me either, yet. :/
Re: [PATCH 00/28] rs6000: Auto-generate builtins from descriptions
On Wed, 2020-06-17 at 14:46 -0500, Bill Schmidt wrote:
> I posted a version of these patches back in stage 4 (February),
> but we agreed that holding off until stage 1 was a better idea.
> Since then I've made more progress and reorganized the patches
> accordingly. This group of patches lays groundwork, but does not
> actually change GCC's behavior yet, other than to generate the
> new initialization information and ignore it.
>
> The current built-in support in the rs6000 back end requires at least
> a master's degree in spelunking to comprehend. It's full of cruft,
> redundancy, and unused bits of code, and long overdue for a
> replacement. This is the first part of my project to do that.
>
> My intent is to make adding new built-in functions as simple as
> adding
> a few lines to a couple of files, and automatically generating as
> much
> of the initialization, overload resolution, and expansion logic as
> possible. This patch series establishes the format of the input
> files
> and creates a new program (rs6000-gen-builtins) to:
>
> * Parse the input files into an internal representation;
> * Generate a file of #defines (rs6000-vecdefines.h) for eventual
>inclusion into altivec.h; and
> * Generate an initialization file to create and initialize tables of
>built-in functions and overloads.
>
> Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen-builtins.
> Patch 8 provides balanced tree search support for parsing
> scalability.
> Patches 2 and 21-27 provide a first cut at the input files.
> Patch 20 incorporates the new code into the GCC build.
> Patch 28 adds comments to some existing files that will help
> during the transition from the previous builtin mechanism.
>
> The patch series is constructed so that any prefix set of the patches
> can be upstreamed without breaking anything, so we can take the
> reviews slowly. There's still plenty of work left, but I think it
> will be helpful to get this big chunk of patches upstream to make
> further progress easier.
>
> Thanks in advance for your reviews!
>
I've read through the series. Nothing significant, just a few cosmetic
nits, i've called them out below here, versus replying to the
individual emails.
generally lgtm.
thanks
-Will
>
> Bill Schmidt (28):
> rs6000: Initial create of rs6000-gen-builtins.c
ok
> rs6000: Add initial input files
Whitespace/tabs in "Legal values of " blurb.
otherwise ok
> rs6000: Add file support and functions for diagnostic support
ok
> rs6000: Add helper functions for parsing
ok
> rs6000: Add functions for matching types, part 1 of 3
ok
> rs6000: Add functions for matching types, part 2 of 3
ok
> rs6000: Add functions for matching types, part 3 of 3
ok
> rs6000: Red-black tree implementation for balanced tree search
ok
> rs6000: Main function with stubs for parsing and output
ok
> rs6000: Parsing built-in input file, part 1 of 3
ok
> rs6000: Parsing built-in input file, part 2 of 3
ok
> rs6000: Parsing built-in input file, part 3 of 3
ok
> rs6000: Parsing of overload input file
use enums or consts instead of hardcoding values ?
> rs6000: Build and store function type identifiers
ok
> rs6000: Write output to the vector definition include file
ok
> rs6000: Write output to the builtins header file
Nit, i'd probably add a 'FIXME' keyword near the "_x" conflict
avoidance so this is easy to find later on. That said, this is rather
obvious, so nbd. :-)
ok.
> rs6000: Write output to the builtins init file, part 1 of 3
BILLDEBUG reference should probably be culled. :-)
ok
> rs6000: Write output to the builtins init file, part 2 of 3
ok
> rs6000: Write output to the builtins init file, part 3 of 3
ok
> rs6000: Incorporate new builtins code into the build machinery
ok
> rs6000: Add remaining MASK_ALTIVEC builtins
A few very long lines (__builtin_vec_init_v16qi, etc) ; but I think
those are fine.
ok
> rs6000: Add MASK_VSX builtins
For the pick-one-or-the-other, i'd tend to the generic looking one.
i.e.
const vf __builtin_vsx_xvcvuxwsp(vui);
CVCVUXWSP_V4SF vsx_xvcvuxwsp {}
.. looks better to me. Though there may be subtlety that I don't
understand.
ok
> rs6000: Add available-everywhere and ancient builtins
No opinion or thoughts on the packtf or unpacktf entries.
ok
> rs6000: Add Power7 builtins
ok
> rs6000: Add MASK_P8_VECTOR builtins
ok
> rs6000: Add MASK_P9_VECTOR and MASK_P9_MISC builtins
ok
> rs6000: Add remaining builtins
ok
> rs6000: Add comments to help with transition
Are the deprecations old enough that these can be dropped outright?
ok
>
> gcc/config.gcc |3 +-
> gcc/config/rs6000/rbtree.c | 233 ++
> gcc/config/rs6000/rbtree.h | 51 +
> gcc/config/rs6000/rs6000-builtin-new.def | 2965
> ++
> gcc/config/rs6000/rs6000-builtin.def | 15 +
> gcc/config/rs6000/rs6000-call.c | 166 ++
>
[PATCH 00/28] rs6000: Auto-generate builtins from descriptions
I posted a version of these patches back in stage 4 (February), but we agreed that holding off until stage 1 was a better idea. Since then I've made more progress and reorganized the patches accordingly. This group of patches lays groundwork, but does not actually change GCC's behavior yet, other than to generate the new initialization information and ignore it. The current built-in support in the rs6000 back end requires at least a master's degree in spelunking to comprehend. It's full of cruft, redundancy, and unused bits of code, and long overdue for a replacement. This is the first part of my project to do that. My intent is to make adding new built-in functions as simple as adding a few lines to a couple of files, and automatically generating as much of the initialization, overload resolution, and expansion logic as possible. This patch series establishes the format of the input files and creates a new program (rs6000-gen-builtins) to: * Parse the input files into an internal representation; * Generate a file of #defines (rs6000-vecdefines.h) for eventual inclusion into altivec.h; and * Generate an initialization file to create and initialize tables of built-in functions and overloads. Patches 1, 3-7, and 9-19 contain the logic for rs6000-gen-builtins. Patch 8 provides balanced tree search support for parsing scalability. Patches 2 and 21-27 provide a first cut at the input files. Patch 20 incorporates the new code into the GCC build. Patch 28 adds comments to some existing files that will help during the transition from the previous builtin mechanism. The patch series is constructed so that any prefix set of the patches can be upstreamed without breaking anything, so we can take the reviews slowly. There's still plenty of work left, but I think it will be helpful to get this big chunk of patches upstream to make further progress easier. Thanks in advance for your reviews! Bill Schmidt (28): rs6000: Initial create of rs6000-gen-builtins.c rs6000: Add initial input files rs6000: Add file support and functions for diagnostic support rs6000: Add helper functions for parsing rs6000: Add functions for matching types, part 1 of 3 rs6000: Add functions for matching types, part 2 of 3 rs6000: Add functions for matching types, part 3 of 3 rs6000: Red-black tree implementation for balanced tree search rs6000: Main function with stubs for parsing and output rs6000: Parsing built-in input file, part 1 of 3 rs6000: Parsing built-in input file, part 2 of 3 rs6000: Parsing built-in input file, part 3 of 3 rs6000: Parsing of overload input file rs6000: Build and store function type identifiers rs6000: Write output to the vector definition include file rs6000: Write output to the builtins header file rs6000: Write output to the builtins init file, part 1 of 3 rs6000: Write output to the builtins init file, part 2 of 3 rs6000: Write output to the builtins init file, part 3 of 3 rs6000: Incorporate new builtins code into the build machinery rs6000: Add remaining MASK_ALTIVEC builtins rs6000: Add MASK_VSX builtins rs6000: Add available-everywhere and ancient builtins rs6000: Add Power7 builtins rs6000: Add MASK_P8_VECTOR builtins rs6000: Add MASK_P9_VECTOR and MASK_P9_MISC builtins rs6000: Add remaining builtins rs6000: Add comments to help with transition gcc/config.gcc |3 +- gcc/config/rs6000/rbtree.c | 233 ++ gcc/config/rs6000/rbtree.h | 51 + gcc/config/rs6000/rs6000-builtin-new.def | 2965 ++ gcc/config/rs6000/rs6000-builtin.def | 15 + gcc/config/rs6000/rs6000-call.c | 166 ++ gcc/config/rs6000/rs6000-gen-builtins.c | 2586 +++ gcc/config/rs6000/rs6000-overload.def| 57 + gcc/config/rs6000/t-rs6000 | 25 +- 9 files changed, 6099 insertions(+), 2 deletions(-) create mode 100644 gcc/config/rs6000/rbtree.c create mode 100644 gcc/config/rs6000/rbtree.h create mode 100644 gcc/config/rs6000/rs6000-builtin-new.def create mode 100644 gcc/config/rs6000/rs6000-gen-builtins.c create mode 100644 gcc/config/rs6000/rs6000-overload.def -- 2.17.1
