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 <vectype>" 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. :/ As stated, I need to check with SJM whether he has a
dependency on this in pveclib, and whether that makes any sense.
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?
They are. I can't destroy them yet until I'm ready to make the complete
switchover to the new methods.
Thanks again!
Bill
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
