diff mbox series

Avoid unnecessarily numbered clone symbols

Message ID 8d03bc9c-cd65-3774-86d0-7deb625b5748@oracle.com
State New
Headers show
Series Avoid unnecessarily numbered clone symbols | expand

Commit Message

Michael Ploujnikov Oct. 19, 2018, 10:26 p.m. UTC 
While working on
https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00228.html I've
accumulated a few easy patches.

The first one renames the functions in question to hopefully encourage
proper future usage. The other ones use the unnumbered version of the
clone name function where I've verified the numbers are not
needed. I've verified these by doing a full bootstrap and a regression
test, by instrumenting the code and by understanding and following the
surrounding code to convince myself that the numbering is indeed not
needed. For the cold functions I've also confirmed with Sriraman
Tallam that they don't need to be numbered.



Regards,
- Michael

Comments

Bernhard Reutner-Fischer Oct. 20, 2018, 11:39 a.m. UTC | #1 
On 20 October 2018 00:26:15 CEST, Michael Ploujnikov <michael.ploujnikov@oracle.com> wrote:
>While working on
>https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00228.html I've
>accumulated a few easy patches.

 
+/* Return decl name IDENTIFIER with string SUFFIX appended.  */
+
+tree
+suffixed_function_name (tree identifier, const char *suffix)
+{
+  const char *name = IDENTIFIER_POINTER (identifier);
+  size_t len = strlen (name);
+  char *prefix;
+
+  prefix = XALLOCAVEC (char, len + strlen (suffix) + 2);
+  memcpy (prefix, name, len);
+  prefix[len] = symbol_table::symbol_suffix_separator ();
+  strcpy (prefix + len + 1, suffix);
+  return get_identifier (prefix);
+}
+

FWIW I think I would phrase this as

char *str = concat (
  IDENTIFIER_POINTER (identifier),
  symbol_table::symbol_suffix_separator (),
  suffix);
  tree ret = get_identifier (str);
  free (str);
  return ret;
Michael Ploujnikov Oct. 20, 2018, 10:56 p.m. UTC | #2 
On 2018-10-20 07:39 AM, Bernhard Reutner-Fischer wrote:
> On 20 October 2018 00:26:15 CEST, Michael Ploujnikov <michael.ploujnikov@oracle.com> wrote:
>> While working on
>> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00228.html I've
>> accumulated a few easy patches.
> 
>  
> +/* Return decl name IDENTIFIER with string SUFFIX appended.  */
> +
> +tree
> +suffixed_function_name (tree identifier, const char *suffix)
> +{
> +  const char *name = IDENTIFIER_POINTER (identifier);
> +  size_t len = strlen (name);
> +  char *prefix;
> +
> +  prefix = XALLOCAVEC (char, len + strlen (suffix) + 2);
> +  memcpy (prefix, name, len);
> +  prefix[len] = symbol_table::symbol_suffix_separator ();
> +  strcpy (prefix + len + 1, suffix);
> +  return get_identifier (prefix);
> +}
> +
> 
> FWIW I think I would phrase this as
> 
> char *str = concat (
>   IDENTIFIER_POINTER (identifier),
>   symbol_table::symbol_suffix_separator (),
>   suffix);
>   tree ret = get_identifier (str);
>   free (str);
>   return ret;
> 

Thanks for the suggestion Bernhard. I also found that the last
argument to concat has to be NULL and I can use ACONCAT to avoid the
explicit free and since symbol_table::symbol_suffix_separator returns
just one char I need to first put it into a string.

I also looked into re-writing numbered_clone_function_name in a
similar way, but before I got too far I found a small issue with my
suffixed_function_name: If I'm going to write an exact replacement for
numbered_clone_function_name then I need to also copy the
double-underscore prefixing behaviour done by ASM_PN_FORMAT (right?)
which is used by ASM_FORMAT_PRIVATE_NAME and write it as:

  char *separator = XALLOCAVEC (char, 2);
  separator[0] = symbol_table::symbol_suffix_separator ();
  separator[1] = 0;
  return get_identifier (ACONCAT ((
#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
    "__",
#endif
    IDENTIFIER_POINTER (identifier),
    separator,
    suffix,
    NULL)));

(I'm not sure if the formatting is correct)

However, then it's not exactly the same as the code that I'm also
trying to replace in cgraph_node::create_virtual_clone because it
doesn't add a double underscore if neither NO_DOT_IN_LABEL nor
NO_DOLLAR_IN_LABEL is defined. Is this just an omission that I should
fix by with my new function or was it indended that way and shouldn't
be changed?

Suggestions anyone?


- Michael
diff mbox series

Patch

From bab4d3da652f895aed877c952c5a0ab6de64239c Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Fri, 19 Oct 2018 16:38:02 -0400
Subject: [PATCH 4/4] There can be at most one .localalias clone per symbol.

gcc:
2018-10-19  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

       * symtab.c (symtab_node::noninterposable_alias): Use
         suffixed_function_name.
---
 gcc/symtab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git gcc/symtab.c gcc/symtab.c
index de65b65..92f0c95 100644
--- gcc/symtab.c
+++ gcc/symtab.c
@@ -1787,7 +1787,7 @@  symtab_node::noninterposable_alias (void)
   /* Otherwise create a new one.  */
   new_decl = copy_node (node->decl);
   DECL_DLLIMPORT_P (new_decl) = 0;
-  DECL_NAME (new_decl) = numbered_clone_function_name (node->decl, "localalias");
+  DECL_NAME (new_decl) = suffixed_function_name (DECL_ASSEMBLER_NAME (node->decl), "localalias");
   if (TREE_CODE (new_decl) == FUNCTION_DECL)
     DECL_STRUCT_FUNCTION (new_decl) = NULL;
   DECL_INITIAL (new_decl) = NULL;
-- 
2.7.4