Re: [PATCH] c++: parser - Support for target address spaces in C++

2022-10-10 Thread Jason Merrill via Gcc-patches

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

2022-10-09 Thread Paul Iannetta via Gcc-patches
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