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 <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.
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



Reply via email to