| Message ID | cover.1580782131.git.wschmidt@linux.ibm.com |
|---|---|
| Headers | show |
| Series | rs6000: Begin replacing built-in support | expand |
Hi! On Mon, Feb 03, 2020 at 08:26:01PM -0600, Bill Schmidt wrote: > 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. Woohoo :-) > 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-genbif) to: Let's call it rs6000-gen-builtins or similar. Not as cryptic. > Note that none of the code in this patch set affects GCC's operation > at all, with the exception of patch #14. Patch 14 causes the program > rs6000-genbif to be built and executed, producing the output files, > and linking rs6000-bif.o into the executable. However, none of the > code in rs6000-bif.o is called, so the only effect is to make the gcc > executable larger. If it builds at all ;-) > I'd like to consider at least patches 1-13 as stage 4 material for the > current release. I'd prefer to also include patch 14 for convenience, > but I understand if that's not deemed acceptable. > > I've attempted to break this up into logical pieces for easy > consumption, but some of the patches may still be a bit large. Please > let me know if you'd like me to break any of them up. I will. "Large" isn't a problem if it is a lot of the same thing. If it is two or more things, having them in separate patches is easier to review; if there is just one case that is different, put it in a separate patch if that can be done; otherwise, please point it out in the patch commit message. > Initial create of rs6000-genbif.c. Subjects do not end in a dot (but do start with a capital). > Add stubs for input files. These will grow much larger. The second half of this does not belong in the title, but in the body. Segher
On Tue, Feb 4, 2020 at 6:40 PM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > Hi! > > On Mon, Feb 03, 2020 at 08:26:01PM -0600, Bill Schmidt wrote: > > 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. > > Woohoo :-) Indeed. > > 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-genbif) to: > > Let's call it rs6000-gen-builtins or similar. Not as cryptic. I believe we talked about this a few years ago. Any reason this is powerpc specific? If sufficiently generic most targets would benefit and maybe even frontends and the middle-end could make use of this. The generator program, that is. (disclaimer: I didn't look into the patches at all) I always wondered if we can make our C frontend spit out things from C declarations (with maybe extra #pragmas for some of the more obscure details) and how to fit that into the bootstrap. > > Note that none of the code in this patch set affects GCC's operation > > at all, with the exception of patch #14. Patch 14 causes the program > > rs6000-genbif to be built and executed, producing the output files, > > and linking rs6000-bif.o into the executable. However, none of the > > code in rs6000-bif.o is called, so the only effect is to make the gcc > > executable larger. > > If it builds at all ;-) > > > I'd like to consider at least patches 1-13 as stage 4 material for the > > current release. I'd prefer to also include patch 14 for convenience, > > but I understand if that's not deemed acceptable. > > > > I've attempted to break this up into logical pieces for easy > > consumption, but some of the patches may still be a bit large. Please > > let me know if you'd like me to break any of them up. > > I will. "Large" isn't a problem if it is a lot of the same thing. If > it is two or more things, having them in separate patches is easier to > review; if there is just one case that is different, put it in a separate > patch if that can be done; otherwise, please point it out in the patch > commit message. > > > Initial create of rs6000-genbif.c. > > Subjects do not end in a dot (but do start with a capital). > > > Add stubs for input files. These will grow much larger. > > The second half of this does not belong in the title, but in the body. > > > Segher
Hi! On Wed, Feb 05, 2020 at 08:57:16AM +0100, Richard Biener wrote: > On Tue, Feb 4, 2020 at 6:40 PM Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > On Mon, Feb 03, 2020 at 08:26:01PM -0600, Bill Schmidt wrote: > > > 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-genbif) to: > > > > Let's call it rs6000-gen-builtins or similar. Not as cryptic. > > I believe we talked about this a few years ago. Any reason this is powerpc > specific? If sufficiently generic most targets would benefit and maybe even > frontends and the middle-end could make use of this. The generator > program, that is. (disclaimer: I didn't look into the patches at all) Absolutely, but we first want to solve the urgent problem for Power (because that is what it is); it's a huge job with that reduction of scope, already. After *that* is done, it will be clearer how to do things for what is wanted generically, will be clearer what is wanted in the first place :-) > I always wondered if we can make our C frontend spit out things from > C declarations (with maybe extra #pragmas for some of the more obscure > details) and how to fit that into the bootstrap. I think there will be too many problem cases, a direct description of the builtins will work better (but is more verbose of course). In any case, Bill's patches keep the exact same approach in rs6000 as we had before, just with some more pre-processing and macros etc.; which results in a much shorter description, many cases folded into one, which as a bonus also fixes bugs (directly, when two things you fold should be the same but are not, at least one of them is wrong; and maybe more importantly indirectly: a reader of the tables will spot errors much more easily if they fit on one screen, if you have similar entries on the screen at the same time so you *can* compare; and there will be more readers as well even, people are actually scared of having to look at it currently). So, yes, this same approach might be a good fit generically, but we'll do it for rs6000 only, in the interest of ever getting it done ;-) The generator programs etc. can move to generic code later, if that helps and there is interest in it, there isn't much (if anything) in here that is specific to our arch. Segher
On 2/5/20 6:30 AM, Segher Boessenkool wrote: > Hi! > > On Wed, Feb 05, 2020 at 08:57:16AM +0100, Richard Biener wrote: >> On Tue, Feb 4, 2020 at 6:40 PM Segher Boessenkool >> <segher@kernel.crashing.org> wrote: >>> On Mon, Feb 03, 2020 at 08:26:01PM -0600, Bill Schmidt wrote: >>>> 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-genbif) to: >>> Let's call it rs6000-gen-builtins or similar. Not as cryptic. >> I believe we talked about this a few years ago. Any reason this is powerpc >> specific? If sufficiently generic most targets would benefit and maybe even >> frontends and the middle-end could make use of this. The generator >> program, that is. (disclaimer: I didn't look into the patches at all) One thing that's powerpc-unique (I believe) is our peculiar overloading infrastructure for the original AltiVec interface (extended to cover quite a bit more territory since). But that's largely an extra level of abstraction that could eventually be optional. There's also some specificity to our vector types (things like vector bool and vector pixel) that would need to be abstracted away. Finally, there's a set of flags for special handling that are definitely Power-specific and would have to be abstracted away also. Nothing that couldn't be dealt with given enough attention, so far as I can see. But honestly I have not looked a great deal into other targets' built-in handling to see what other landmines might be present. > Absolutely, but we first want to solve the urgent problem for Power > (because that is what it is); it's a huge job with that reduction of > scope, already. After *that* is done, it will be clearer how to do > things for what is wanted generically, will be clearer what is wanted > in the first place :-) Yes, this is a necessary first step to even be able to see what's going on... > >> I always wondered if we can make our C frontend spit out things from >> C declarations (with maybe extra #pragmas for some of the more obscure >> details) and how to fit that into the bootstrap. > I think there will be too many problem cases, a direct description of > the builtins will work better (but is more verbose of course). > > In any case, Bill's patches keep the exact same approach in rs6000 as > we had before, just with some more pre-processing and macros etc.; > which results in a much shorter description, many cases folded into one, > which as a bonus also fixes bugs (directly, when two things you fold > should be the same but are not, at least one of them is wrong; and maybe > more importantly indirectly: a reader of the tables will spot errors > much more easily if they fit on one screen, if you have similar entries > on the screen at the same time so you *can* compare; and there will be > more readers as well even, people are actually scared of having to look > at it currently). > > So, yes, this same approach might be a good fit generically, but we'll > do it for rs6000 only, in the interest of ever getting it done ;-) > The generator programs etc. can move to generic code later, if that > helps and there is interest in it, there isn't much (if anything) in > here that is specific to our arch. I'll keep this possibility in mind as we move forward. It's probably a matter of months to get everything converted over just for Power. But this set of patches is the most generic; the remaining patches will all be quite Power-specific. Thanks, Bill > > Segher
On Feb 4, 2020, at 9:40 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote: >> 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-genbif) to: > > Let's call it rs6000-gen-builtins or similar. Not as cryptic. Or, config/gen-builtins. :-) We have one around here, and there are things that a generator can have that are really, generally useful. It's better to share support for common thing and for very target specific things, of course, they can go down in the rs6000 area. Our generators are written in python, and for this type of code, python is nice. My workflow with my generators let's me validate changes to the generator easily, as I can compare a before and after of the generated content. Once you push the common parts down and invite others to join the party, the next port can reuse the bits you have, and add the 1 or 5 things they need, and presto, you now have more beef, should you need it in the future. Around here, we have more beef than anyone, as we always hit things no one else has. In/out rtl parameters, out parameters, overloading, test case generation, early, late expansion, type fun, vector fun, architecture fun, optional parameters, gosh, the list just goes on and on... Anyway, as a new port comes on line, it's easier for them to retrofit the 3 new things they need into a nice infrastructure that sings and dances. Indeed, some of the older gcc port interfaces, for example, registers (FIXED_REGISTERS, CALL_REALLY_USED_REGISTERS, REG_ALLOC_ORDER, HARD_REGNO_CALLER_SAVE_MODE, REG_CLASS_NAMES, REG_CLASS_CONTENTS, REGISTER_NAMES), I think would be a nice area for a generator as well. Every port has registers, and yet, those interfaces are too clunky. But, now I guess I digress. Just wanted to say, I like your plan.
Hi! On Fri, Feb 14, 2020 at 10:34:40AM -0800, Mike Stump wrote: > On Feb 4, 2020, at 9:40 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > >> 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-genbif) to: > > > > Let's call it rs6000-gen-builtins or similar. Not as cryptic. > > Or, config/gen-builtins. :-) As explained before, to get somewhere in a reasonable time, we first need to do it for rs6000 only. This patchset is solving an acute problem for us, too, that is the *purpose* of it. It remains to be seen how much of it can be used by other targets. Pretending something is generic while it in fact is just a mass of special cases isn't useful, it's just costly. > Indeed, some of the older gcc port interfaces, for example, registers (FIXED_REGISTERS, CALL_REALLY_USED_REGISTERS, REG_ALLOC_ORDER, HARD_REGNO_CALLER_SAVE_MODE, REG_CLASS_NAMES, REG_CLASS_CONTENTS, REGISTER_NAMES), I think would be a nice area for a generator as well. Every port has registers, and yet, those interfaces are too clunky. Yes... Like, last year I renumbered the rs6000 registers. Things have to be changed in a bunch of tables, but also in a few other places that you cannot generate the code for. So on one hand a generator for this certainly would have helped, but on the other hand, it isn't the holy grail you might be after either. > Just wanted to say, I like your plan. :-) It'll be a lot of work still, let's hope we can make it for GCC 11. Segher
On Feb 14, 2020, at 1:26 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > It remains to be seen how much of it can be used by other targets. > Pretending something is generic while it in fact is just a mass of > special cases isn't useful, it's just costly. Oh, yeah, of course. Ours was designed in two pieces, the target bits, which were 100% target specific, and then the infrastructure to make those target bits go. 5,661 lines in the generator, and around 11 lines per builtin on the target side. The generator was 100% free of target things. (define_code_attr cond_name [(unordered "Unordered") (ordered "Ordered") (unlt "Less Than or Unordered") (unge "Greater Than or Equal or Unordered") (uneq "Equal or Unordered") (ltgt "Not Equal or Unordered") (unle "Less Than or Equal or Unorderd") (ungt "Greater Than or Unordered") (eq "Equal") (ne "Not Equal") (gt "Greater Than") (ge "Greater Equal") (lt "Less Than") (le "Less Equal") (gtu "Greater Than Unsigned") (geu "Greater Than Unsigned") (ltu "Less Than Unsigned") (leu "Less Than Unsigned")]) (define_builtin "arch_cmp<icond>" "arch_cmp<icond>_<type>" [ (define_desc "Compare <cond_name>") (define_outputs [(var_operand:<T_ALL_INT:TU_TYPE> 0)]) (define_inputs [(var_operand:T_ALL_INT 1) (var_operand:T_ALL_INT 2)]) (code_iterator icond) (define_rtl_pattern "arch_cmp<icond>_<mode>" [0 1 2]) (attributes [pure]) (use_note "<GCCOP>") ] ) was the typical sort of thing for the target side. We've since moved on and now generate even more from way less. 3 lines of input for an instruction, and even the 3 lines are auto extracted from the documentation for the instruction. So, roughly zero lines of input per builtin. About 5600 lines in the generator code, but that generator, oh my, it does a ton more. The new one doesn't have the nice clean split between target and non-target code, however. The later does significantly more work however, so, I don't feel bad. The one thing missing I think more from modern compiler port development is the sharing of the commonalities between ports that would make the port writing even easier.