On 10/26/22 03:18, Paul Iannetta wrote:
On Wed, Oct 19, 2022 at 02:55:21PM -0400, Jason Merrill wrote:On 10/18/22 13:01, Paul Iannetta wrote: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.Yes, but register variables are automatic, so they'll get that error; I don't think they need their own specific error.Noted.+ { + 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.How we need to handle differing qualifiers varies between different template argument deduction contexts. The code you wrote above is correct for the function call context, since https://eel.is/c++draft/temp.deduct.call#4.2 says the deduced type can be convertable by qualification conversion, i.e. parm more qualified than arg (and my "LESS" above was backwards). This is a bit different for address space qualifiers given that the qualification conversion would be removing the address space qualifier or changing it to a more general one, but the principle is the same. But the allowance for qualifier changes doesn't apply to all deduction contexts: for instance, template <class T> void f(T * const *); struct A { template <class T> operator T**(); }; int main() { f((void**)0); // void** -> void*const* is a valid qualification conv (void *const*)A(); // same conversion void (*p)(void **) = f; // error, type mismatch } so similarly, template <class T> void f(T **); struct A { template <class T> operator T*__seg_fs*(); }; int main() { f((void* __seg_fs *)0); // void*__seg_fs* -> void** should be OK void (*p)(void * __seg_fs *) = f; // error }I do not completely agree here. Currently, my implementation rejects all deductions which would change or remove an address space no matter the context, which is very conservative. I tried using "strict" as the other qualifiers do, and as I expected, it keeps rejecting f((void* __seg_fs *)0); // void*__seg_fs* -> void** should be OK which is to be expected, since a pointer can't jump from an address space to another unless there is a common superset and here __seg_fs is disjoint from the generic address space.
Aha, I was thinking that the generic address space was a superset.
I don't really understand what is done in
(void **)A(); // same conversion
but it is similarly rejected (implicit conversion from A to (void**))
The third one is strangely accepted, and clang accept is as well (only
the address space variant, the one with const is duly rejected).
I will investigate what clang does here, as I think it would be better
if the behavior of clang and gcc concerning this feature matches as
much as possible from a user standpoint, since the C++ side of this
feature is, to my knowledge, completely undocumented.
I assume this is a bug in clang as well.
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.Agreed.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.Agreed.If as_parm is not the generic address space (ie, as_parm != 0)Looks like this comment got cut off? This is the case I was talking about. When we are trying to unify __as1 T = __as2 U and __as2 is a subset of __as1, I think we want T to be deduced to __as2 U, and then substitution will need to handle substituting __as2 U for T into __as1 T to get __as2 U.I more or less agree, but I think that the substitution will need to handle substituting __as2 U for T into __as1 T to get __as1 U. (Leading to __as1 U and not __as2 U, since __as1 is the biggest address space and the templated function expect __as1 T).
After substitution, I would think we want to end up with the smaller address space; in general, we want the more specialized form.
Nevertheless, this means that when fully deduced __as1 T becomes __as1 __as2 U and then the substitution mechanism would make it into __as1 U. Could you please tell me where the substitution mechanism takes place so that I can account for this case and that if we end up with two compatible address spaces the biggest is selected?
I think it would be best to handle it in cp_build_qualified_type.
/* 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.This function serves a very different function from comp_target_types, which deals with the types that pointers point to; this function is ignoring top-level qualifiers that should not affect the type. ...except now I see that cp_build_binary_op is wierdly using this function for pointer subtraction. I'd think it should use composite_pointer_type instead, like EQ_EXPR does.I think this is because of https://eel.is/c++draft/expr.add#2.2 and I am not sure that composite_pointer_type can replace it here since it does try to merge the two list of qualifiers.
Ah, good point. So I guess it does make sense for it to use same_type_ignoring_top_level_qualifiers_p there, but the other callers of that function don't want this address space check; it should be enough to do it only in pointer_diff.
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.As above, I think this should have been handled in cp_build_binary_op.I don't really understand why you don't want the address space conversion (which might be needed for subtraction) to happen at the same time as the conversion to the "common_pointer_type".
Agreed, except you'll want a diagnostic here now.
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.Then let's change the comment to /* give internal linkage to q */Agreed.+__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 }Or put the x86 requirement in dg-require-effective-target, and put c++11 in the dg-do target spec, either way.Agreed.+// 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. */
