On 5/15/26 8:17 AM, Thomas Schwinge wrote:
[Note that emails to <[email protected]> bounce; please use Paul's
other email address: <[email protected]>.]
Hi!
I'd like to resume this patch submission here, which is adding support
for named address spaces to GNU C++, as is implemented for GNU C. As far
as I can tell, there wasn't any specific technical reason that this patch
review stalled, back then, in 2022-11? (Jason?)
I've now rebased this onto recent GCC trunk, see attached
'0001-c-parser-Support-for-target-address-spaces-in-C.patch'. There were
just a few merge conflicts that I had to fix up (nothing serious), and
I've bootstrap-tested on x86_64-pc-linux-gnu (only, so far).
The 'gcc/targhooks.cc:default_addr_space_subset_p' change that appeared
in this v4:
bool
default_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
{
- return (subset == superset);
+ return (subset == 2 && superset == 0) || (subset == superset);
}
... is entirely unmotivated, as far as I can tell: probably a local hack
for testing, that sneaked into the patch posted? This change regresses
'gcc.target/i386/addr-space-typeck-1.c' (notice: C test case), which
again PASSes with that change undone.
This should definitely be dropped.
The new test case 'g++.dg/template/spec-addr-space.C' needs minor
adjusting per changed GCC diagnostics (not related to the new named
addess space support):
@@ -1,8 +1,9 @@
// { 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\*." }
+int f (T __seg_gs *p) { return *p; } // { dg-note "candidate 1: '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 }
+// { dg-note "there is 1 candidate" "" { target *-*-* } .-1 }
+// { dg-note "types .__seg_gs T. and .__seg_fs int. have incompatible
cv-qualifiers" "" { target *-*-* } .-2 }
With that, all new test cases PASS, and no regressions.
If we get a general "go", I'll then offer to fix up a few places for GCC
coding style conformance (so please don't review for that, yet), and I'll
work on test cases some more. I'll also examine if, since then, any new
code has appeared where 'ADDR_SPACE_CONVERT_EXPR' needs to be handled.
PR69549 "Named Address Spaces does not compile in C++" is going to be
resolved by this patch, so should get referenced in the Git log.
Full-quote of Paul's email from back then:
On 2022-11-10T16:42:22+0100, Paul Iannetta <[email protected]> wrote:
Hi,
It took a bit of time to rework the rough corners. I tried to be
mirror as much as possible the C front-end, especially when it comes
to implicit conversions, and warnings; and expose the same hooks as
the C front-end does. The C++ front-end produces as much warnings
that the C front-end, however they are sometimes a bit less
informative (especially so, when passing arguments to functions).
I also address the following points:
1. The handling of the register storage class is grouped with the
other variables with automatic storage. This incurs a slight
dis-alignment since you cannot have global register variables do not
trigger an error, only a warning.
2. In template unification, I maintain that we don't want any
changes to address spaces whatsoever during the unification process,
hence ignoring the strict flag. Nevertheless, we allow implicit
conversion, and I have verified that, indeed,
template <class T> void f(T **);
struct A {
template <class T> operator T*__seg_fs*();
};
int main()
{
f((void* __seg_fs *)0); // (1): void*__seg_fs* -> void** should
be OK
void (*p)(void * __seg_fs *) = f; // (2): error
}
works as intended. That is, (1) works if we set __seg_fs as a subspace
of the generic address space, and (2) is always an error.
3. In template unification, when unifying __as1 T = __as2 U we want
to unify to the __as1 at most, never to __as2 at most, because the
function requiring __as1 T may want to mix address space from the
bigger address space, therefore I think that we want to have the
smaller address-space to be unified into the bigger of the two.
4. I left untouched same_type_ignoring_top_level_qualifiers_p, even
though that was very convenient and often lead to better error
messages since error were caught earlier.
5. The handling of conversions is done very late in the calling
chain, because I absolutely want to fold the conversion and force
the conversion to appear as an ADDR_SPACE_CONV_EXPR after
gimplification.
6. Currently, I do not handle classes. I see what I can do in a
further revision and maybe add a target hook to hand down to targets
the choice of the address space of the vtable.
7. This can't be added as a test case, but I checked that:
// In this test case, __seg_gs is a subset of the generic address
// space.
int f (int *);
int main ()
{
int __seg_fs *pa;
int __seg_gs *pb;
int *pc;
pa = (__seg_fs int *) pb; return *pa; // warning: cast to ‘__seg_fs’
address space pointer from disjoint ‘__seg_gs’ address space pointer
pa = (int __seg_fs *) pc; return *pa; // warning: cast to ‘__seg_fs’
address space pointer from disjoint generic address space pointer
pa = pb; return *pa; // error: invalid conversion from ‘__seg_gs int*’ to
‘__seg_fs int*’
pc = pb; return *pc; // __seg_gs int * -> int * converted through
ADDR_SPACE_CONV_EXPR
pb = pc; return *pb; // error: invalid conversion from ‘int*’ to ‘__seg_gs
int*’
pc = pa; return *pb; // error: invalid conversion from ‘__seg_fs int*’ to
‘int*’
return f (pb); // __seg_gs int * -> int * converted through
ADDR_SPACE_CONV_EXPR
// return f (pa); // error: invalid conversion from ‘__seg_fs int*’ to
‘int*’
// note: initializing argument 1 of ‘int f(int*)’
}
Thanks,
Paul
Rebased on current trunk, bootstrapped and regtested.
Subject: [PATCH] c++: parser - Support for target address spaces in C++
gcc/cp/ChangeLog:
Missing the PR reference and rationale.
* call.cc (convert_like_internal): Add support for implicit
conversion between compatible address spaces. This is done
here (and not in a higher caller) because we want to force the
use of cp_fold_convert, which later enable back-end writers to
tune-up the fine details of the conversion.
This kind of explanation belongs in a code comment rather than the
ChangeLog.
This comment suggests that we want to support these conversions, but the
change is to the part of the function that handles "bad" conversions. I
would expect handling of conversions we want to support to go in the
ck_ptr case near the bottom of the function.
I was surprised that no change to standard_conversion was needed, but I
guess the change to comp_ptr_ttypes_real covers that.
I would also expect compare_ics to have an opinion about such
conversions vs conversions within the same address space.
@@ -840,12 +840,11 @@ extern const struct scoped_attribute_specs
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);
We don't need to declare it twice.
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;
+
+ if (!addr_space_superset (as0, as1, &as_common))
+ {
+ error_at (loc, "pointers to disjoint address spaces "
+ "%qT and %qT in substraction",
Typo in "subtraction".
@@ -17139,6 +17145,49 @@ grokdeclarator (const cp_declarator *declarator,
&& (!VAR_P (decl) || !DECL_TEMPLATE_INSTANTIATED (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 (VAR_P (decl) && decl_context == NORMAL)
+ {
+ duration_kind dk = decl_storage_duration (decl);
+ switch (dk)
+ {
+ case dk_auto:
+ if (! toplevel_bindings_p ())
+ error ("%qs specified for variable %qs with automatic storage",
+ c_addr_space_name (address_space), name);
The !toplevel_bindings_p check seems redundant with automatic storage
duration.
@@ -26334,10 +26349,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;
It seems unclear to talk about as_common being equal to as_parm and then
define a new as_common to 0; let's clarify the comment something like
it means that `check_cv_quals_for_unify' succeeded and that either
AS_PARM was 0 (i.e. no constraints), so the deduced AS_COMMON is AS_ARG,
or AS_COMMON was AS_PARM, so the deduced AS_COMMON is 0 (i.e. no
additional constraints).
+static void
+warn_about_cast_crossing_as_boundaries (location_t loc, tree type,
"as" is unclear outside the context of this patch, how about
"warn_about_address_space_cast"?
+ tree expr, tsubst_flags_t complain)
+{
+ if (TREE_CODE (type) == POINTER_TYPE
+ && TREE_CODE (TREE_TYPE (expr)) == POINTER_TYPE
+ && !NULLPTR_TYPE_P (expr)
This line seems useless: NULLPTR_TYPE_P (expr) can never be true, an
expr is not a type. And even TREE_TYPE (expr) has already been
established to be POINTER_TYPE, so it can't also be NULLPTR_TYPE.
Jason
