Re: [PATCH] c++: parser - Support for target address spaces in C++
On 10/9/22 12:12, Paul Iannetta wrote: Hi, On Thu, Oct 06, 2022 at 01:34:40PM -0400, Jason Merrill wrote: [snip] Hmm? We mangle __restrict: void f(int *__restrict *p) { } // _Z1fPrPi Indeed, I have overlooked that point. Thank you for pointing it out. but cv-qualifiers (including restrict) are dropped on the top-level type of a parameter. If address spaces should be mangled, it probably makes sense to use the same mangling that we already use for attributes in write_CV_qualifiers_for_type. That's a nice feature! LLVM is using AS to mangle the address-name qualified types but that would have required an update of libiberty's demangler in the binutils as well. And they haven't proposed this mangling to https://github.com/itanium-cxx-abi/cxx-abi/ yet, either. Following your advice, I implemented the mangling of address_names as U__address_name (eg., U8__seg_fs), underscores may appear but that does not seem to be a problem. I don't think that there should be anything to do on the template side, but I am no expert of what is expected of templates when it comes to strip or forward qualifiers. So, please tell me if I overlooked something on that side. You certainly want some template tests, say template int f (T *p) { return *p; } __seg_fs int *a; int main() { f(a); } // test for mangling of f<__seg_fs int> - template int f (T __seg_gs *p) { return *p; } __seg_fs int *a; int main() { f(a); } // error, conflicting address spaces Now concerning the meat of the patch itself. The C part of the support of address spaces does not appear to have dedicated tests, especially since it is mostly a target specific feature. I, however, added a few tests, one for the mangling, and others which test the different error messages that might appear when using address spaces in an unsafe way. Most of the patch tries to mirror the commit 309434d678721acedb299f20cdc2b4778062b180 which introduces address spaces to the C front-end. The C and C++ parsers share a lot but are subtly different so there might be some discrepancies. In particular, I ensure that address spaces do not conflict, that only one address space can be specified at a time, and that the type conversion mechanism pick the superset of the address spaces if available. I also forbid address spaces in compound literals. I have only tested the patch on x86 and on our not-yet-upstream architecture, without seeing any regressions. NB: I do not add a DSO for the moment because I have started the process of assigning copyright to the FSF. # >8 Add support for custom address spaces in C++ gcc/ * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses. gcc/c/ * c-decl.cc: Remove c_register_addr_space. gcc/c-family/ * c-common.cc (c_register_addr_space): Imported from c-decl.cc (addr_space_superset): Imported from gcc/c/c-typecheck.cc * c-common.h: Remove the FIXME. (addr_space_superset): Add prototype. gcc/cp/ * cp-tree.h (enum cp_decl_spec): Add addr_space support. (struct cp_decl_specifier_seq): Likewise. * decl.cc (get_type_quals): Likewise. (check_tag_decl): Likewise. * parser.cc (cp_parser_type_specifier): Likewise. (cp_parser_cv_qualifier_seq_opt): Likewise. (cp_parser_postfix_expression): Likewise. (cp_parser_type_specifier): Likewise. (set_and_check_decl_spec_loc): Likewise. * typeck.cc (composite_pointer_type): Likewise (comp_ptr_ttypes_real): Likewise. * tree.cc: Remove c_register_addr_space stub. * mangle.cc (write_CV_qualifiers_for_type): Mangle address spaces using the extended qualifier notation. gcc/doc * extend.texi (Named Address Spaces): add a mention about C++ support. gcc/testsuite/ * g++.dg/abi/mangle-addr-space1.C: New test. * g++.dg/parse/addr-space.C: New test. * g++.dg/parse/addr-space1.C: New test. * g++.dg/parse/addr-space2.C: New test. # >8 diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index bb0544eeaea..ff1146ecc25 100644 --- a/gcc/c-family/c-common.cc +++ b/gcc/c-family/c-common.cc @@ -615,6 +615,33 @@ c_addr_space_name (addr_space_t as) return IDENTIFIER_POINTER (ridpointers [rid]); } +/* Return true if between two named address spaces, whether there is a superset + named address space that encompasses both address spaces. If there is a + superset, return which address space is the superset. */ + +bool +addr_space_superset (addr_space_t as1, addr_space_t as2, +addr_space_t * common) +{ + if (as1 == as2) +{ + *common = as1; + return true; +} + else if (targetm.addr_space.subset_p (as1, as2)) +{ + *common = as2; + return true; +} + else if (targetm.addr_space.
Re: [PATCH] c++: parser - Support for target address spaces in C++
Hi, On Thu, Oct 06, 2022 at 01:34:40PM -0400, Jason Merrill wrote: [snip] > > Hmm? We mangle __restrict: > > void f(int *__restrict *p) { } // _Z1fPrPi > Indeed, I have overlooked that point. Thank you for pointing it out. > but cv-qualifiers (including restrict) are dropped on the top-level type of > a parameter. > > If address spaces should be mangled, it probably makes sense to use the same > mangling that we already use for attributes in > write_CV_qualifiers_for_type. > That's a nice feature! LLVM is using AS to mangle the address-name qualified types but that would have required an update of libiberty's demangler in the binutils as well. Following your advice, I implemented the mangling of address_names as U__address_name (eg., U8__seg_fs), underscores may appear but that does not seem to be a problem. I don't think that there should be anything to do on the template side, but I am no expert of what is expected of templates when it comes to strip or forward qualifiers. So, please tell me if I overlooked something on that side. Now concerning the meat of the patch itself. The C part of the support of address spaces does not appear to have dedicated tests, especially since it is mostly a target specific feature. I, however, added a few tests, one for the mangling, and others which test the different error messages that might appear when using address spaces in an unsafe way. Most of the patch tries to mirror the commit 309434d678721acedb299f20cdc2b4778062b180 which introduces address spaces to the C front-end. The C and C++ parsers share a lot but are subtly different so there might be some discrepancies. In particular, I ensure that address spaces do not conflict, that only one address space can be specified at a time, and that the type conversion mechanism pick the superset of the address spaces if available. I also forbid address spaces in compound literals. I have only tested the patch on x86 and on our not-yet-upstream architecture, without seeing any regressions. NB: I do not add a DSO for the moment because I have started the process of assigning copyright to the FSF. # >8 Add support for custom address spaces in C++ gcc/ * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses. gcc/c/ * c-decl.cc: Remove c_register_addr_space. gcc/c-family/ * c-common.cc (c_register_addr_space): Imported from c-decl.cc (addr_space_superset): Imported from gcc/c/c-typecheck.cc * c-common.h: Remove the FIXME. (addr_space_superset): Add prototype. gcc/cp/ * cp-tree.h (enum cp_decl_spec): Add addr_space support. (struct cp_decl_specifier_seq): Likewise. * decl.cc (get_type_quals): Likewise. (check_tag_decl): Likewise. * parser.cc (cp_parser_type_specifier): Likewise. (cp_parser_cv_qualifier_seq_opt): Likewise. (cp_parser_postfix_expression): Likewise. (cp_parser_type_specifier): Likewise. (set_and_check_decl_spec_loc): Likewise. * typeck.cc (composite_pointer_type): Likewise (comp_ptr_ttypes_real): Likewise. * tree.cc: Remove c_register_addr_space stub. * mangle.cc (write_CV_qualifiers_for_type): Mangle address spaces using the extended qualifier notation. gcc/doc * extend.texi (Named Address Spaces): add a mention about C++ support. gcc/testsuite/ * g++.dg/abi/mangle-addr-space1.C: New test. * g++.dg/parse/addr-space.C: New test. * g++.dg/parse/addr-space1.C: New test. * g++.dg/parse/addr-space2.C: New test. # >8 diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index bb0544eeaea..ff1146ecc25 100644 --- a/gcc/c-family/c-common.cc +++ b/gcc/c-family/c-common.cc @@ -615,6 +615,33 @@ c_addr_space_name (addr_space_t as) return IDENTIFIER_POINTER (ridpointers [rid]); } +/* Return true if between two named address spaces, whether there is a superset + named address space that encompasses both address spaces. If there is a + superset, return which address space is the superset. */ + +bool +addr_space_superset (addr_space_t as1, addr_space_t as2, +addr_space_t * common) +{ + if (as1 == as2) +{ + *common = as1; + return true; +} + else if (targetm.addr_space.subset_p (as1, as2)) +{ + *common = as2; + return true; +} + else if (targetm.addr_space.subset_p (as2, as1)) +{ + *common = as1; + return true; +} + else +return false; +} + /* Push current bindings for the function name VAR_DECLS. */ void @@ -2809,6 +2836,25 @@ c_build_bitfield_integer_type (unsigned HOST_WIDE_INT width, int unsignedp) return build_nonstandard_integer_type (width, unsignedp); } +/* Register reserved keyword WORD as qualifier for address space AS. */ + +void +c_register_addr_space (c