Thank you very much for the detailed review.
On Tue, Oct 18, 2022 at 10:24:23AM -0400, Jason Merrill wrote:
> On 10/18/22 03:37, Paul Iannetta wrote:
> > On Fri, Oct 14, 2022 at 11:19:50AM -0400, Jason Merrill wrote:
> > > On 10/13/22 17:57, Paul Iannetta wrote:
> > > > On Thu, Oct 13, 2022 at 03:41:16PM -0400, Jason Merrill wrote:
> > > > > On 10/13/22 12:02, Paul Iannetta wrote:
> > > > > > On Thu, Oct 13, 2022 at 11:47:42AM -0400, Jason Merrill wrote:
> > > > > > > On 10/13/22 11:23, Paul Iannetta wrote:
> > > > > > > > On Thu, Oct 13, 2022 at 11:02:24AM -0400, Jason Merrill wrote:
> > > > > > > > > On 10/12/22 20:52, Paul Iannetta wrote:
> > > > > > > > > > There are quite a few things I would like to clarify
> > > > > > > > > > concerning some
> > > > > > > > > > implementation details.
> > > > > > > > > > - A variable with automatic storage (which is
> > > > > > > > > > neither a pointer nor
> > > > > > > > > > a reference) cannot be qualified with an address
> > > > > > > > > > space. I detect
> > > > > > > > > > this by the combination of `sc_none' and `!
> > > > > > > > > > toplevel_bindings_p ()',
> > > > > > > > > > but I've also seen the use of `at_function_scope'
> > > > > > > > > > at other places.
> > > > > > > > > > And I'm unsure which one is appropriate here.
> > > > > > > > > > This detection happens at the very end of
> > > > > > > > > > grokdeclarator because I
> > > > > > > > > > need to know that the type is a pointer, which is
> > > > > > > > > > not know until
> > > > > > > > > > very late in the function.
> > > > > > > > >
> > > > > > > > > At that point you have the decl, and you can ask directly
> > > > > > > > > what its storage
> > > > > > > > > duration is, perhaps using decl_storage_duration.
> > > > > > > > >
> > > > > > > > > But why do you need to know whether the type is a pointer?
> > > > > > > > > The attribute
> > > > > > > > > applies to the target type of the pointer, not the pointer
> > > > > > > > > type. I think
> > > > > > > > > the problem is that you're looking at declspecs when you
> > > > > > > > > ought to be looking
> > > > > > > > > at type_quals.
> > > > > > > >
> > > > > > > > I need to know that the base type is a pointer to reject invalid
> > > > > > > > declarations such as:
> > > > > > > >
> > > > > > > > int f (__seg_fs int a) { } or int f () {
> > > > > > > > __seg_fs int a; }
> > > > > > > >
> > > > > > > > because parameters and auto variables can have an address space
> > > > > > > > qualifier only if they are pointer or reference type, which I
> > > > > > > > can't
> > > > > > > > tell only from type_quals.
> > > > > > >
> > > > > > > But "int *__seg_fs a" is just as invalid as the above; the
> > > > > > > difference is not
> > > > > > > whether a is a pointer, but whether the address-space-qualified
> > > > > > > is the type
> > > > > > > of a itself or some sub-type.
> > > > > >
> > > > > > I agree that "int * __seg_fs a" is invalid but it is accepted by
> > > > > > the C
> > > > > > front-end, and by clang (both C and C++), the behavior is that the
> > > > > > address-name is silently ignored.
> > > > >
> > > > > Hmm, that sounds like a bug; in that case, presumably the user meant
> > > > > to
> > > > > qualify the pointed-to type, and silently ignoring seems unlikely to
> > > > > give
> > > > > the effect they want.
> > > > >
> > > >
> > > > Well, actually, I'm re-reading the draft and "int * __seg_fs a" is
> > > > valid. It means "pointer in address space __seg_fs pointing to an
> > > > object in the generic address space", whereas "__seg_fs int * a" means
> > > > "pointer in the generic address space pointing to an object in the
> > > > __seg_fs address-space".
> > > >
> > > > Oddities such as, "__seg_fs int * __seg_gs a" are also perfectly
> > > > valid.
> > >
> > > If a has static storage duration, sure; I was still thinking about
> > > declarations with automatic storage duration such as in your example
> > > above.
> > >
> >
> > Thanks, I only use type_quals now. I also took into account the style
> > recommendations from Jakub, and included the other template tests.
> > I rebased over trunk, bootstrapped the compiler and run the "make
> > check-gcc" with no regressions on x86.
> >
> > Paul
> >
> > # ------------------------ >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): New declaration.
> >
> > 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.
> > (grokdeclarator): 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.
> > (same_type_ignoring_top_level_qualifiers_p): Likewise.
> > * pt.cc (check_cv_quals_for_unify): Likewise.
> > (unify): 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/abi/mangle-addr-space2.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.
> > * g++.dg/parse/template/spec-addr-space.C: New test.
> > * g++.dg/ext/addr-space-decl.C: New test.
> > * g++.dg/ext/addr-space-ref.C: New test.
> > * g++.dg/ext/addr-space-ops.C: New test.
> > * g++.dg/template/addr-space-overload.C: New test.
> > * g++.dg/template/addr-space-strip1.C: New test.
> > * g++.dg/template/addr-space-strip2.C: New test.
> >
> > # ------------------------ >8 ------------------------
> > diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> > index 9ec9100cc90..3b79dc47515 100644
> > --- a/gcc/c-family/c-common.cc
> > +++ b/gcc/c-family/c-common.cc
> > @@ -588,6 +588,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
> > @@ -2785,6 +2812,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 62ab4ba437b..a3864d874aa 100644
> > --- a/gcc/c-family/c-common.h
> > +++ b/gcc/c-family/c-common.h
> > @@ -829,12 +829,11 @@ extern const struct attribute_spec
> > c_common_format_attribute_table[];
> > extern tree (*make_fname_decl) (location_t, tree, int);
> > -/* In c-decl.cc and cp/tree.cc. FIXME. */
> > -extern void c_register_addr_space (const char *str, addr_space_t as);
> > -
> > /* In c-common.cc. */
> > extern bool in_late_binary_op;
> > extern const char *c_addr_space_name (addr_space_t as);
> > +extern const char *c_addr_space_name (addr_space_t as);
> > +extern bool addr_space_superset (addr_space_t, addr_space_t, addr_space_t
> > *);
> > extern tree identifier_global_value (tree);
> > extern tree identifier_global_tag (tree);
> > extern bool names_builtin_p (const char *);
> > @@ -951,6 +950,7 @@ extern bool c_common_init (void);
> > extern void c_common_finish (void);
> > extern void c_common_parse_file (void);
> > 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.cc b/gcc/c/c-decl.cc
> > index a7571cc7542..b1f69997ff7 100644
> > --- a/gcc/c/c-decl.cc
> > +++ b/gcc/c/c-decl.cc
> > @@ -12531,25 +12531,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/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > index fdb96c28c51..2a700bbaff3 100644
> > --- a/gcc/c/c-typeck.cc
> > +++ b/gcc/c/c-typeck.cc
> > @@ -303,32 +303,6 @@ c_type_promotes_to (tree type)
> > return type;
> > }
> > -/* 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. */
> > -
> > -static 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;
> > -}
> > -
> > /* Return a variant of TYPE which has all the type qualifiers of LIKE
> > as well as those of TYPE. */
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index e2607f09c19..0248569a95b 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -6235,6 +6235,7 @@ enum cp_decl_spec {
> > ds_const,
> > ds_volatile,
> > ds_restrict,
> > + ds_addr_space,
> > ds_inline,
> > ds_virtual,
> > ds_explicit,
> > @@ -6281,6 +6282,8 @@ struct cp_decl_specifier_seq {
> > cp_storage_class storage_class;
> > /* For the __intN declspec, this stores the index into the int_n_*
> > arrays. */
> > int int_n_idx;
> > + /* The address space that the declaration belongs to. */
> > + addr_space_t address_space;
> > /* True iff TYPE_SPEC defines a class or enum. */
> > BOOL_BITFIELD type_definition_p : 1;
> > /* True iff multiple types were (erroneously) specified for this
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 85b892cddf0..a87fed04529 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -5290,6 +5290,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;
> > }
> > @@ -5412,6 +5414,10 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
> > error_at (declspecs->locations[ds_restrict],
> > "%<__restrict%> can only be specified for objects and "
> > "functions");
> > + else if (decl_spec_seq_has_spec_p (declspecs, ds_addr_space))
> > + error_at (declspecs->locations[ds_addr_space],
> > + "address space can only be specified for objects and "
> > + "functions");
> > else if (decl_spec_seq_has_spec_p (declspecs, ds_thread))
> > error_at (declspecs->locations[ds_thread],
> > "%<__thread%> can only be specified for objects "
> > @@ -14608,6 +14614,59 @@ grokdeclarator (const cp_declarator *declarator,
> > if (!processing_template_decl)
> > cp_apply_type_quals_to_decl (type_quals, decl);
> > + /* Warn about address space used for things other than static memory or
> > + pointers. */
> > + addr_space_t address_space = DECODE_QUAL_ADDR_SPACE (type_quals);
> > + if (!ADDR_SPACE_GENERIC_P (address_space))
> > + {
> > + if (decl_context == NORMAL)
> > + {
> > + switch (storage_class)
>
> I would still suggest checking decl_storage_duration at this point rather
> than the storage_class specifier.
Unless I misunderstand something, I can't weed out register variables
if I rely on decl_storage_duration.
> > + {
> > + case sc_auto:
> > + error ("%qs combined with C++98 %<auto%> qualifier for %qs",
> > + c_addr_space_name (address_space), name);
> > + break;
> > + case sc_register:
> > + error ("%qs combined with %<register%> qualifier for %qs",
> > + c_addr_space_name (address_space), name);
> > + break;
> > + case sc_none:
> > + if (! toplevel_bindings_p ())
> > + error ("%qs specified for auto variable %qs",
>
> And let's refer to automatic storage duration rather than shorten to 'auto'.
>
Right.
> > + c_addr_space_name (address_space), name);
> > + break;
> > + case sc_mutable:
> > + error ("%qs combined with %<mutable%> qualifier for %qs",
> > + c_addr_space_name (address_space), name);
> > + break;
> > + case sc_static:
> > + case sc_extern:
> > + break;
> > + default:
> > + gcc_unreachable ();
> > + }
> > + }
> > + else if (decl_context == PARM && TREE_CODE (type) != ARRAY_TYPE)
> > + {
> > + if (name)
> > + error ("%qs specified for parameter %qs",
> > + c_addr_space_name (address_space), name);
> > + else
> > + error ("%qs specified for unnamed parameter",
> > + c_addr_space_name (address_space));
> > + }
> > + else if (decl_context == FIELD)
> > + {
> > + if (name)
> > + error ("%qs specified for structure field %qs",
> > + c_addr_space_name (address_space), name);
> > + else
> > + error ("%qs specified for structure field",
> > + c_addr_space_name (address_space));
> > + }
> > + }
> > +
> > return decl;
> > }
> > }
> > diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> > index 1215463089b..aafff98f05a 100644
> > --- a/gcc/cp/mangle.cc
> > +++ b/gcc/cp/mangle.cc
> > @@ -2520,6 +2520,14 @@ write_CV_qualifiers_for_type (const tree type)
> > array. */
> > cp_cv_quals quals = TYPE_QUALS (type);
> > + if (addr_space_t as = DECODE_QUAL_ADDR_SPACE (quals))
> > + {
> > + const char *as_name = c_addr_space_name (as);
> > + write_char ('U');
> > + write_unsigned_number (strlen (as_name));
> > + write_string (as_name);
> > + ++num_qualifiers;
> > + }
> > if (quals & TYPE_QUAL_RESTRICT)
> > {
> > write_char ('r');
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index 9ddfb027ff9..c82059d1efd 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -7703,6 +7703,15 @@ cp_parser_postfix_expression (cp_parser *parser,
> > bool address_p, bool cast_p,
> > postfix_expression = error_mark_node;
> > break;
> > }
> > + if (type != error_mark_node
> > + && !ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (type))
> > + && current_function_decl)
> > + {
> > + error
> > + ("compound literal qualified by address-space "
> > + "qualifier");
> > + type = error_mark_node;
> > + }
> > /* Form the representation of the compound-literal. */
> > postfix_expression
> > = finish_compound_literal (type, initializer,
> > @@ -19445,6 +19454,15 @@ 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;
> > + }
> > +
> > +
>
> I don't think we need two blank lines before and after this block, one each
> should be enough.
>
Indeed.
> > /* Handle simple keywords. */
> > if (ds != ds_last)
> > {
> > @@ -23837,6 +23855,7 @@ cp_parser_ptr_operator (cp_parser* parser,
> > GNU Extension:
> > cv-qualifier:
> > + address-space-qualifier
> > __restrict__
> > Returns a bitmask representing the cv-qualifiers. */
> > @@ -23873,6 +23892,11 @@ 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;
> > @@ -32893,6 +32917,8 @@ set_and_check_decl_spec_loc (cp_decl_specifier_seq
> > *decl_specs,
> > decl_specs->locations[ds] = location;
> > if (ds == ds_thread)
> > decl_specs->gnu_thread_keyword_p = token_is__thread (token);
> > + else if (ds == ds_addr_space)
> > + decl_specs->address_space = token->keyword - RID_FIRST_ADDR_SPACE;
> > }
> > else
> > {
> > @@ -32925,6 +32951,25 @@ set_and_check_decl_spec_loc (cp_decl_specifier_seq
> > *decl_specs,
> > error_at (&richloc, "duplicate %qD", token->u.value);
> > }
> > }
> > + else if (ds == ds_addr_space)
> > + {
> > + addr_space_t as1 = decl_specs->address_space;
> > + addr_space_t as2 = token->keyword - RID_FIRST_ADDR_SPACE;
> > +
> > + gcc_rich_location richloc (location);
> > + richloc.add_fixit_remove ();
> > + if (!ADDR_SPACE_GENERIC_P (as1) && !ADDR_SPACE_GENERIC_P (as2)
> > + && as1 != as2)
> > + error_at (&richloc,
> > + "conflicting named address spaces (%s vs %s)",
> > + c_addr_space_name (as1), c_addr_space_name (as2));
> > + if (as1 == as2 && !ADDR_SPACE_GENERIC_P (as1))
> > + error_at (&richloc,
> > + "duplicate named address space %s",
> > + c_addr_space_name (as1));
> > +
> > + decl_specs->address_space = as2;
> > + }
> > else
> > {
> > static const char *const decl_spec_names[] = {
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index e4dca9d4f9d..7b73a57091e 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -23778,8 +23778,19 @@ template_decl_level (tree decl)
> > static int
> > check_cv_quals_for_unify (int strict, tree arg, tree parm)
> > {
> > - int arg_quals = cp_type_quals (arg);
> > - int parm_quals = cp_type_quals (parm);
> > + int arg_quals = CLEAR_QUAL_ADDR_SPACE (cp_type_quals (arg));
> > + int parm_quals = CLEAR_QUAL_ADDR_SPACE (cp_type_quals (parm));
> > +
> > + /* Try to unify ARG's address space into PARM's address space.
> > + If PARM does not have any address space qualifiers (ie., as_parm is
> > 0),
> > + there are no constraints on address spaces for this type. */
> > + addr_space_t as_arg = DECODE_QUAL_ADDR_SPACE (cp_type_quals (arg));
> > + addr_space_t as_parm = DECODE_QUAL_ADDR_SPACE (cp_type_quals (parm));
> > + addr_space_t as_common;
> > + addr_space_superset (as_arg, as_parm, &as_common);
> > +
> > + if (!(as_parm == as_common || as_parm == 0))
> > + return 0;
>
> I'd expect address space qualifiers to follow the 'strict' parameter like
> the other qualifiers; the above test seems to assume
> UNIFY_ALLOW_{OUTER_,}LESS_CV_QUAL.
>
The reason I ignored strict was to enforce that the deduced address
space is always at most "as_parm" unless "as_parm" is the generic address
space, and prevent unifying if the two address spaces are disjoint
unless "parm" does not have any address space constraints; and avoid the
addition/deletion of an address space to "arg" during the unifying
process.
Since I don't really understand the whole picture behind strict, and when
check_cv_quals_for_unify gets called with which variant of restrict it
might be me who tried to be overcareful when unifying the address
spaces.
> > if (TREE_CODE (parm) == TEMPLATE_TYPE_PARM
> > && !(strict & UNIFY_ALLOW_OUTER_MORE_CV_QUAL))
> > @@ -24415,10 +24426,28 @@ unify (tree tparms, tree targs, tree parm, tree
> > arg, int strict,
> > arg, parm))
> > return unify_cv_qual_mismatch (explain_p, parm, arg);
> > + int arg_cv_quals = cp_type_quals (arg);
> > + int parm_cv_quals = cp_type_quals (parm);
> > +
> > + /* If PARM does not contain any address spaces constraints it can
> > + fully match the address space of ARG. However, if PARM contains an
> > + address space constraints, it becomes the upper bound. That is,
> > + AS_ARG may be promoted to AS_PARM but not the converse. If we
> > + ended up here, it means that `check_cv_quals_for_unify' succeeded
> > + and that either AS_PARM is 0 (ie., no constraints) or AS_COMMON ==
> > + AS_PARM. */
> > + addr_space_t as_arg = DECODE_QUAL_ADDR_SPACE (arg_cv_quals);
> > + addr_space_t as_parm = DECODE_QUAL_ADDR_SPACE (parm_cv_quals);
> > + addr_space_t as_common = as_parm ? 0 : as_arg;
>
> Hmm, I'd think we also want as_common = as_arg when it's a subset of
> as_parm.
>
Let's assume that "PARM" is "__as1 T", and since the call to
check_cv_quals_for_unify succeeded we know that "as_common" is
"__as1". That is ARG is of the form "__as2 U" with "__as2" a
subset of "__as1", hence we are trying to unify
__as1 T = __as1 U
which does not give any constraints over PARM since it alreay contains
the common address space, hence there is no more constraints on T and
as_common = 0.
However, if PARM's address space is 0, we are trying to unify
T = __as1 U
and we need to add __addr_space1 to the constraints of T.
If as_parm is not the generic address space (ie, as_parm != 0)
> > /* Consider the case where ARG is `const volatile int' and
> > PARM is `const T'. Then, T should be `volatile int'. */
> > arg = cp_build_qualified_type
> > (arg, cp_type_quals (arg) & ~cp_type_quals (parm), tf_none);
> > + int unified_cv =
> > + (CLEAR_QUAL_ADDR_SPACE (arg_cv_quals & ~parm_cv_quals)
> > + | ENCODE_QUAL_ADDR_SPACE (as_common));
> > + arg = cp_build_qualified_type (arg, unified_cv, tf_none);
> > if (arg == error_mark_node)
> > return unify_invalid (explain_p);
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index 45348c58bb6..1f330ca93ed 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -6072,15 +6072,6 @@ cp_free_lang_data (tree t)
> > DECL_CHAIN (t) = NULL_TREE;
> > }
> > -/* Stub for c-common. Please keep in sync with c-decl.cc.
> > - 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/cp/typeck.cc b/gcc/cp/typeck.cc
> > index da0e1427b97..93cfdc70e2d 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -803,10 +803,28 @@ composite_pointer_type (const op_location_t &location,
> > else
> > return error_mark_node;
> > }
> > + /* If possible merge the address space into the superset of the
> > address
> > + spaces of t1 and t2, or raise an error. */
> > + addr_space_t as_t1 = TYPE_ADDR_SPACE (t1);
> > + addr_space_t as_t2 = TYPE_ADDR_SPACE (t2);
> > + addr_space_t as_common;
> > +
> > + /* If the two named address spaces are different, determine the
> > common
> > + superset address space. If there isn't one, raise an error. */
> > + if (!addr_space_superset (as_t1, as_t2, &as_common))
> > + {
> > + as_common = as_t1;
> > + error_at (location,
> > + "%qT and %qT are in disjoint named address spaces",
> > + t1, t2);
>
> Why not return error_mark_node here?
>
That's a mistake. Thanks.
> > + }
> > + int quals_t1 = cp_type_quals (TREE_TYPE (t1));
> > + int quals_t2 = cp_type_quals (TREE_TYPE (t2));
> > result_type
> > = cp_build_qualified_type (void_type_node,
> > - (cp_type_quals (TREE_TYPE (t1))
> > - | cp_type_quals (TREE_TYPE (t2))));
> > + (CLEAR_QUAL_ADDR_SPACE (quals_t1)
> > + | CLEAR_QUAL_ADDR_SPACE (quals_t2)
> > + | ENCODE_QUAL_ADDR_SPACE (as_common)));
> > result_type = build_pointer_type (result_type);
> > /* Merge the attributes. */
> > attributes = (*targetm.merge_type_attributes) (t1, t2);
> > @@ -1731,7 +1749,9 @@ comptypes (tree t1, tree t2, int strict)
> > }
> > /* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
> > - top-level qualifiers. */
> > + top-level qualifiers, except for named address spaces. If the pointers
> > point
> > + to different named addresses spaces, then we must determine if one
> > address
> > + space is a subset of the other. */
> > bool
> > same_type_ignoring_top_level_qualifiers_p (tree type1, tree type2)
> > @@ -1741,6 +1761,14 @@ same_type_ignoring_top_level_qualifiers_p (tree
> > type1, tree type2)
> > if (type1 == type2)
> > return true;
> > + addr_space_t as_type1 = TYPE_ADDR_SPACE (type1);
> > + addr_space_t as_type2 = TYPE_ADDR_SPACE (type2);
> > + addr_space_t as_common;
> > +
> > + /* Fail if pointers point to incompatible address spaces. */
> > + if (!addr_space_superset (as_type1, as_type2, &as_common))
> > + return false;
>
> Why do you need this change? I'd expect this function to ignore top level
> address space qualifiers like the other qualifiers.
>
I am mirroring the C front-end here, which does the same thing in
"comp_target_types" (gcc/c/c-typeck.cc), which ignores qualifiers but
not address spaces when checking if two pointer types are equivalent.
> > type1 = cp_build_qualified_type (type1, TYPE_UNQUALIFIED);
> > type2 = cp_build_qualified_type (type2, TYPE_UNQUALIFIED);
> > return same_type_p (type1, type2);
> > @@ -6672,10 +6700,32 @@ static tree
> > pointer_diff (location_t loc, tree op0, tree op1, tree ptrtype,
> > tsubst_flags_t complain, tree *instrument_expr)
> > {
> > - tree result, inttype;
> > tree restype = ptrdiff_type_node;
> > + tree result, inttype;
> > +
> > + addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op0)));
> > + addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op1)));
> > tree target_type = TREE_TYPE (ptrtype);
> > + /* If the operands point into different address spaces, we need to
> > + explicitly convert them to pointers into the common address space
> > + before we can subtract the numerical address values. */
> > + if (as0 != as1)
> > + {
> > + addr_space_t as_common;
> > + tree common_type;
> > +
> > + /* Determine the common superset address space. This is guaranteed
> > + to exist because the caller verified that comp_target_types
> > + returned non-zero. */
> > + if (!addr_space_superset (as0, as1, &as_common))
> > + gcc_unreachable ();
> > +
> > + common_type = common_pointer_type (TREE_TYPE (op0), TREE_TYPE (op1));
> > + op0 = convert (common_type, op0);
> > + op1 = convert (common_type, op1);
> > + }
>
> I think you shouldn't need to change pointer_diff if composite_pointer_type
> returns error_mark_node above.
I'll have a look, the idea here is to prevent "a - b" with "a" and "b"
from different address spaces.
>
> > if (!complete_type_or_maybe_complain (target_type, NULL_TREE, complain))
> > return error_mark_node;
> > @@ -11286,6 +11336,19 @@ comp_ptr_ttypes_real (tree to, tree from, int
> > constp)
> > to_more_cv_qualified = true;
> > }
> > + /* Warn about conversions between pointers to disjoint
> > + address spaces. */
> > + if (TREE_CODE (from) == POINTER_TYPE
> > + && TREE_CODE (to) == POINTER_TYPE)
> > + {
> > + addr_space_t as_from = TYPE_ADDR_SPACE (TREE_TYPE (from));
> > + addr_space_t as_to = TYPE_ADDR_SPACE (TREE_TYPE (to));
> > + addr_space_t as_common;
> > +
> > + if (!addr_space_superset (as_to, as_from, &as_common))
> > + return false;
>
> I think you also want to check that as_common == as_to here?
>
Yes.
> > + }
> > +
> > if (constp > 0)
> > constp &= TYPE_READONLY (to);
> > }
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index cfbe32afce9..ef75f6b83a2 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -1448,7 +1448,7 @@ Fixed-point types are supported by the DWARF debug
> > information format.
> > @section Named Address Spaces
> > @cindex Named Address Spaces
> > -As an extension, GNU C supports named address spaces as
> > +As an extension, GNU C and GNU C++ support named address spaces as
> > defined in the N1275 draft of ISO/IEC DTR 18037. Support for named
> > address spaces in GCC will evolve as the draft technical report
> > changes. Calling conventions for any target might also change. At
> > diff --git a/gcc/testsuite/g++.dg/abi/mangle-addr-space1.C
> > b/gcc/testsuite/g++.dg/abi/mangle-addr-space1.C
> > new file mode 100644
> > index 00000000000..c01f8d6054a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/abi/mangle-addr-space1.C
> > @@ -0,0 +1,10 @@
> > +// { dg-do run { target { i?86-*-* x86_64-*-* } } }
>
> This can be dg-do compile, I don't think you get anything from running an
> empty main.
>
Yes.
> > +// { dg-options "-fabi-version=8 -Wabi -save-temps" }
>
> And then you don't need -save-temps. What are the other options for?
>
I forgot to remove -Wabi and -fabi-version, this was from my first
attempt when I used AS<number> to mangle which changed the ABI. I'll
remove them.
> > +// { dg-final { scan-assembler "_Z1fPU8__seg_fsVi" } }
> > +
> > +int f (int volatile __seg_fs *a)
> > +{
> > + return *a;
> > +}
> > +
> > +int main () {}
> > diff --git a/gcc/testsuite/g++.dg/abi/mangle-addr-space2.C
> > b/gcc/testsuite/g++.dg/abi/mangle-addr-space2.C
> > new file mode 100644
> > index 00000000000..862bbbdcdf2
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/abi/mangle-addr-space2.C
> > @@ -0,0 +1,9 @@
> > +// { dg-do run { target { i?86-*-* x86_64-*-* } } }
> > +// { dg-options "-fabi-version=8 -Wabi -save-temps" }
>
> Also not clear that running is important for this test.
>
Noted.
> > +// { dg-final { scan-assembler "_Z1fIU8__seg_fsiEiPT_" } }
> > +
> > +template <class T>
> > +int f (T *p) { return *p; }
> > +int g (__seg_fs int *p) { return *p; }
> > +__seg_fs int *a;
> > +int main() { f(a); }
> > diff --git a/gcc/testsuite/g++.dg/ext/addr-space-decl.C
> > b/gcc/testsuite/g++.dg/ext/addr-space-decl.C
> > new file mode 100644
> > index 00000000000..c04d2f497da
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/addr-space-decl.C
> > @@ -0,0 +1,5 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +__seg_fs char a, b, c;
> > +__seg_fs const int *p;
> > +static /* give internal linkage to the following anonymous struct */
>
> Hmm, this 'static' gives internal linkage to the variable q, not the type.
> What do you want it for?
>
Yes, the idea is to give internal linkage to q, otherwise g++
complains in -std=c++98 mode because q is externally visible but it
can't be reffered from anywhere else since there is no tag for this
structure.
> > +__seg_fs struct { int a; char b; } * __seg_gs q;
> > diff --git a/gcc/testsuite/g++.dg/ext/addr-space-ops.C
> > b/gcc/testsuite/g++.dg/ext/addr-space-ops.C
> > new file mode 100644
> > index 00000000000..86c02d1e7f5
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/addr-space-ops.C
> > @@ -0,0 +1,20 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +int __seg_fs * fs1;
> > +int __seg_fs * fs2;
> > +float __seg_gs * gs1;
> > +float __seg_gs * gs2;
> > +
> > +int
> > +main ()
> > +{
> > + fs1 + fs2; // { dg-error "invalid operands of types .__seg_fs int.. and
> > .__seg_fs int.. to binary .operator.." }
> > + fs1 - fs2;
> > + fs1 - gs2; // { dg-error "invalid operands of types .__seg_fs int.. and
> > .__seg_gs float.. to binary .operator.." }
> > + fs1 == fs2;
> > + fs1 != gs2; // { dg-error "comparison between distinct pointer types
> > .__seg_fs int.. and .__seg_gs float.. lacks a cast" }
> > + fs1 = fs2;
> > + fs1 = gs2; // { dg-error "cannot convert .__seg_gs float.. to .__seg_fs
> > int.. in assignment" }
> > + fs1 > fs2;
> > + fs1 < gs2; // { dg-error "comparison between distinct pointer types
> > .__seg_fs int.. and .__seg_gs float.. lacks a cast" }
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/ext/addr-space-ref.C
> > b/gcc/testsuite/g++.dg/ext/addr-space-ref.C
> > new file mode 100644
> > index 00000000000..12d7975e560
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/addr-space-ref.C
> > @@ -0,0 +1,31 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +// { dg-prune-output "does not allow .register. storage class specifier" }
> > +int __seg_fs * outer_b;
> > +
> > +struct s {
> > + __seg_fs int * ok;
> > + __seg_gs int ko; // { dg-error ".__seg_gs. specified for structure field
> > .ko." }
> > +};
> > +
> > +int register __seg_fs reg_fs; // { dg-error ".__seg_fs. combined with
> > .register. qualifier for .reg_fs." }
> > +
> > +namespace ns_a
> > +{
> > + int __seg_fs * inner_b;
> > +
> > + template<typename T>
> > + int f (T &a) { return a; }
> > + int g (__seg_fs int a) { return a; } // { dg-error ".__seg_fs. specified
> > for parameter .a." }
> > + int h (__seg_fs int *a) { return *a; }
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + int register __seg_gs reg_gs; // { dg-error ".__seg_gs. combined with
> > .register. qualifier for .reg_gs." }
> > + static __seg_gs int static_gs;
> > + __seg_fs int auto_fs; // { dg-error ".__seg_fs. specified for auto
> > variable .auto_fs." }
> > + __seg_fs int *pa = outer_b;
> > + __seg_fs int& ra = *ns_a::inner_b;
> > + return ns_a::f(ra) + ns_a::f(*pa);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/parse/addr-space.C
> > b/gcc/testsuite/g++.dg/parse/addr-space.C
> > new file mode 100644
> > index 00000000000..ebb6316054a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/parse/addr-space.C
> > @@ -0,0 +1,9 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +
> > +__seg_fs struct foo; // { dg-error "address space can only be specified
> > for objects and functions" }
> > +
> > +int
> > +main ()
> > +{
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/parse/addr-space1.C
> > b/gcc/testsuite/g++.dg/parse/addr-space1.C
> > new file mode 100644
> > index 00000000000..2e8ee32a885
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/parse/addr-space1.C
> > @@ -0,0 +1,10 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +// { dg-options "-std=gnu++98" }
> > +
> > +int
> > +main ()
> > +{
> > + struct foo {int a; char b[2];} structure;
> > + structure = ((__seg_fs struct foo) {1 + 2, 'a', 0}); // { dg-error
> > "compound literal qualified by address-space qualifier" }
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/parse/addr-space2.C
> > b/gcc/testsuite/g++.dg/parse/addr-space2.C
> > new file mode 100644
> > index 00000000000..5b2c0f28078
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/parse/addr-space2.C
> > @@ -0,0 +1,9 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +
> > +__seg_fs __seg_gs int *a; // { dg-error "conflicting named address spaces
> > .__seg_fs vs __seg_gs." }
> > +
> > +int
> > +main ()
> > +{
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/template/addr-space-overload.C
> > b/gcc/testsuite/g++.dg/template/addr-space-overload.C
> > new file mode 100644
> > index 00000000000..70dfcce53fa
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/addr-space-overload.C
> > @@ -0,0 +1,17 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +
> > +int __seg_fs * fs1;
> > +int __seg_gs * gs1;
> > +
> > +template<typename T, typename U>
> > +__seg_fs T* f (T __seg_fs * a, U __seg_gs * b) { return a; }
> > +template<typename T, typename U>
> > +__seg_gs T* f (T __seg_gs * a, U __seg_fs * b) { return a; }
> > +
> > +int
> > +main ()
> > +{
> > + f (fs1, gs1);
> > + f (gs1, fs1);
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/template/addr-space-strip1.C
> > b/gcc/testsuite/g++.dg/template/addr-space-strip1.C
> > new file mode 100644
> > index 00000000000..5df115db939
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/addr-space-strip1.C
> > @@ -0,0 +1,17 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +// { dg-skip-if "" { *-*-* } { "-std=c++98" "-std=c++03" "-std=gnu++98"
> > "-std=gnu++03" } { "" } }
>
> This can be { dg-require-effective-target c++11 }
>
> > +// decltype is ony available since c++11
>
> "only"
>
> > +
> > +int __seg_fs * fs1;
> > +int __seg_gs * gs1;
> > +
> > +template<typename T> struct strip;
> > +template<typename T> struct strip<__seg_fs T *> { typedef T type; };
> > +template<typename T> struct strip<__seg_gs T *> { typedef T type; };
> > +
> > +int
> > +main ()
> > +{
> > + *(strip<decltype(fs1)>::type *) fs1 == *(strip<decltype(gs1)>::type *)
> > gs1;
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/template/addr-space-strip2.C
> > b/gcc/testsuite/g++.dg/template/addr-space-strip2.C
> > new file mode 100644
> > index 00000000000..526bbaa56b7
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/addr-space-strip2.C
> > @@ -0,0 +1,16 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +
> > +int __seg_fs * fs1;
> > +int __seg_gs * gs1;
> > +
> > +template<typename T, typename U>
> > +bool f (T __seg_fs * a, U __seg_gs * b)
> > +{
> > + return *(T *) a == *(U *) b;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + return f (fs1, gs1);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/template/spec-addr-space.C
> > b/gcc/testsuite/g++.dg/template/spec-addr-space.C
> > new file mode 100644
> > index 00000000000..ae9f4de0e1f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/spec-addr-space.C
> > @@ -0,0 +1,8 @@
> > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
> > +
> > +template <class T>
> > +int f (T __seg_gs *p) { return *p; } // { dg-note "candidate:
> > 'template<class T> int f.__seg_gs T\*." }
> > + // { dg-note "template argument
> > deduction/substitution failed:" "" { target *-*-* } .-1 }
> > +__seg_fs int *a;
> > +int main() { f(a); } // { dg-error "no matching" }
> > +// { dg-note "types .__seg_gs T. and .__seg_fs int. have incompatible
> > cv-qualifiers" "" { target *-*-* } .-1 }
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index 9af971cf401..4aebfef854b 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -2292,7 +2292,7 @@ extern tree vector_element_bits_tree (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. */
