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

[Andrew Pinski <pinskia@gmail.com>]

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.

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.

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.  */
>
>
>
>