Re: [PATCH 01/14] Initial create of rs6000-genbif.c.
On Tue, Feb 04, 2020 at 04:44:04PM -0600, Bill Schmidt wrote: > >"ldv" certainly is shorter and nicer in principle, but it is a bit > >cryptic. As I said, it's probably not too hard to get used to it; and > >maybe a better name will present itself? > Maybe ldvec and stvec would serve without introducing specific builtin > confusion. Let's go with that, if nothing better shows up. > >That's not what I meant... Can you say > > [TARGET_ALTIVEC && TARGET_64BIT] > >here? Or even just > > [!TARGET_ALTIVEC] > >or > > [1] > >for always, or > > [0] > >for never ("commented out"). > Ah! Sorry for misunderstanding. Right now just an identifier is > allowed, but we could certainly grab the whole string between the [] and > drop it in with no concerns. Hopefully we both remember when we get to > the patch that reads the stanzas... :-) Segher
Re: [PATCH 01/14] Initial create of rs6000-genbif.c.
On 2/4/20 4:36 PM, Segher Boessenkool wrote: On Tue, Feb 04, 2020 at 03:10:32PM -0600, Bill Schmidt wrote: I really don't think using the new acronym "bif" helps; built-in functions already are often called "builtins" (or "intrinsics", which is problematic itself). Until we manage to replace the old methods, we already have rs6000-builtin.def, so I am a bit constrained in my choices. Given that restriction, what name would you prefer? I can use rs6000-builtins.def (the plural) if you like. As we discussed (offline), maybe rs6000-builtin-new.def is best (and at the end of this conversion, just move it). +1 + ldv Needs special handling for vec_ld semantics + stv Needs special handling for vec_st semantics Call those "vec_ld" and "vec_st", then? Or should I get used to it, the names aren't obvious, but cut-and-paste always is ;-) Hm. Well, vec_ld is a specific built-in, but this applies to a few more than just that one. But sure, if you want. "ldv" certainly is shorter and nicer in principle, but it is a bit cryptic. As I said, it's probably not too hard to get used to it; and maybe a better name will present itself? Maybe ldvec and stvec would serve without introducing specific builtin confusion. +[TARGET_ALTIVEC] Can this be a C expression? Most gen* programs just copy similar things to the generated C code, which can be interesting to debug, but works perfectly well otherwise. I rather prefer the way it is. I do generate C code from this in the subsequent patches. But I like table-driven code to use things that look like tables for input. :-) That's not what I meant... Can you say [TARGET_ALTIVEC && TARGET_64BIT] here? Or even just [!TARGET_ALTIVEC] or [1] for always, or [0] for never ("commented out"). Ah! Sorry for misunderstanding. Right now just an identifier is allowed, but we could certainly grab the whole string between the [] and drop it in with no concerns. Hopefully we both remember when we get to the patch that reads the stanzas... + Blank lines may be used as desired in these files. Between stanzas and stuff only? There are places where newlines are significant and not just whitespace, right? I don't believe so, although there may be places where I forgot to allow a line to be advanced -- that would be a bug, though, so let me know if you see any. Blank lines don't have any inherent meaning in the input files. Not blank lines, I'm asking about newlines :-) But those are not allowed to be inserted just anywhere, a line has to be one line, iiuc? Yes. Additional newlines can follow a newline, but the individual lines must contain everything that's expected in them. Bill Segher
Re: [PATCH 01/14] Initial create of rs6000-genbif.c.
On Tue, Feb 04, 2020 at 03:10:32PM -0600, Bill Schmidt wrote: > >I really don't think using the new acronym "bif" helps; built-in > >functions already are often called "builtins" (or "intrinsics", which is > >problematic itself). > > Until we manage to replace the old methods, we already have > rs6000-builtin.def, so I am a bit constrained in my choices. Given that > restriction, what name would you prefer? I can use rs6000-builtins.def > (the plural) if you like. As we discussed (offline), maybe rs6000-builtin-new.def is best (and at the end of this conversion, just move it). > >>+ ldv Needs special handling for vec_ld semantics > >>+ stv Needs special handling for vec_st semantics > >Call those "vec_ld" and "vec_st", then? Or should I get used to it, the > >names aren't obvious, but cut-and-paste always is ;-) > > Hm. Well, vec_ld is a specific built-in, but this applies to a few more > than just that one. But sure, if you want. "ldv" certainly is shorter and nicer in principle, but it is a bit cryptic. As I said, it's probably not too hard to get used to it; and maybe a better name will present itself? > >>+[TARGET_ALTIVEC] > >Can this be a C expression? Most gen* programs just copy similar things > >to the generated C code, which can be interesting to debug, but works > >perfectly well otherwise. > > I rather prefer the way it is. I do generate C code from this in the > subsequent patches. But I like table-driven code to use things that > look like tables for input. :-) That's not what I meant... Can you say [TARGET_ALTIVEC && TARGET_64BIT] here? Or even just [!TARGET_ALTIVEC] or [1] for always, or [0] for never ("commented out"). > >>+ Blank lines may be used as desired in these files. > >Between stanzas and stuff only? There are places where newlines are > >significant and not just whitespace, right? > > I don't believe so, although there may be places where I forgot to allow > a line to be advanced -- that would be a bug, though, so let me know if > you see any. Blank lines don't have any inherent meaning in the input > files. Not blank lines, I'm asking about newlines :-) But those are not allowed to be inserted just anywhere, a line has to be one line, iiuc? Segher
Re: [PATCH 01/14] Initial create of rs6000-genbif.c.
On 2/4/20 12:27 PM, Segher Boessenkool wrote: Hi! On Mon, Feb 03, 2020 at 08:26:02PM -0600, Bill Schmidt wrote: Includes header documentation and initial set of include directives. Please use full sentences in commit messages. OK. +/* This program generates built-in function initialization and + recognition code for Power targets, based on text files that + describe the built-in functions and vector overloads: + + rs6000-bif.def Table of built-in functions + rs6000-overload.def Table of overload functions I really don't think using the new acronym "bif" helps; built-in functions already are often called "builtins" (or "intrinsics", which is problematic itself). Until we manage to replace the old methods, we already have rs6000-builtin.def, so I am a bit constrained in my choices. Given that restriction, what name would you prefer? I can use rs6000-builtins.def (the plural) if you like. I didn't think I was inventing "bif" as shorthand, but maybe that was an LLVM thing... + ext Process as a vec_extract function Please spell out "extract"? There are too many other words starting with "ext", some of which you could expect here ("extend", "extension", maybe even "extra"); OK. + ldv Needs special handling for vec_ld semantics + stv Needs special handling for vec_st semantics Call those "vec_ld" and "vec_st", then? Or should I get used to it, the names aren't obvious, but cut-and-paste always is ;-) Hm. Well, vec_ld is a specific built-in, but this applies to a few more than just that one. But sure, if you want. +[TARGET_ALTIVEC] Can this be a C expression? Most gen* programs just copy similar things to the generated C code, which can be interesting to debug, but works perfectly well otherwise. I rather prefer the way it is. I do generate C code from this in the subsequent patches. But I like table-driven code to use things that look like tables for input. :-) + const vector signed char __builtin_altivec_abs_v16qi (vector signed char); +ABS_V16QI absv16qi2 {abs} + const vector signed short __builtin_altivec_abs_v8hi (vector signed short); +ABS_V8HI absv8hi2 {abs} + + Note the use of indentation, which is recommended but not required. It does require a single newline at the end of each such line, right? Does that work aout almost always, or do you get very long lines? Yes, for now I am requiring the newline at the end of each line. I found that it does indeed get very long (unreadably long) lines for vector signatures. I forgot to update this documentation when I changed my format. I am now using abbreviations for vector types that match those we use often in our test cases ("vuc" for "vector unsigned char", "vsll" for "vector signed long long", etc.). This makes for very nicely readable inputs (see patch #2). The above now becomes const vsc __builtin_altivec_abs_v16qi (vsc); ABS_V16QI absv16qi2 {abs} const vss __builtin_altivec_abs_v8hi (vss); ABS_V8HI absv8hi2 {abs} I will fix the documentation! + [, , ] Hrm, "internal" suggests "name within the GCC code", but that is not what it means. Maybe something like abi-name and builtin-name? OK, that's reasonable. + Blank lines may be used as desired in these files. Between stanzas and stuff only? There are places where newlines are significant and not just whitespace, right? I don't believe so, although there may be places where I forgot to allow a line to be advanced -- that would be a bug, though, so let me know if you see any. Blank lines don't have any inherent meaning in the input files. Great docs, thanks! Thanks for the review! Bill Segher
Re: [PATCH 01/14] Initial create of rs6000-genbif.c.
Hi! On Mon, Feb 03, 2020 at 08:26:02PM -0600, Bill Schmidt wrote: > Includes header documentation and initial set of include directives. Please use full sentences in commit messages. > +/* This program generates built-in function initialization and > + recognition code for Power targets, based on text files that > + describe the built-in functions and vector overloads: > + > + rs6000-bif.def Table of built-in functions > + rs6000-overload.def Table of overload functions I really don't think using the new acronym "bif" helps; built-in functions already are often called "builtins" (or "intrinsics", which is problematic itself). > + ext Process as a vec_extract function Please spell out "extract"? There are too many other words starting with "ext", some of which you could expect here ("extend", "extension", maybe even "extra"); > + ldv Needs special handling for vec_ld semantics > + stv Needs special handling for vec_st semantics Call those "vec_ld" and "vec_st", then? Or should I get used to it, the names aren't obvious, but cut-and-paste always is ;-) > +[TARGET_ALTIVEC] Can this be a C expression? Most gen* programs just copy similar things to the generated C code, which can be interesting to debug, but works perfectly well otherwise. > + const vector signed char __builtin_altivec_abs_v16qi (vector signed char); > +ABS_V16QI absv16qi2 {abs} > + const vector signed short __builtin_altivec_abs_v8hi (vector signed short); > +ABS_V8HI absv8hi2 {abs} > + > + Note the use of indentation, which is recommended but not required. It does require a single newline at the end of each such line, right? Does that work aout almost always, or do you get very long lines? > + [, , ] Hrm, "internal" suggests "name within the GCC code", but that is not what it means. Maybe something like abi-name and builtin-name? > + Blank lines may be used as desired in these files. Between stanzas and stuff only? There are places where newlines are significant and not just whitespace, right? Great docs, thanks! Segher