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

[Paul Iannetta <piannetta@kalrayinc.com>]

On Thu, Oct 06, 2022 at 09:52:45AM -0700, Andrew Pinski wrote:
> On Thu, Oct 6, 2022 at 7:15 AM Paul Iannetta via Gcc <gcc@gcc.gnu.org> wrote:
> >
> > Hi,
> >
> > Presently, GCC supports target address spaces as a GNU extension (cf.
> > `info -n "(gcc)Named Address Spaces"`).  This is however supported
> > only by the C frontend, which is a bit sad, since all the GIMPLE
> > machinery is readily available and, moreover, LLVM supports this GNU
> > extension both for C and C++.
> >
> > Here is a first attempt at a patch to enable the support for target
> > address spaces in C++.  The basic idea is only to make the parser
> > recognize address spaces, and lower them into GIMPLE, in the same
> > fashion as the C parser.  This also makes it possible to merge the
> > function `c_register_addr_space` in one place which is better than
> > before.
> >
> > The patch still has some drawbacks compared to its C counterpart.
> > Namely, much like the `__restrict__` keyword, it is always enabled and
> > -std=c++11 won't disable it (whereas -std=c11) would reject address
> > spaces.  Not also that the mangler ignores address spaces, much like
> > it ignores __restrict__.
> 
> Without the mangler support or overloading support, there would will
> be wrong code emitted as some named address spaces don't overlap with
> the unnamed ones.
> E.g. on SPU (which has been since removed), the name address support
> was added to support automatically DMAing from the PPU memory on the
> cell. So two address spaces are disjoint.
> This is also true on other targets too.
> This is biggest reason why this was not added to the C++ front-end.
> If clang supports it, does it do overload/template mangling support. I
> think we don't want a partial support added which will just break
> under simple usage.
> 

I don't know to what extent clang supports their usage in templates but it
does support proper overloading/name mangling. I'll add the currently
missing mangling support, and how it behaves with templates.

> Also note named address support is not exactly a GNU specific
> extension either;
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1275.pdf has the
> definition of it for C and GCC tries to follow that.
> Fixed types support in GCC also follows that draft.
> 

Good to know!

Thank,
Paul

> Thanks,
> Andrew Pinski
> 
> 
> >
> > Depending on the reception, I'll add further testcases and will update
> > the relevant part of the documentation.
> >
> > Cheers,
> > Paul
> >
> > Author: Paul Iannetta <piannetta@kalray.eu>
> > Date:   Wed Oct 5 16:44:36 2022 +0200
> >
> >     Add support for custom address spaces in C++
> >
> > gcc/
> >         * tree.h (ENCODE_QUAL_ADDR_SPACE): Missing parentheses.
> >
> > gcc/c/
> >         * c-decl.c: Remove c_register_addr_space.
> >
> > gcc/c-family/
> >         * c-common.c (c_register_addr_space): Imported from c-decl.c
> >         * c-common.h: Remove the FIXME.
> >
> > gcc/cp/
> >         * cp-tree.h (enum cp_decl_spec): Add addr_space support.
> >             (struct cp_decl_specifier_seq): Likewise.
> >         * decl.c (get_type_quals): Likewise.
> >         * parser.c (cp_parser_type_specifier): Likewise.
> >             (cp_parser_cv_qualifier_seq_opt): Likewise
> >         * tree.c: Remove c_register_addr_space stub.
> >
> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > index 064c2f263f0..282ba54ab70 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -2615,6 +2615,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 (const char *word, addr_space_t as)
> > +{
> > +  int rid = RID_FIRST_ADDR_SPACE + as;
> > +  tree id;
> > +
> > +  /* Address space qualifiers are only supported
> > +     in C with GNU extensions enabled.  */
> > +  if (c_dialect_objc () || flag_no_asm)
> > +    return;
> > +
> > +  id = get_identifier (word);
> > +  C_SET_RID_CODE (id, rid);
> > +  TREE_LANG_FLAG_0 (id) = 1;
> > +  ridpointers[rid] = id;
> > +}
> > +
> >  /* The C version of the register_builtin_type langhook.  */
> >
> >  void
> > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > index ed39b7764bf..f2c1df0c8de 100644
> > --- a/gcc/c-family/c-common.h
> > +++ b/gcc/c-family/c-common.h
> > @@ -808,9 +808,6 @@ extern const struct attribute_spec c_common_format_attribute_table[];
> >
> >  extern tree (*make_fname_decl) (location_t, tree, int);
> >
> > -/* In c-decl.c and cp/tree.c.  FIXME.  */
> > -extern void c_register_addr_space (const char *str, addr_space_t as);
> > -
> >  /* In c-common.c.  */
> >  extern bool in_late_binary_op;
> >  extern const char *c_addr_space_name (addr_space_t as);
> > @@ -926,6 +923,7 @@ extern void c_common_finish (void);
> >  extern void c_common_parse_file (void);
> >  extern FILE *get_dump_info (int, dump_flags_t *);
> >  extern alias_set_type c_common_get_alias_set (tree);
> > +extern void c_register_addr_space (const char *, addr_space_t);
> >  extern void c_register_builtin_type (tree, const char*);
> >  extern bool c_promoting_integer_type_p (const_tree);
> >  extern bool self_promoting_args_p (const_tree);
> > diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> > index 8e24b522ee4..278d1248d1c 100644
> > --- a/gcc/c/c-decl.c
> > +++ b/gcc/c/c-decl.c
> > @@ -11927,25 +11927,6 @@ c_parse_final_cleanups (void)
> >    ext_block = NULL;
> >  }
> >
> > -/* Register reserved keyword WORD as qualifier for address space AS.  */
> > -
> > -void
> > -c_register_addr_space (const char *word, addr_space_t as)
> > -{
> > -  int rid = RID_FIRST_ADDR_SPACE + as;
> > -  tree id;
> > -
> > -  /* Address space qualifiers are only supported
> > -     in C with GNU extensions enabled.  */
> > -  if (c_dialect_objc () || flag_no_asm)
> > -    return;
> > -
> > -  id = get_identifier (word);
> > -  C_SET_RID_CODE (id, rid);
> > -  C_IS_RESERVED_WORD (id) = 1;
> > -  ridpointers [rid] = id;
> > -}
> > -
> >  /* Return identifier to look up for omp declare reduction.  */
> >
> >  tree
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 15ec4cd199f..4ae971ccb90 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -5974,6 +5974,7 @@ enum cp_decl_spec {
> >    ds_const,
> >    ds_volatile,
> >    ds_restrict,
> > +  ds_addr_space,
> >    ds_inline,
> >    ds_virtual,
> >    ds_explicit,
> > @@ -6046,6 +6047,8 @@ struct cp_decl_specifier_seq {
> >    /* True iff the alternate "__intN__" form of the __intN type has been
> >       used.  */
> >    BOOL_BITFIELD int_n_alt: 1;
> > +  /* The address space that the declaration belongs to.  */
> > +  addr_space_t address_space;
> >  };
> >
> >  /* The various kinds of declarators.  */
> > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > index 03cd0a3a238..b8fc8b58ac7 100644
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -4954,6 +4954,8 @@ get_type_quals (const cp_decl_specifier_seq *declspecs)
> >      type_quals |= TYPE_QUAL_VOLATILE;
> >    if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
> >      type_quals |= TYPE_QUAL_RESTRICT;
> > +  if (decl_spec_seq_has_spec_p (declspecs, ds_addr_space))
> > +    type_quals |= ENCODE_QUAL_ADDR_SPACE (declspecs->address_space);
> >
> >    return type_quals;
> >  }
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index f48c856fa94..8d6b2a44086 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -17796,6 +17796,16 @@ cp_parser_type_specifier (cp_parser* parser,
> >        break;
> >      }
> >
> > +
> > +  if (RID_FIRST_ADDR_SPACE <= keyword && keyword <= RID_LAST_ADDR_SPACE)
> > +    {
> > +      ds = ds_addr_space;
> > +      if (is_cv_qualifier)
> > +       *is_cv_qualifier = true;
> > +      decl_specs->address_space = keyword - RID_FIRST_ADDR_SPACE;
> > +    }
> > +
> > +
> >    /* Handle simple keywords.  */
> >    if (ds != ds_last)
> >      {
> > @@ -21853,6 +21863,7 @@ cp_parser_ptr_operator (cp_parser* parser,
> >     GNU Extension:
> >
> >     cv-qualifier:
> > +     address-space-qualifier
> >       __restrict__
> >
> >     Returns a bitmask representing the cv-qualifiers.  */
> > @@ -21889,6 +21900,13 @@ cp_parser_cv_qualifier_seq_opt (cp_parser* parser)
> >           break;
> >         }
> >
> > +      if (RID_FIRST_ADDR_SPACE <= token->keyword &&
> > +         token->keyword <= RID_LAST_ADDR_SPACE)
> > +       {
> > +         cv_qualifier =
> > +           ENCODE_QUAL_ADDR_SPACE (token->keyword - RID_FIRST_ADDR_SPACE);
> > +       }
> > +
> >        if (!cv_qualifier)
> >         break;
> >
> > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> > index 10b818d1370..55b57085618 100644
> > --- a/gcc/cp/tree.c
> > +++ b/gcc/cp/tree.c
> > @@ -5749,15 +5749,6 @@ cp_free_lang_data (tree t)
> >      DECL_CHAIN (t) = NULL_TREE;
> >  }
> >
> > -/* Stub for c-common.  Please keep in sync with c-decl.c.
> > -   FIXME: If address space support is target specific, then this
> > -   should be a C target hook.  But currently this is not possible,
> > -   because this function is called via REGISTER_TARGET_PRAGMAS.  */
> > -void
> > -c_register_addr_space (const char * /*word*/, addr_space_t /*as*/)
> > -{
> > -}
> > -
> >  /* Return the number of operands in T that we care about for things like
> >     mangling.  */
> >
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index bb80e81d389..210ef7b85d7 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -2108,7 +2108,7 @@ extern machine_mode vector_type_mode (const_tree);
> >
> >  /* Encode/decode the named memory support as part of the qualifier.  If more
> >     than 8 qualifiers are added, these macros need to be adjusted.  */
> > -#define ENCODE_QUAL_ADDR_SPACE(NUM) ((NUM & 0xFF) << 8)
> > +#define ENCODE_QUAL_ADDR_SPACE(NUM) (((NUM) & 0xFF) << 8)
> >  #define DECODE_QUAL_ADDR_SPACE(X) (((X) >> 8) & 0xFF)
> >
> >  /* Return all qualifiers except for the address space qualifiers.  */
> >
> >
> >
> >
> 
> 
> 
>