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 <vectype>" 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 ++
> 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
>
