Am 23.06.26 um 01:07 schrieb Arsen Arsenović:
Jason Merrill <[email protected]> writes:

But we don't do that for const.  In C++ the quals apply to the element type
rather than the array type; see build_cplus_array_type and cp_type_quals for
places that deal with that.  Places that call

   cp_apply_type_quals_to_decl (cp_type_quals (type), decl);

handle setting TREE_READONLY and such on the decl; is there a parallel for
address space?

AFAICT no, I see nothing similar to DECL_READONLY etc but for address
spaces on decls.

On top of that, anything that wants to know the address space of
whatever it may be checks the type of the decl/operand/... for it
directly (and, manually each time, so there's not even a
EXPR_OR_DECL_ADDR_SPACE or such :S), at a glance with grep[1].

The narrow issue Johann originally reported can be fixed by teaching
varasm to strip arrays before concluding what address space some
variable is.

Thank you, that did the trick.I have tried the attached patch with
-f[no-]data-sections.

diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 0dbc35d926a..11e42bb6ade 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -1268,7 +1268,6 @@ compute_reloc_for_var (tree decl)
 section *
 get_variable_section (tree decl, bool prefer_noswitch_p)
 {
-  addr_space_t as = ADDR_SPACE_GENERIC;
   int reloc;
   varpool_node *vnode = varpool_node::get (decl);
   if (vnode)
@@ -1277,8 +1276,10 @@ get_variable_section (tree decl, bool prefer_noswitch_p)
       decl = vnode->decl;
     }

-  if (TREE_TYPE (decl) != error_mark_node)
-    as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
+  tree as_type = strip_array_types (TREE_TYPE (decl));
+  addr_space_t as = as_type == error_mark_node
+    ? ADDR_SPACE_GENERIC
+    : TYPE_ADDR_SPACE (as_type);

   /* We need the constructor to figure out reloc flag.  */
   if (vnode)
@@ -1661,12 +1662,10 @@ make_decl_rtl (tree decl)
     x = create_block_symbol (name, get_block_for_decl (decl), -1);
   else
     {
-      machine_mode address_mode = Pmode;
-      if (TREE_TYPE (decl) != error_mark_node)
-       {
-         addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
-         address_mode = targetm.addr_space.address_mode (as);
-       }
+      tree as_type = strip_array_types (TREE_TYPE (decl));
+      machine_mode address_mode = as_type == error_mark_node
+       ? Pmode
+       : targetm.addr_space.address_mode (TYPE_ADDR_SPACE (as_type));
       x = gen_rtx_SYMBOL_REF (address_mode, name);
     }
   SYMBOL_REF_WEAK (x) = DECL_WEAK (decl);


But the rest of the compiler would probably need similar changes, as,
AFAICT, it all assumes that TREE_ADDR_SPACE (array) is set to
TREE_ADDR_SPACE (TREE_TYPE (array)) prior to using it.

Alternatively, of course, we could make such a EXPR_OR_DECL_ADDR_SPACE
and replace all (applicable) usages of TYPE_ADDR_SPACE with it.

I think it'd be semantically sound to make GENERIC semantics match C++
when dealing with address spaces on array types (i.e. define that the
the address space of an array type is the address space of the element
type; nothing else really makes sense after all), so it may be best to
make TYPE_ADDR_SPACE strip arrays.

I am not sure if that's a good idea.  For example the outcome of
strip_array_types() might be error_mark_node.

Moreover, while convenient in many places, it would be confusing.

Johann


This would probably make it not an lvalue, but only a handful of places
use TYPE_ADDR_SPACE as an lvalue anyway, so that's not a big burden.

Though, that may be a bit awkward in relation to TYPE_QUALS, as,
suddenly, one of the qualifiers it reads is not necessarily applicable
on the type given to it.

What do you think is best?  I'm thinking that the latter gets rid of the
whole ambiguity, and so, may be the best solution overall.

[1] rg -NI -o -g '*.cc' -g '!cp' -g '!c-family' -g '!c' 
'[A-Za-z0-9_]*_ADDR_SPACE(_[A-Za-z0-9_]*)?' | sort -u
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 0dbc35d926a..11e42bb6ade 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -1268,7 +1268,6 @@ compute_reloc_for_var (tree decl)
 section *
 get_variable_section (tree decl, bool prefer_noswitch_p)
 {
-  addr_space_t as = ADDR_SPACE_GENERIC;
   int reloc;
   varpool_node *vnode = varpool_node::get (decl);
   if (vnode)
@@ -1277,8 +1276,10 @@ get_variable_section (tree decl, bool prefer_noswitch_p)
       decl = vnode->decl;
     }
 
-  if (TREE_TYPE (decl) != error_mark_node)
-    as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
+  tree as_type = strip_array_types (TREE_TYPE (decl));
+  addr_space_t as = as_type == error_mark_node
+    ? ADDR_SPACE_GENERIC
+    : TYPE_ADDR_SPACE (as_type);
 
   /* We need the constructor to figure out reloc flag.  */
   if (vnode)
@@ -1661,12 +1662,10 @@ make_decl_rtl (tree decl)
     x = create_block_symbol (name, get_block_for_decl (decl), -1);
   else
     {
-      machine_mode address_mode = Pmode;
-      if (TREE_TYPE (decl) != error_mark_node)
-	{
-	  addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
-	  address_mode = targetm.addr_space.address_mode (as);
-	}
+      tree as_type = strip_array_types (TREE_TYPE (decl));
+      machine_mode address_mode = as_type == error_mark_node
+	? Pmode
+	: targetm.addr_space.address_mode (TYPE_ADDR_SPACE (as_type));
       x = gen_rtx_SYMBOL_REF (address_mode, name);
     }
   SYMBOL_REF_WEAK (x) = DECL_WEAK (decl);

Reply via email to