| Message ID | mptk19vfemc.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a simulate_builin_function_decl langhook | expand |
On 9/26/19 6:04 AM, Richard Sandiford wrote: > Although it's possible to define the SVE intrinsics in a normal header > file, it's much more convenient to define them directly in the compiler. > This also speeds up compilation and gives better error messages. > > The idea is therefore for arm_sve.h (the main intrinsics header file) > to have the pragma: > > #pragma GCC aarch64 "arm_sve.h" > > telling GCC to define (almost) everything arm_sve.h needs to define. > The target then needs a way of injecting new built-in function > declarations during compilation. > > The main hook for defining built-in functions is add_builtin_function. > This is designed for use at start-up, and so has various features that > are correct in that context but not for the pragma above: > > (1) the location is always BUILTINS_LOCATION, whereas for arm_sve.h > it ought to be the location of the pragma. > > (2) the function is only immediately visible if it's in the implementation > namespace, whereas the pragma is deliberately injecting functions > into the general namespace. > > (3) there's no attempt to emulate a normal function declaration in > C or C++, whereas functions declared by the pragma should be > checked in the same way as an open-coded declaration would be. > E.g. we should get an error if there was a previous incompatible > declaration. > > (4) in C++, the function is treated as extern "C" and so can't be > overloaded, whereas SVE intrinsics do use function overloading. > > This patch therefore adds a hook that targets can use to inject > the equivalent of a source-level function declaration, but bound > to a BUILT_IN_MD function. > > The main SVE intrinsic patch has tests to make sure that we report an > error for conflicting definitions that appear either before or after > including arm_sve.h. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? I'm struggling to see how this is significantly better than a suitable arm_sve.h file. Can you walk me through more of the motivation side? jeff
Jeff Law <law@redhat.com> writes: > On 9/26/19 6:04 AM, Richard Sandiford wrote: >> Although it's possible to define the SVE intrinsics in a normal header >> file, it's much more convenient to define them directly in the compiler. >> This also speeds up compilation and gives better error messages. >> >> The idea is therefore for arm_sve.h (the main intrinsics header file) >> to have the pragma: >> >> #pragma GCC aarch64 "arm_sve.h" >> >> telling GCC to define (almost) everything arm_sve.h needs to define. >> The target then needs a way of injecting new built-in function >> declarations during compilation. >> >> The main hook for defining built-in functions is add_builtin_function. >> This is designed for use at start-up, and so has various features that >> are correct in that context but not for the pragma above: >> >> (1) the location is always BUILTINS_LOCATION, whereas for arm_sve.h >> it ought to be the location of the pragma. >> >> (2) the function is only immediately visible if it's in the implementation >> namespace, whereas the pragma is deliberately injecting functions >> into the general namespace. >> >> (3) there's no attempt to emulate a normal function declaration in >> C or C++, whereas functions declared by the pragma should be >> checked in the same way as an open-coded declaration would be. >> E.g. we should get an error if there was a previous incompatible >> declaration. >> >> (4) in C++, the function is treated as extern "C" and so can't be >> overloaded, whereas SVE intrinsics do use function overloading. >> >> This patch therefore adds a hook that targets can use to inject >> the equivalent of a source-level function declaration, but bound >> to a BUILT_IN_MD function. >> >> The main SVE intrinsic patch has tests to make sure that we report an >> error for conflicting definitions that appear either before or after >> including arm_sve.h. >> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > I'm struggling to see how this is significantly better than a suitable > arm_sve.h file. Can you walk me through more of the motivation side? Sure. This message is going to go to the other extreme, sorry, but I'm not sure which part will be the most convincing (if any). I guess I should start by saying that SVE intrinsics have three types of predication (zeroing, merging, and "don't care") that combine with the usual type suffixes seen in arm_neon.h. This gives 3,736 functions for baseline SVE (SVE2 adds more). The vast majority of those functions can't be open-coded using the core parts of C and C++, even with GNU extensions. Some could perhaps be coded using new library extensions, but that just shifts the question from "how do we implement this feature in arm_sve.h?" to "how we implement this feature in the library extension?". An alternative to open coding using the core language is to use asm statements. But implementing intrinsics that way produces poor code, and we wanted to avoid it even as a temporary measure. So that leaves us using built-in functions for almost all of those 3,736 functions. With the traditional approach, the target would need to register the functions at start-up and then define the header file in terms of them. There are two approaches to doing that. One is to define the built-in functions under their header file name so that they become active once a prototype is seen. But that's only appropriate for functions like printf that have linkage. The arm_sve.h functions don't have linkage and there's a chance that non-SVE code could be using the same names for something else (perhaps even with the same prototype, in the case of things like uint64_t svcntb (void); that don't mention SVE types). The other alternative is to define builtins in the "__builtin_" namespace and wrap them in inline wrappers, which I think is what most intrinsics header files do. E.g., from arm_neon.h: __extension__ extern __inline float64x2_t __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) vabsq_f64 (float64x2_t __a) { return __builtin_aarch64_absv2df (__a); } But that's 6 lines per function. Counting a blank line between each one, we'd end up with a header file of at least 26,151 lines. (OK, so arm_neon.h is already longer than that. But with SVE2 and with other considerations mentioned below, arm_sve.h could easily end up into 6 figures this way.) It's very hard to maintain header files like that by hand without introducing errors, such as forgetting to put the arguments safely in the implementation namespace ("__a" rather than "a" etc.). Kyrill fixed some arm_neon.h instances of this the other week. And using macros to handle common patterns just makes the error messages worse, since the macros then show up in error backtraces. An alternative to maintaining the header file by hand is to generate it via a script. Ideally the script would use the same metadata as the compiler itself uses when registering the built-in functions. But this means writing two pieces of code to process the metadata, one to generate text for the inline wrappers and one to register the built-ins. And we still end up with the same very long header file. A more fundamental problem with inline wrappers is that they make it harder to honour the spec for constant arguments. If an instruction requires a constant operand, Arm has traditionally required the associated intrinsic argument to be an integer constant expression (in the C and C++ sense). GCC has tended to fudge this and instead only requires an integer constant at expand time, after inlining and constant propagation have taken place. But this means that all sorts of other optimisations have happened too, and so what's constant at expand time wasn't necessarily constant at the language level. Admittedly some people like that behaviour :-), but it means that what's acceptable depends on the vagaries compiler optimisation. It also means that code is often not portable between GCC and clang, which implements the spec more closely. So the pragma approach seemed better for several reasons: (1) The compiler registers (almost) the same built-in functions as it would anyway, but it registers them when they're actually needed rather than at start-up. This saves the compile-time cost and memory footprint associated with registering thousands of functions that most TUs won't need. (2) It avoids the need to maintain a long header file by hand, or maintain separate scripts/programs to generate the header. (3) It avoids the compile-time overhead of parsing a large header file. (4) The frontend can check that the arguments to a function are conforming (using the new TARGET_CHECK_BUILTIN_CALL hook) rather than expand having to fill the gap. [It might be that even TARGET_CHECK_BUILTIN_CALL doesn't have a true idea of which arguments are integer constant expressions, but that's something we could iterate on, and it's going to much closer than the alternative.] (5) It improves the diagnostics: there are no inline wrappers, and so inline wrappers don't show up in the backtrace of error messages. That's why I think this approach is better even for traditional intrinsic headers like arm_neon.h. But there's more :-) For SVE we wanted to provide overloaded functions to reduce verbosity, rather than require the types of the arguments to be given as part of the function name. This is especially useful for gathers and scatters, where fully specifying the types involved leads to long function names. For C++, the overloading of course comes as part of the language. An implementation in the header file could just define the overloaded functions in the same way as the non-overloaded functions, although that bulks the header file quite a bit. But the overloaded intrinsics are available in C too. Clang has an "overloadable" attribute that allows the above C++ definitions to work for C as well, which is one way of implementing the C side. We don't have that attribute in GCC though. Another way of implementing the C overloading would be to use _Generic, and the spec was designed to be compatible with that approach for implementations that chose to use it. But _Generic was never going to be the perfect approach from a QoI perspective, for several reasons: (a) it has exponential behaviour for nested calls (see PR91937 for a recent example of this) (b) it gives poor error messages for invalid calls (c) the complicated _Generic macro shows up in the error backtrace As Jakub says in PR91937, we now implement tgmath.h overloading directly in GCC, even though that's what _Generic was originally designed for. And it seemed like that was the best approach for arm_sve.h too. So what we did was: (a) For C++, have the pragma define the individual instances of overloaded functions in the same way as the header file would, except that they're BUILT_IN_MD functions and so have no function body. (b) For C, have the pragma define a single built-in function per overloaded function name and make TARGET_RESOLVE_OVERLOADED_BUILTIN resolve calls appropriately. IMO the error messages we get from (b) are better than the messages we get from (a). If there's no matching overload, the C++ frontend just prints a list of candidate functions, which is usually quite long and probably not that helpful. (There might be times when the user genuinely doesn't know which overloads are available, but more often than not these kinds of messages come from simple mistakes.) (b) can instead work to a higher-level view of what the functions do and so has a clearer idea what the "real" error is likely to be. FWIW, the current arm_sve.h is just 7 lines, not including the guard and licence. Thanks, Richard
On 10/5/19 5:29 AM, Richard Sandiford wrote: > > Sure. This message is going to go to the other extreme, sorry, but I'm > not sure which part will be the most convincing (if any). No worries. Worst case going to the other extreme is I have to read it more than once after nodding off halfway through :-) Seriously though. THanks for the more expansive write up. > > I guess I should start by saying that SVE intrinsics have three types of > predication (zeroing, merging, and "don't care") that combine with the usual > type suffixes seen in arm_neon.h. This gives 3,736 functions for baseline > SVE (SVE2 adds more). Ugh. That'd be a lot of stuff to add into a header. As you noted later, there's significant costs to doing so. > > The vast majority of those functions can't be open-coded using the > core parts of C and C++, even with GNU extensions. Some could perhaps > be coded using new library extensions, but that just shifts the question > from "how do we implement this feature in arm_sve.h?" to "how we implement > this feature in the library extension?". True. But one could ask if those extensions are something that we're likely to need beyond what you're doing now. But I don't necessarily think that would override the desire not to have so much stuff in the header. I'm guessing that even though you can't describe what you need at the C/C++ level, but you can describe at least some of what you want in gimple/rtl? Otherwise I'm not sure what you get above and beyond asms. > > So that leaves us using built-in functions for almost all of those 3,736 > functions. With the traditional approach, the target would need to > register the functions at start-up and then define the header file in > terms of them. Yup. And there's a cost you missed -- those things tend to end up in the debugging output as well. That's caused problems with system tools in the past. > > There are two approaches to doing that. One is to define the built-in > functions under their header file name so that they become active once > a prototype is seen. But that's only appropriate for functions like > printf that have linkage. The arm_sve.h functions don't have linkage > and there's a chance that non-SVE code could be using the same names for > something else (perhaps even with the same prototype, in the case of > things like > > uint64_t svcntb (void); > > that don't mention SVE types). > > The other alternative is to define builtins in the "__builtin_" > namespace and wrap them in inline wrappers, which I think is what > most intrinsics header files do. E.g., from arm_neon.h: > > __extension__ extern __inline float64x2_t > __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) > vabsq_f64 (float64x2_t __a) > { > return __builtin_aarch64_absv2df (__a); > } > > But that's 6 lines per function. Counting a blank line between > each one, we'd end up with a header file of at least 26,151 lines. > (OK, so arm_neon.h is already longer than that. But with SVE2 and with > other considerations mentioned below, arm_sve.h could easily end up into > 6 figures this way.) Yea, that's kind of what I would expect for exposing this stuff. But I didn't realize how many of these you were going to need. [ ... ] > > It's very hard to maintain header files like that by hand without > introducing errors, such as forgetting to put the arguments safely > in the implementation namespace ("__a" rather than "a" etc.). Kyrill > fixed some arm_neon.h instances of this the other week. And using macros > to handle common patterns just makes the error messages worse, since the > macros then show up in error backtraces. Yup. We've seen this across multiple targets. Y'all aren't alone here. > > An alternative to maintaining the header file by hand is to generate > it via a script. Ideally the script would use the same metadata as > the compiler itself uses when registering the built-in functions. > But this means writing two pieces of code to process the metadata, > one to generate text for the inline wrappers and one to register the > built-ins. And we still end up with the same very long header file. Yup. It's not ideal. > > A more fundamental problem with inline wrappers is that they make it > harder to honour the spec for constant arguments. If an instruction > requires a constant operand, Arm has traditionally required the > associated intrinsic argument to be an integer constant expression > (in the C and C++ sense). GCC has tended to fudge this and instead only > requires an integer constant at expand time, after inlining and constant > propagation have taken place. But this means that all sorts of other > optimisations have happened too, and so what's constant at expand time > wasn't necessarily constant at the language level. > > Admittedly some people like that behaviour :-), but it means that what's > acceptable depends on the vagaries compiler optimisation. It also means > that code is often not portable between GCC and clang, which implements > the spec more closely. I go back and forth on this kind of thing. Sometimes I'd rather see us be more strict, other times such strictness seems to be getting in the way for no good reason, particularly when after inlining/optimization, etc we're going to get the form we need. I'm sure from a user standpoint the inconsistency is annoying as hell. > > So the pragma approach seemed better for several reasons: [ ... ] I tend to agree, particularly due to the number of builtins you'd end up needing. It's not so bad when the number is small, but at the scale you're doing the old fashioned way seems inferior. Let's go with with it. Again, thanks for the write-up. jeff
Jeff Law <law@redhat.com> writes: > On 10/5/19 5:29 AM, Richard Sandiford wrote: >> >> Sure. This message is going to go to the other extreme, sorry, but I'm >> not sure which part will be the most convincing (if any). > No worries. Worst case going to the other extreme is I have to read it > more than once after nodding off halfway through :-) :-) > Seriously though. THanks for the more expansive write up. > >> I guess I should start by saying that SVE intrinsics have three types of >> predication (zeroing, merging, and "don't care") that combine with the usual >> type suffixes seen in arm_neon.h. This gives 3,736 functions for baseline >> SVE (SVE2 adds more). > Ugh. That'd be a lot of stuff to add into a header. As you noted > later, there's significant costs to doing so. > >> The vast majority of those functions can't be open-coded using the >> core parts of C and C++, even with GNU extensions. Some could perhaps >> be coded using new library extensions, but that just shifts the question >> from "how do we implement this feature in arm_sve.h?" to "how we implement >> this feature in the library extension?". > True. But one could ask if those extensions are something that we're > likely to need beyond what you're doing now. But I don't necessarily > think that would override the desire not to have so much stuff in the > header. > > I'm guessing that even though you can't describe what you need at the > C/C++ level, but you can describe at least some of what you want in > gimple/rtl? Otherwise I'm not sure what you get above and beyond asms. Yeah, some of the simpler stuff, like building length-agnostic constants, can be represented directly in gimple. Some other things map directly to internal functions like IFN_MASK_LOAD and IFN_MASK_STORE. The vast majority of the functions still needs to be a built-in function during gimple though, e.g. so that we don't "lose" a necessary or useful predicate argument. The situation's similar in rtl, with a lot of stuff mapping to unspecs. >> So that leaves us using built-in functions for almost all of those 3,736 >> functions. With the traditional approach, the target would need to >> register the functions at start-up and then define the header file in >> terms of them. > Yup. And there's a cost you missed -- those things tend to end up in > the debugging output as well. That's caused problems with system tools > in the past. Ah, yeah, that's true. And for the C overloading I mentioned, the traditional approach would still need to use _Generic-based macros for some cases, which would also show up if macro debug output is enabled. And the macros wouldn't be useful for debugging, since what they expand to wouldn't be callable interactively. >> There are two approaches to doing that. One is to define the built-in >> functions under their header file name so that they become active once >> a prototype is seen. But that's only appropriate for functions like >> printf that have linkage. The arm_sve.h functions don't have linkage >> and there's a chance that non-SVE code could be using the same names for >> something else (perhaps even with the same prototype, in the case of >> things like >> >> uint64_t svcntb (void); >> >> that don't mention SVE types). >> >> The other alternative is to define builtins in the "__builtin_" >> namespace and wrap them in inline wrappers, which I think is what >> most intrinsics header files do. E.g., from arm_neon.h: >> >> __extension__ extern __inline float64x2_t >> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) >> vabsq_f64 (float64x2_t __a) >> { >> return __builtin_aarch64_absv2df (__a); >> } >> >> But that's 6 lines per function. Counting a blank line between >> each one, we'd end up with a header file of at least 26,151 lines. >> (OK, so arm_neon.h is already longer than that. But with SVE2 and with >> other considerations mentioned below, arm_sve.h could easily end up into >> 6 figures this way.) > Yea, that's kind of what I would expect for exposing this stuff. But I > didn't realize how many of these you were going to need. > > [ ... ] > > >> >> It's very hard to maintain header files like that by hand without >> introducing errors, such as forgetting to put the arguments safely >> in the implementation namespace ("__a" rather than "a" etc.). Kyrill >> fixed some arm_neon.h instances of this the other week. And using macros >> to handle common patterns just makes the error messages worse, since the >> macros then show up in error backtraces. > Yup. We've seen this across multiple targets. Y'all aren't alone here. > >> >> An alternative to maintaining the header file by hand is to generate >> it via a script. Ideally the script would use the same metadata as >> the compiler itself uses when registering the built-in functions. >> But this means writing two pieces of code to process the metadata, >> one to generate text for the inline wrappers and one to register the >> built-ins. And we still end up with the same very long header file. > Yup. It's not ideal. > >> >> A more fundamental problem with inline wrappers is that they make it >> harder to honour the spec for constant arguments. If an instruction >> requires a constant operand, Arm has traditionally required the >> associated intrinsic argument to be an integer constant expression >> (in the C and C++ sense). GCC has tended to fudge this and instead only >> requires an integer constant at expand time, after inlining and constant >> propagation have taken place. But this means that all sorts of other >> optimisations have happened too, and so what's constant at expand time >> wasn't necessarily constant at the language level. >> >> Admittedly some people like that behaviour :-), but it means that what's >> acceptable depends on the vagaries compiler optimisation. It also means >> that code is often not portable between GCC and clang, which implements >> the spec more closely. > I go back and forth on this kind of thing. Sometimes I'd rather see us > be more strict, other times such strictness seems to be getting in the > way for no good reason, particularly when after inlining/optimization, > etc we're going to get the form we need. Yeah, for C especially it can be awkward, since users often have to fall back to macros or #include tricks when parameterising based on a constant. For C++, constant template arguments and constexpr should help a lot. > I'm sure from a user standpoint the inconsistency is annoying as hell. > >> So the pragma approach seemed better for several reasons: > [ ... ] > I tend to agree, particularly due to the number of builtins you'd end up > needing. It's not so bad when the number is small, but at the scale > you're doing the old fashioned way seems inferior. > > Let's go with with it. Again, thanks for the write-up. Thanks! Is the related enum patch: https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01523.html also OK? (I really should have posted them together as a proper series.) Richard
Index: gcc/langhooks.h =================================================================== --- gcc/langhooks.h 2019-03-08 18:15:33.660751905 +0000 +++ gcc/langhooks.h 2019-09-26 13:02:42.543520465 +0100 @@ -494,6 +494,15 @@ struct lang_hooks backend must add all of the builtins at program initialization time. */ tree (*builtin_function_ext_scope) (tree decl); + /* Do language-specific processing for target-specific built-in + function DECL, so that it is defined in the global scope (only) + and is available without needing to be explicitly declared. + + This is intended for targets that want to inject declarations of + built-in functions into the source language (such as in response + to a pragma) rather than providing them in the source language itself. */ + tree (*simulate_builtin_function_decl) (tree decl); + /* Used to set up the tree_contains_structure array for a frontend. */ void (*init_ts) (void); @@ -562,6 +571,8 @@ extern tree add_builtin_function_ext_sco enum built_in_class cl, const char *library_name, tree attrs); +extern tree simulate_builtin_function_decl (location_t, const char *, tree, + int, const char *, tree); extern tree add_builtin_type (const char *name, tree type); /* Language helper functions. */ Index: gcc/langhooks-def.h =================================================================== --- gcc/langhooks-def.h 2019-03-08 18:15:39.328730361 +0000 +++ gcc/langhooks-def.h 2019-09-26 13:02:42.543520465 +0100 @@ -122,6 +122,7 @@ #define LANG_HOOKS_TREE_SIZE lhd_tree_s #define LANG_HOOKS_TYPES_COMPATIBLE_P lhd_types_compatible_p #define LANG_HOOKS_BUILTIN_FUNCTION lhd_builtin_function #define LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE LANG_HOOKS_BUILTIN_FUNCTION +#define LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL LANG_HOOKS_BUILTIN_FUNCTION #define LANG_HOOKS_EXPR_TO_DECL lhd_expr_to_decl #define LANG_HOOKS_TO_TARGET_CHARSET lhd_to_target_charset #define LANG_HOOKS_INIT_TS lhd_do_nothing @@ -338,6 +339,7 @@ #define LANG_HOOKS_INITIALIZER { \ LANG_HOOKS_GIMPLIFY_EXPR, \ LANG_HOOKS_BUILTIN_FUNCTION, \ LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE, \ + LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL, \ LANG_HOOKS_INIT_TS, \ LANG_HOOKS_EXPR_TO_DECL, \ LANG_HOOKS_EH_PERSONALITY, \ Index: gcc/langhooks.c =================================================================== --- gcc/langhooks.c 2019-08-25 19:10:35.194159619 +0100 +++ gcc/langhooks.c 2019-09-26 13:02:42.543520465 +0100 @@ -599,19 +599,16 @@ lhd_omp_mappable_type (tree type) return true; } -/* Common function for add_builtin_function and - add_builtin_function_ext_scope. */ +/* Common function for add_builtin_function, add_builtin_function_ext_scope + and simulate_builtin_function_decl. */ + static tree -add_builtin_function_common (const char *name, - tree type, - int function_code, - enum built_in_class cl, - const char *library_name, - tree attrs, - tree (*hook) (tree)) +build_builtin_function (location_t location, const char *name, tree type, + int function_code, enum built_in_class cl, + const char *library_name, tree attrs) { tree id = get_identifier (name); - tree decl = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, id, type); + tree decl = build_decl (location, FUNCTION_DECL, id, type); TREE_PUBLIC (decl) = 1; DECL_EXTERNAL (decl) = 1; @@ -632,8 +629,7 @@ add_builtin_function_common (const char else decl_attributes (&decl, NULL_TREE, 0); - return hook (decl); - + return decl; } /* Create a builtin function. */ @@ -646,9 +642,9 @@ add_builtin_function (const char *name, const char *library_name, tree attrs) { - return add_builtin_function_common (name, type, function_code, cl, - library_name, attrs, - lang_hooks.builtin_function); + tree decl = build_builtin_function (BUILTINS_LOCATION, name, type, + function_code, cl, library_name, attrs); + return lang_hooks.builtin_function (decl); } /* Like add_builtin_function, but make sure the scope is the external scope. @@ -666,9 +662,40 @@ add_builtin_function_ext_scope (const ch const char *library_name, tree attrs) { - return add_builtin_function_common (name, type, function_code, cl, - library_name, attrs, - lang_hooks.builtin_function_ext_scope); + tree decl = build_builtin_function (BUILTINS_LOCATION, name, type, + function_code, cl, library_name, attrs); + return lang_hooks.builtin_function_ext_scope (decl); +} + +/* Simulate a declaration of a target-specific built-in function at + location LOCATION, as though it had been declared directly in the + source language. NAME is the name of the function, TYPE is its function + type, FUNCTION_CODE is the target-specific function code, LIBRARY_NAME + is the name of the underlying library function (NULL if none) and + ATTRS is a list of function attributes. + + Return the decl of the declared function. */ + +tree +simulate_builtin_function_decl (location_t location, const char *name, + tree type, int function_code, + const char *library_name, tree attrs) +{ + tree decl = build_builtin_function (location, name, type, + function_code, BUILT_IN_MD, + library_name, attrs); + tree new_decl = lang_hooks.simulate_builtin_function_decl (decl); + + /* Give the front end a chance to create a new decl if necessary, + but if the front end discards the decl in favour of a conflicting + (erroneous) previous definition, return the decl that we tried but + failed to add. This allows the caller to process the returned decl + normally, even though the source code won't be able to use it. */ + if (TREE_CODE (new_decl) == FUNCTION_DECL + && fndecl_built_in_p (new_decl, function_code, BUILT_IN_MD)) + return new_decl; + + return decl; } tree Index: gcc/c/c-tree.h =================================================================== --- gcc/c/c-tree.h 2019-07-10 19:41:20.543944894 +0100 +++ gcc/c/c-tree.h 2019-09-26 13:02:42.539520492 +0100 @@ -579,6 +579,7 @@ extern struct c_declarator *set_array_de struct c_declarator *); extern tree c_builtin_function (tree); extern tree c_builtin_function_ext_scope (tree); +extern tree c_simulate_builtin_function_decl (tree); extern void shadow_tag (const struct c_declspecs *); extern void shadow_tag_warned (const struct c_declspecs *, int); extern tree start_enum (location_t, struct c_enum_contents *, tree); Index: gcc/c/c-decl.c =================================================================== --- gcc/c/c-decl.c 2019-09-21 13:56:06.983948786 +0100 +++ gcc/c/c-decl.c 2019-09-26 13:02:42.539520492 +0100 @@ -4481,6 +4481,16 @@ c_builtin_function_ext_scope (tree decl) return decl; } + +/* Implement LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL. */ + +tree +c_simulate_builtin_function_decl (tree decl) +{ + tree type = TREE_TYPE (decl); + C_DECL_BUILTIN_PROTOTYPE (decl) = prototype_p (type); + return pushdecl (decl); +} /* Called when a declaration is seen that contains no names to declare. If its type is a reference to a structure, union or enum inherited Index: gcc/c/c-objc-common.h =================================================================== --- gcc/c/c-objc-common.h 2019-04-04 08:34:51.661939350 +0100 +++ gcc/c/c-objc-common.h 2019-09-26 13:02:42.539520492 +0100 @@ -60,6 +60,9 @@ #define LANG_HOOKS_MISSING_NORETURN_OK_P #define LANG_HOOKS_BUILTIN_FUNCTION c_builtin_function #undef LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE #define LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE c_builtin_function_ext_scope +#undef LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL +#define LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL \ + c_simulate_builtin_function_decl #undef LANG_HOOKS_EMITS_BEGIN_STMT #define LANG_HOOKS_EMITS_BEGIN_STMT true Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h 2019-09-26 08:38:00.759713359 +0100 +++ gcc/cp/cp-tree.h 2019-09-26 13:02:42.539520492 +0100 @@ -6461,6 +6461,7 @@ extern tmpl_spec_kind current_tmpl_spec_ extern tree cp_fname_init (const char *, tree *); extern tree cxx_builtin_function (tree decl); extern tree cxx_builtin_function_ext_scope (tree decl); +extern tree cxx_simulate_builtin_function_decl (tree); extern tree check_elaborated_type_specifier (enum tag_types, tree, bool); extern void warn_extern_redeclared_static (tree, tree); extern tree cxx_comdat_group (tree); Index: gcc/cp/decl.c =================================================================== --- gcc/cp/decl.c 2019-09-17 15:27:10.958069743 +0100 +++ gcc/cp/decl.c 2019-09-26 13:02:42.543520465 +0100 @@ -71,7 +71,6 @@ static tree grokvardecl (tree, tree, tre int, int, int, bool, int, tree, location_t); static void check_static_variable_definition (tree, tree); static void record_unknown_type (tree, const char *); -static tree builtin_function_1 (tree, tree, bool); static int member_function_or_else (tree, tree, enum overload_flags); static tree local_variable_p_walkfn (tree *, int *, void *); static const char *tag_name (enum tag_types); @@ -4567,8 +4566,12 @@ cp_make_fname_decl (location_t loc, tree return decl; } +/* IS_SIMULATED_DECL is true if the decl should simulate a global + declaration in the source language. */ + static tree -builtin_function_1 (tree decl, tree context, bool is_global) +builtin_function_1 (tree decl, tree context, bool is_global, + bool is_simulated_decl = false) { tree id = DECL_NAME (decl); const char *name = IDENTIFIER_POINTER (id); @@ -4576,7 +4579,10 @@ builtin_function_1 (tree decl, tree cont retrofit_lang_decl (decl); DECL_ARTIFICIAL (decl) = 1; - SET_DECL_LANGUAGE (decl, lang_c); + if (is_simulated_decl) + SET_DECL_LANGUAGE (decl, lang_cplusplus); + else + SET_DECL_LANGUAGE (decl, lang_c); /* Runtime library routines are, by definition, available in an external shared object. */ DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT; @@ -4584,21 +4590,24 @@ builtin_function_1 (tree decl, tree cont DECL_CONTEXT (decl) = context; - /* A function in the user's namespace should have an explicit - declaration before it is used. Mark the built-in function as - anticipated but not actually declared. */ - if (name[0] != '_' || name[1] != '_') - DECL_ANTICIPATED (decl) = 1; - else if (strncmp (name + 2, "builtin_", strlen ("builtin_")) != 0) - { - size_t len = strlen (name); - - /* Treat __*_chk fortification functions as anticipated as well, - unless they are __builtin_*. */ - if (len > strlen ("___chk") - && memcmp (name + len - strlen ("_chk"), - "_chk", strlen ("_chk") + 1) == 0) + if (!is_simulated_decl) + { + /* A function in the user's namespace should have an explicit + declaration before it is used. Mark the built-in function as + anticipated but not actually declared. */ + if (name[0] != '_' || name[1] != '_') DECL_ANTICIPATED (decl) = 1; + else if (strncmp (name + 2, "builtin_", strlen ("builtin_")) != 0) + { + size_t len = strlen (name); + + /* Treat __*_chk fortification functions as anticipated as well, + unless they are __builtin_*. */ + if (len > strlen ("___chk") + && memcmp (name + len - strlen ("_chk"), + "_chk", strlen ("_chk") + 1) == 0) + DECL_ANTICIPATED (decl) = 1; + } } if (is_global) @@ -4650,6 +4659,14 @@ cxx_builtin_function_ext_scope (tree dec return builtin_function_1 (decl, NULL_TREE, true); } +/* Implement LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL. */ + +tree +cxx_simulate_builtin_function_decl (tree decl) +{ + return builtin_function_1 (decl, NULL_TREE, true, true); +} + /* Generate a FUNCTION_DECL with the typical flags for a runtime library function. Not called directly. */ Index: gcc/cp/cp-objcp-common.h =================================================================== --- gcc/cp/cp-objcp-common.h 2019-03-08 18:15:25.000784821 +0000 +++ gcc/cp/cp-objcp-common.h 2019-09-26 13:02:42.539520492 +0100 @@ -100,6 +100,9 @@ #define LANG_HOOKS_POST_COMPILATION_PARS #define LANG_HOOKS_BUILTIN_FUNCTION cxx_builtin_function #undef LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE #define LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE cxx_builtin_function_ext_scope +#undef LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL +#define LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL \ + cxx_simulate_builtin_function_decl #undef LANG_HOOKS_TYPE_HASH_EQ #define LANG_HOOKS_TYPE_HASH_EQ cxx_type_hash_eq #undef LANG_HOOKS_COPY_LANG_QUALIFIERS
Although it's possible to define the SVE intrinsics in a normal header file, it's much more convenient to define them directly in the compiler. This also speeds up compilation and gives better error messages. The idea is therefore for arm_sve.h (the main intrinsics header file) to have the pragma: #pragma GCC aarch64 "arm_sve.h" telling GCC to define (almost) everything arm_sve.h needs to define. The target then needs a way of injecting new built-in function declarations during compilation. The main hook for defining built-in functions is add_builtin_function. This is designed for use at start-up, and so has various features that are correct in that context but not for the pragma above: (1) the location is always BUILTINS_LOCATION, whereas for arm_sve.h it ought to be the location of the pragma. (2) the function is only immediately visible if it's in the implementation namespace, whereas the pragma is deliberately injecting functions into the general namespace. (3) there's no attempt to emulate a normal function declaration in C or C++, whereas functions declared by the pragma should be checked in the same way as an open-coded declaration would be. E.g. we should get an error if there was a previous incompatible declaration. (4) in C++, the function is treated as extern "C" and so can't be overloaded, whereas SVE intrinsics do use function overloading. This patch therefore adds a hook that targets can use to inject the equivalent of a source-level function declaration, but bound to a BUILT_IN_MD function. The main SVE intrinsic patch has tests to make sure that we report an error for conflicting definitions that appear either before or after including arm_sve.h. Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Richard 2019-09-26 Richard Sandiford <richard.sandiford@arm.com> gcc/ * langhooks.h (lang_hooks::simulate_builtin_function_decl): New hook. (simulate_builtin_function_decl): Declare. * langhooks-def.h (LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL): Define. (LANG_HOOKS_INITIALIZER): Include it. * langhooks.c (add_builtin_function_common): Rename to... (build_builtin_function): ...this. Add a location parameter and use it instead of BUILTINS_LOCATION. Remove the hook parameter and return the decl instead. (add_builtin_function): Update accordingly, passing the returned decl to the lang hook. (add_builtin_function_ext_scope): Likewise (simulate_builtin_function_decl): New function. gcc/c/ * c-tree.h (c_simulate_builtin_function_decl): Declare. * c-decl.c (c_simulate_builtin_function_decl): New function. * c-objc-common.h (LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL): Define to the above. gcc/cp/ * cp-tree.h (cxx_simulate_builtin_function_decl): Declare. * decl.c (builtin_function_1): Add an is_simulated_source_decl parameter. When true, treat the function as a C++ function rather than as extern "C", and don't treat it as anticipated. (cxx_simulate_builtin_function_decl): New function. * cp-objcp-common.h (LANG_HOOKS_SIMULATE_BUILTIN_FUNCTION_DECL): Define to the above.