[PATCH 00/28] rs6000: Auto-generate builtins from descriptions

[Bill Schmidt]

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