diff mbox series

symtab: also change RTL decl name [was: RFH: attr target_clones default assembler name ignored?]

Message ID 20221104173920.5313660c@nbbrfq
State Accepted
Headers
Series symtab: also change RTL decl name [was: RFH: attr target_clones default assembler name ignored?] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Bernhard Reutner-Fischer Nov. 4, 2022, 4:39 p.m. UTC 
  On Thu, 3 Nov 2022 23:23:55 +0100
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:

> Hi!
> 
> I encounter a problem/pilot error when using the target_clones
> attribute.
> 
> The symptom is that the default clone is not renamed in the output and
> thus conflicts with the (proper) global name which is used for the
> ifunc:
> 
> $ nl -ba < /tmp/cc3jBX3x.s | grep sub1
>     12		.type	__m_MOD_sub1, @function
>     13	__m_MOD_sub1:
>     35		.size	__m_MOD_sub1, .-__m_MOD_sub1
> ...
>     87	__m_MOD_sub1.resolver:
>     95		movl	$__m_MOD_sub1.avx, %eax
>    104		movl	$__m_MOD_sub1, %eax
>    105		movl	$__m_MOD_sub1.sse, %edx
>    110		.size	__m_MOD_sub1.resolver, .-__m_MOD_sub1.resolver
>    111		.globl	__m_MOD_sub1
>    112		.type	__m_MOD_sub1, @gnu_indirect_function
>    113		.set	__m_MOD_sub1,__m_MOD_sub1.resolver
> 
> I think that line 13 and 104 should talk about __m_MOD_sub1.default.
> 
> AFAICT the target_clones attr is built well.
> gcc/multiple_target.cc:expand_target_clones() adds the required
> attr "target" "default"
> and properly does:
> (gdb) p old_name
> $6 = 0x7ffff6dfc090 "__m_MOD_sub1"
> (gdb) call debug_tree ( DECL_ASSEMBLER_NAME (decl) )
>  <identifier_node 0x7ffff6dfd640 __m_MOD_sub1>
> (gdb) n
> 301		  && DECL_RTL_SET_P (decl))
> (gdb) n
> 300	      if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
> (gdb) n
> 304	      SET_DECL_ASSEMBLER_NAME (decl, name);
> (gdb) n
> 305	      if (alias)
> (gdb) call debug_tree ( DECL_ASSEMBLER_NAME (decl) )
>  <identifier_node 0x7ffff6a0b758 __m_MOD_sub1.default>
> 
> So if that would have been used for real, all would be fine i think.
> 
> But still, that name is somehow not used, see fnname:
> #0  assemble_start_function (decl=<function_decl 0x7ffff6df9500 sub1>, fnname=fnname@entry=0x7ffff6dfc090 "__m_MOD_sub1") at ../../../src/gcc-13.mine/gcc/varasm.cc:1979

> I'd be grateful for help to understand why/who chooses to pick an unpleasant
> name, and, primarily, how to avoid that problem.
> 
> Thoughts or hints?
> TIA!
> PS: Should i have rather asked on gcc-help?

We make_decl_rtl rather early when creating a function and set the name
to the (unversioned, from the POV of target_clones) initial name of the
function. Then we parse the attributes and attach it to the fndecl when
lowering. Later on the ME creates the clones and renames the function
via symbol_table::change_decl_assembler_name() which changes just the
DECL_ASSEMBLER_NAME and not the name in the DECL_RTL.

With this, the target_clones attribute works for me from the Fortran
FE.

The following tests cleanly for --enable-languages=c,fortran,c++,lto
Ok for trunk?

PS: I did not look if it would be possible to change the way this
fnname (as per get_fnname_from_decl()) is stored in both RTL and tree
in apparently two different places. I would have assumed that varasm
uses a name via cgraph but that's obviously not (always) the case.

gcc/ChangeLog:

	* symtab.cc: Remove stray comment.
	(symbol_table::change_decl_assembler_name): Also update the
	name in DECL_RTL.
diff mbox series

Patch

diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index f2d96c0268b..53a1a2a26bf 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -153,10 +153,8 @@  symbol_table::decl_assembler_name_equal (tree decl, const_tree asmname)
   return assembler_names_equal_p (decl_str, asmname_str);
 }
 
 
-/* Returns nonzero if P1 and P2 are equal.  */
-
 /* Insert NODE to assembler name hash.  */
 
 void
 symbol_table::insert_to_assembler_name_hash (symtab_node *node,
@@ -302,8 +301,12 @@  symbol_table::change_decl_assembler_name (tree decl, tree name)
 	  && DECL_RTL_SET_P (decl))
 	warning (0, "%qD renamed after being referenced in assembly", decl);
 
       SET_DECL_ASSEMBLER_NAME (decl, name);
+      /* Set the new name in rtl.  */
+      if (DECL_RTL_SET_P (decl))
+	XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);
+
       if (alias)
 	{
 	  IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;
 	  TREE_CHAIN (name) = alias;