Re: [PATCH v7] Introduce attribute sym_alias

[Alexandre Oliva <oliva@adacore.com>]

On Dec  6, 2023, Jan Hubicka <hubicka@ucw.cz> wrote:

>> On Nov 30, 2023, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> >> +      if (VAR_P (replaced))
>> >> +	varpool_node::create_alias (sym_node->decl, replacement);
>> >> +      else
>> >> +	cgraph_node::create_alias (sym_node->decl, replacement);
>> 
>> Unfortunately, this change didn't work.  Several of the C++ tests
>> regressed with it.  Going back to same-body aliases, they work.
>> 
>> I suspect this may have to do with the infrastructure put in to deal
>> with cdtors clones.

> Do you have short testcase for this?

attr-sym-alias-[13].C are not too big, and show various regressions with
the incremental patchlet below (to be applied on top of v7], but here's
a minimal testcase that triggers the problem:

struct foo {
  __attribute__ ((__sym_alias__ ("FOODTR_A"))) ~foo() {}
};
foo bar;

> THe main oddities with same body
> aliases comes from the fact that C++ FE creates them early during
> parsing before all declaration flags are finished.

*nod*, this is probably why some of the cdtor and even sym_aliases for
inline member functions (FOOBAR_B and FOOBAR_C in attr-sym-alias-1.C,
and FOOCLS_INT_VIRT in attr-sym-alias-3.C) fail to be output when not
using create_same_body_alias.

diff --git a/gcc/attribs.cc b/gcc/attribs.cc
index abfbbbf6294..65ce610f2d4 100644
--- a/gcc/attribs.cc
+++ b/gcc/attribs.cc
@@ -2717,8 +2717,8 @@ create_sym_alias_decl (tree decl, tree id)
     // node = varpool_node::create_extra_name_alias (clone, decl);
     node = varpool_node::create_alias (clone, decl);
   else
-    node = cgraph_node::create_same_body_alias (clone, decl);
-    // node = cgraph_node::create_alias (clone, decl);
+    // node = cgraph_node::create_same_body_alias (clone, decl);
+    node = cgraph_node::create_alias (clone, decl);
   if (symtab_node *dnode = symtab_node::get_create (decl))
     node->copy_visibility_from (dnode);
 
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index 44df52095c1..e40240077f2 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -1993,8 +1993,8 @@ symtab_node::remap_sym_alias_target (tree replaced, tree replacement)
 	// varpool_node::create_extra_name_alias (sym_node->decl, replacement);
 	varpool_node::create_alias (sym_node->decl, replacement);
       else
-	cgraph_node::create_same_body_alias (sym_node->decl, replacement);
-	// cgraph_node::create_alias (sym_node->decl, replacement);
+	// cgraph_node::create_same_body_alias (sym_node->decl, replacement);
+	cgraph_node::create_alias (sym_node->decl, replacement);
       sym_node->copy_visibility_from (repl_node);
     }
 }

> Fixup copies some flags such as inline flags, visibility and comdat
> groups which can change during parsing process.

*nod*, I've run into some of that, and had to add visibility propagation
to the sym_aliases to make up for it.

But I'm not sure that that's the issue you're getting at.  Some
sym_aliases don't even get output with this patchlet.  FOODTR_A* aliases
get created during parsing, when maybe_clone_body creates the dtor
clones and their cgraph nodes to set their comdat group.  ISTM that it's
the later visibility copying because of same body alias that enables the
alias declaration to get the same (final) visibility as the declarations
they alias.  Which suggests that there could be another way to ensure
the update takes place, but the best spot for it has so far eluded me.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive