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