public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
To: gcc-patches@gcc.gnu.org, Jan Hubicka <hubicka@ucw.cz>
Cc: rep.dot.nop@gmail.com
Subject: [PATCH] symtab: also change RTL decl name [was: RFH: attr target_clones default assembler name ignored?]
Date: Fri, 4 Nov 2022 17:39:20 +0100	[thread overview]
Message-ID: <20221104173920.5313660c@nbbrfq> (raw)
In-Reply-To: <20221103232355.5eb1d235@nbbrfq>

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 --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;



      reply	other threads:[~2022-11-04 16:39 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 22:23 RFH: attr target_clones default assembler name ignored? Bernhard Reutner-Fischer
2022-11-04 16:39 ` Bernhard Reutner-Fischer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221104173920.5313660c@nbbrfq \
    --to=rep.dot.nop@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).