This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Avoid unnecessarily numbered clone symbols


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
From 0bbcf3b8c20498f4d861e088ff7ab38e2a43800b Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Tue, 7 Aug 2018 20:36:53 -0400
Subject: [PATCH 1/4] Rename clone_function_name_1 and clone_function_name to
 clarify usage.

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

       * gcc/cgraph.h: Rename clone_function_name_1 to
         numbered_clone_function_name_1. Rename clone_function_name to
         numbered_clone_function_name.
       * cgraphclones.c: Ditto.
       * config/rs6000/rs6000.c: Ditto.
       * lto/lto-partition.c: Ditto.
       * multiple_target.c: Ditto.
       * omp-expand.c: Ditto.
       * omp-low.c: Ditto.
       * omp-simd-clone.c: Ditto.
       * symtab.c: Ditto.
---
 gcc/cgraph.h               |  4 ++--
 gcc/cgraphclones.c         | 20 +++++++++++---------
 gcc/config/rs6000/rs6000.c |  2 +-
 gcc/lto/lto-partition.c    |  4 ++--
 gcc/multiple_target.c      |  8 ++++----
 gcc/omp-expand.c           |  2 +-
 gcc/omp-low.c              |  4 ++--
 gcc/omp-simd-clone.c       |  2 +-
 gcc/symtab.c               |  2 +-
 9 files changed, 25 insertions(+), 23 deletions(-)

diff --git gcc/cgraph.h gcc/cgraph.h
index a8b1b4c..3583f7e 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2368,8 +2368,8 @@ basic_block init_lowered_empty_function (tree, bool, profile_count);
 tree thunk_adjust (gimple_stmt_iterator *, tree, bool, HOST_WIDE_INT, tree);
 /* In cgraphclones.c  */
 
-tree clone_function_name_1 (const char *, const char *);
-tree clone_function_name (tree decl, const char *);
+tree numbered_clone_function_name_1 (const char *, const char *);
+tree numbered_clone_function_name (tree decl, const char *);
 
 void tree_function_versioning (tree, tree, vec<ipa_replace_map *, va_gc> *,
 			       bool, bitmap, bool, bitmap, basic_block);
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 6e84a31..cdb183d 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -316,7 +316,7 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node)
   gcc_checking_assert (!DECL_RESULT (new_decl));
   gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
 
-  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
+  DECL_NAME (new_decl) = numbered_clone_function_name (thunk->decl, "artificial_thunk");
   SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
 
   new_thunk = cgraph_node::create (new_decl);
@@ -514,11 +514,11 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count,
 
 static GTY(()) unsigned int clone_fn_id_num;
 
-/* Return a new assembler name for a clone with SUFFIX of a decl named
-   NAME.  */
+/* Return NAME appended with string SUFFIX and a unique unspecified
+   number.  */
 
 tree
-clone_function_name_1 (const char *name, const char *suffix)
+numbered_clone_function_name_1 (const char *name, const char *suffix)
 {
   size_t len = strlen (name);
   char *tmp_name, *prefix;
@@ -531,13 +531,15 @@ clone_function_name_1 (const char *name, const char *suffix)
   return get_identifier (tmp_name);
 }
 
-/* Return a new assembler name for a clone of DECL with SUFFIX.  */
+/* Return a new assembler name for a clone of DECL. Apart from the
+   string SUFFIX, the new name will end with a unique unspecified
+   number.  */
 
 tree
-clone_function_name (tree decl, const char *suffix)
+numbered_clone_function_name (tree decl, const char *suffix)
 {
   tree name = DECL_ASSEMBLER_NAME (decl);
-  return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+  return numbered_clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
 }
 
 
@@ -585,7 +587,7 @@ cgraph_node::create_virtual_clone (vec<cgraph_edge *> redirect_callers,
   strcpy (name + len + 1, suffix);
   name[len] = '.';
   DECL_NAME (new_decl) = get_identifier (name);
-  SET_DECL_ASSEMBLER_NAME (new_decl, clone_function_name (old_decl, suffix));
+  SET_DECL_ASSEMBLER_NAME (new_decl, numbered_clone_function_name (old_decl, suffix));
   SET_DECL_RTL (new_decl, NULL);
 
   new_node = create_clone (new_decl, count, false,
@@ -964,7 +966,7 @@ cgraph_node::create_version_clone_with_body
       = build_function_decl_skip_args (old_decl, args_to_skip, skip_return);
 
   /* Generate a new name for the new version. */
-  DECL_NAME (new_decl) = clone_function_name (old_decl, suffix);
+  DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, suffix);
   SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
   SET_DECL_RTL (new_decl, NULL);
 
diff --git gcc/config/rs6000/rs6000.c gcc/config/rs6000/rs6000.c
index c2af4b8..f9c9936 100644
--- gcc/config/rs6000/rs6000.c
+++ gcc/config/rs6000/rs6000.c
@@ -36512,7 +36512,7 @@ make_resolver_func (const tree default_decl,
 {
   /* Make the resolver function static.  The resolver function returns
      void *.  */
-  tree decl_name = clone_function_name (default_decl, "resolver");
+  tree decl_name = numbered_clone_function_name (default_decl, "resolver");
   const char *resolver_name = IDENTIFIER_POINTER (decl_name);
   tree type = build_function_type_list (ptr_type_node, NULL_TREE);
   tree decl = build_fn_decl (resolver_name, type);
diff --git gcc/lto/lto-partition.c gcc/lto/lto-partition.c
index c7a5710..dc3b950 100644
--- gcc/lto/lto-partition.c
+++ gcc/lto/lto-partition.c
@@ -964,8 +964,8 @@ privatize_symbol_name_1 (symtab_node *node, tree decl)
 
   name = maybe_rewrite_identifier (name);
   symtab->change_decl_assembler_name (decl,
-				      clone_function_name_1 (name,
-							     "lto_priv"));
+				      numbered_clone_function_name_1 (name,
+								      "lto_priv"));
 
   if (node->lto_file_data)
     lto_record_renamed_decl (node->lto_file_data, name,
diff --git gcc/multiple_target.c gcc/multiple_target.c
index a1fe09a..592e7b9 100644
--- gcc/multiple_target.c
+++ gcc/multiple_target.c
@@ -162,8 +162,8 @@ create_dispatcher_calls (struct cgraph_node *node)
     }
 
   symtab->change_decl_assembler_name (node->decl,
-				      clone_function_name (node->decl,
-							   "default"));
+				      numbered_clone_function_name (node->decl,
+								    "default"));
 
   /* FIXME: copy of cgraph_node::make_local that should be cleaned up
 	    in next stage1.  */
@@ -312,8 +312,8 @@ create_target_clone (cgraph_node *node, bool definition, char *name)
       new_node = cgraph_node::get_create (new_decl);
       /* Generate a new name for the new version.  */
       symtab->change_decl_assembler_name (new_node->decl,
-					  clone_function_name (node->decl,
-							       name));
+					  numbered_clone_function_name (node->decl,
+									name));
     }
   return new_node;
 }
diff --git gcc/omp-expand.c gcc/omp-expand.c
index d2a77c0..3aeaae0 100644
--- gcc/omp-expand.c
+++ gcc/omp-expand.c
@@ -7625,7 +7625,7 @@ grid_expand_target_grid_body (struct omp_region *target)
     expand_omp (gpukernel->inner);
 
   tree kern_fndecl = copy_node (orig_child_fndecl);
-  DECL_NAME (kern_fndecl) = clone_function_name (kern_fndecl, "kernel");
+  DECL_NAME (kern_fndecl) = numbered_clone_function_name (kern_fndecl, "kernel");
   SET_DECL_ASSEMBLER_NAME (kern_fndecl, DECL_NAME (kern_fndecl));
   tree tgtblock = gimple_block (tgt_stmt);
   tree fniniblock = make_node (BLOCK);
diff --git gcc/omp-low.c gcc/omp-low.c
index 843c66f..6748e05 100644
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -1531,8 +1531,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
 static tree
 create_omp_child_function_name (bool task_copy)
 {
-  return clone_function_name (current_function_decl,
-			      task_copy ? "_omp_cpyfn" : "_omp_fn");
+  return numbered_clone_function_name (current_function_decl,
+				       task_copy ? "_omp_cpyfn" : "_omp_fn");
 }
 
 /* Return true if CTX may belong to offloaded code: either if current function
diff --git gcc/omp-simd-clone.c gcc/omp-simd-clone.c
index b15adf0..806c486 100644
--- gcc/omp-simd-clone.c
+++ gcc/omp-simd-clone.c
@@ -444,7 +444,7 @@ simd_clone_create (struct cgraph_node *old_node)
     {
       tree old_decl = old_node->decl;
       tree new_decl = copy_node (old_node->decl);
-      DECL_NAME (new_decl) = clone_function_name (old_decl, "simdclone");
+      DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, "simdclone");
       SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
       SET_DECL_RTL (new_decl, NULL);
       DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
diff --git gcc/symtab.c gcc/symtab.c
index c5464cb..de65b65 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) = clone_function_name (node->decl, "localalias");
+  DECL_NAME (new_decl) = numbered_clone_function_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

From daa2b38cf927a5a73f0228f2dac89fe018fa1014 Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Tue, 7 Aug 2018 20:47:12 -0400
Subject: [PATCH 2/4] Cold sections don't need to be numbered.

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

       * cgraph.h: Add suffixed_function_name.
       * cgraphclones.c: Ditto.
       * final.c (final_scan_insn_1): Use it.

testsuite:
2018-10-19  Michael Ploujnikov  <michael.ploujnikov@oracle.com>
       * gcc.dg/tree-prof/cold_partition_label.c: Update for new cold
         section names.
       * gcc.dg/tree-prof/section-attr-1.c: ditto.
       * gcc.dg/tree-prof/section-attr-2.c: ditto.
       * gcc.dg/tree-prof/section-attr-3.c: ditto.
---
 gcc/cgraph.h                                          |  1 +
 gcc/cgraphclones.c                                    | 16 ++++++++++++++++
 gcc/final.c                                           |  2 +-
 gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c |  4 ++--
 gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c       |  2 +-
 gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c       |  2 +-
 gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c       |  2 +-
 7 files changed, 23 insertions(+), 6 deletions(-)

diff --git gcc/cgraph.h gcc/cgraph.h
index 3583f7e..f68a14b 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2368,6 +2368,7 @@ basic_block init_lowered_empty_function (tree, bool, profile_count);
 tree thunk_adjust (gimple_stmt_iterator *, tree, bool, HOST_WIDE_INT, tree);
 /* In cgraphclones.c  */
 
+tree suffixed_function_name (tree identifier, const char *);
 tree numbered_clone_function_name_1 (const char *, const char *);
 tree numbered_clone_function_name (tree decl, const char *);
 
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index cdb183d..8419b6b 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -514,6 +514,22 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count,
 
 static GTY(()) unsigned int clone_fn_id_num;
 
+/* 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);
+}
+
 /* Return NAME appended with string SUFFIX and a unique unspecified
    number.  */
 
diff --git gcc/final.c gcc/final.c
index 842e5e0..622b43c 100644
--- gcc/final.c
+++ gcc/final.c
@@ -2203,7 +2203,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 
 	  if (in_cold_section_p)
 	    cold_function_name
-	      = clone_function_name (current_function_decl, "cold");
+	      = suffixed_function_name (DECL_ASSEMBLER_NAME (current_function_decl), "cold");
 
 	  if (dwarf2out_do_frame ())
 	    {
diff --git gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
index 52518cf..450308d 100644
--- gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
+++ gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
@@ -37,6 +37,6 @@ main (int argc, char *argv[])
   return 0;
 }
 
-/* { dg-final-use { scan-assembler "foo\[._\]+cold\[\._\]+0" { target *-*-linux* *-*-gnu* } } } */
-/* { dg-final-use { scan-assembler "size\[ \ta-zA-Z0-0\]+foo\[._\]+cold\[\._\]+0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "foo\[._\]+cold" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "size\[ \ta-zA-Z0-0\]+foo\[._\]+cold" { target *-*-linux* *-*-gnu* } } } */
 /* { dg-final-use { scan-tree-dump-not "Invalid sum" "optimized"} } */
diff --git gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
index ee6662e..1bb8cd9 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
@@ -42,4 +42,4 @@ foo (int path)
     }
 }
 
-/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold\.0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
diff --git gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
index 898a395..31be7eb 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
@@ -41,4 +41,4 @@ foo (int path)
     }
 }
 
-/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold\.0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
diff --git gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
index 36829dc..0e64001 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
@@ -42,4 +42,4 @@ foo (int path)
     }
 }
 
-/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold\.0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
-- 
2.7.4

From b45b3e6ed9666f98dcb004b275afcb799fda411f Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Mon, 13 Aug 2018 13:59:46 -0400
Subject: [PATCH 3/4] Use suffixed_function_name in
 cgraph_node::create_virtual_clone.

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

       * cgraphclones.c (cgraph_node::create_virtual_clone): Use
         suffixed_function_name.
---
 gcc/cgraphclones.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 8419b6b..0dfc2c2 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -573,9 +573,8 @@ cgraph_node::create_virtual_clone (vec<cgraph_edge *> redirect_callers,
   tree old_decl = decl;
   cgraph_node *new_node = NULL;
   tree new_decl;
-  size_t len, i;
+  size_t i;
   ipa_replace_map *map;
-  char *name;
 
   gcc_checking_assert (local.versionable);
   gcc_assert (local.can_change_signature || !args_to_skip);
@@ -597,12 +596,7 @@ cgraph_node::create_virtual_clone (vec<cgraph_edge *> redirect_callers,
      sometimes storing only clone decl instead of original.  */
 
   /* Generate a new name for the new version. */
-  len = IDENTIFIER_LENGTH (DECL_NAME (old_decl));
-  name = XALLOCAVEC (char, len + strlen (suffix) + 2);
-  memcpy (name, IDENTIFIER_POINTER (DECL_NAME (old_decl)), len);
-  strcpy (name + len + 1, suffix);
-  name[len] = '.';
-  DECL_NAME (new_decl) = get_identifier (name);
+  DECL_NAME (new_decl) = suffixed_function_name (DECL_NAME (old_decl), suffix);
   SET_DECL_ASSEMBLER_NAME (new_decl, numbered_clone_function_name (old_decl, suffix));
   SET_DECL_RTL (new_decl, NULL);
 
-- 
2.7.4

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

Attachment: signature.asc
Description: OpenPGP digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]