public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
To: gcc-patches@gcc.gnu.org
Subject: Re: [PING v2][PATCH] Make function clone name numbering independent.
Date: Fri, 31 Aug 2018 18:49:00 -0000	[thread overview]
Message-ID: <078bf036-f02b-3bdb-545d-3f5ad67bc786@oracle.com> (raw)
In-Reply-To: <3167e521-aa5b-e47c-6d9b-956a1af2a886@oracle.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 4381 bytes --]

On 2018-08-13 07:58 PM, Michael Ploujnikov wrote:
> Ping and I've updated the patch since last time as follows:
> 
>   - unittest scans assembly rather than the constprop dump because its
>     forward changed
>   - unittests should handle different hosts where any of
>     NO_DOT_IN_LABEL, NO_DOLLAR_IN_LABEL or __USER_LABEL_PREFIX__ may
>     be defined
>   - not 100% it's safe to change DECL_NAME to DECL_ASSEMBLER_NAME in
>     cgraph_node::create_virtual_clone, but I've attempted to reduce
>     some code duplication
>   - lto-partition.c: privatize_symbol_name_1 *does* need numbered
>     names
>   - but cold sections don't
>   - Expecting an IDENTIFIER_NODE in clone_function_name_1 avoids
>     unreliable string pointer use as pointed out in the first review
>   - renamed clone_function_name_1 and clone_function_name to
>     numbered_clone_function_name_1 and numbered_clone_function_name to
>     clarify purpose and discourage future unintended uses
> 
> Also bootstrapped and regtested.

Ping.

I've done some more digging into the current uses of
numbered_clone_function_name and checked if any tests fail if I change
it to suffixed_function_name:

  - gcc/cgraphclones.c:  DECL_NAME (new_decl) = numbered_clone_function_name (thunk->decl, "artificial_thunk");
    - no new tests fail, inconclusive
    - and despite the comment on redirect_callee_duplicating_thunks
      about "one or more" duplicates it doesn't seem like
      duplicate_thunk_for_node would be called more than once for each
      node, assuming each node is named uniquely, but I'm far from an
      expert in this area
  - gcc/cgraphclones.c:  SET_DECL_ASSEMBLER_NAME (new_decl, numbered_clone_function_name (old_decl, suffix));
    - called by cgraph_node::create_virtual_clone, definitely needs it
    - this is where clones like .constprop come from
  - gcc/cgraphclones.c:  DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, suffix);
    - more tests fail
    - this is where clones like .simdclone come from
    - called by cgraph_node::create_version_clone_with_body, most likely needs it
  - gcc/config/rs6000/rs6000.c:  tree decl_name = numbered_clone_function_name (default_decl, "resolver");
    - doesn't look like this needs the numbering as there should only
      be one resolver per multi-version function, but need someone
      with an rs/6000 to confirm
  - gcc/lto/lto-partition.c:                                      numbered_clone_function_name_1 (identifier,
    - definitely needed for disambiguation, shown by unit tests failing
  - gcc/multiple_target.c:                                numbered_clone_function_name (node->decl,
    - create_dispatcher_calls says it only creates the dispatcher once
    - no new tests fail, inconclusive
  - gcc/multiple_target.c:                                    numbered_clone_function_name (node->decl,
    - I have a feeling this isn't necessary
    - no new tests fail, inconclusive
  - gcc/omp-expand.c:  DECL_NAME (kern_fndecl) = numbered_clone_function_name (kern_fndecl, "kernel");
    - no new tests fail, inconclusive
    - I didn't see (and couldn't figure out a way to get) any of the
      existing omp/acc tests actually exercise this codeptah
  - gcc/omp-low.c:  return numbered_clone_function_name (current_function_decl,
    - definitely needed based on
      gcc/testsuite/c-c++-common/goacc/kernels-loop-2.c and others
  - gcc/omp-simd-clone.c:      DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, "simdclone");
    - no tests fail, inconclusive
    - can definitely have more than one simdclone per function as above, but
      maybe not these external types
    - some tests, like declare-simd.c actually exercise this codepath,
      but I couldn't find the resulting .simdclone decl the the
      simdclone pass dump nor in any of the other dumps to verify
  - gcc/symtab.c:  DECL_NAME (new_decl) = numbered_clone_function_name (node->decl, "localalias");
    - no tests fail, inconclusive
    - my understanding of function_and_variable_visibility says that
      there can only be one per function so maybe this isn't; hubicka?

I'll add patches to switch these once someone with expertise in these
areas can confirm that the numbering isn't needed in the respective
decl names.


- Michael


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Make-function-assembly-more-independent.patch --]
[-- Type: text/x-patch; name="0001-Make-function-assembly-more-independent.patch", Size: 6060 bytes --]

From adde0632266d3f1b0540e09b9db931df4302d2bc Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Mon, 16 Jul 2018 12:55:49 -0400
Subject: [PATCH 1/4] Make function assembly more independent.

This changes clone_function_name such that clone names are based on a
per-function counter rather than a global one.

gcc:
2018-08-02  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

       Make function clone name numbering independent.
       * cgraphclones.c: Replace clone_fn_id_num with clone_fn_ids.
       (clone_function_name_1): Take an IDENTIFIER_NODE instead of a
       string. Use clone_fn_id_num.

lto:
2018-08-02  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

       * lto-partition.c (privatize_symbol_name_1): Pass a persistent
       identifier node to clone_function_name_1.

testsuite:
2018-08-02  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

       Clone id counters should be completely independent from one another.
       * gcc.dg/independent-cloneids-1.c: New test.
---
 gcc/cgraph.h                                  |  2 +-
 gcc/cgraphclones.c                            | 19 +++++++++-----
 gcc/lto/lto-partition.c                       |  5 ++--
 gcc/testsuite/gcc.dg/independent-cloneids-1.c | 38 +++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/independent-cloneids-1.c

diff --git gcc/cgraph.h gcc/cgraph.h
index a8b1b4c..fadc107 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2368,7 +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 clone_function_name_1 (const char *, const char *);
+tree clone_function_name_1 (tree identifier, const char *);
 tree clone_function_name (tree decl, const char *);
 
 void tree_function_versioning (tree, tree, vec<ipa_replace_map *, va_gc> *,
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 6e84a31..fb81fbd 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -512,14 +512,15 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count,
   return new_node;
 }
 
-static GTY(()) unsigned int clone_fn_id_num;
+static GTY(()) hash_map<const char *, unsigned> *clone_fn_ids;
 
-/* Return a new assembler name for a clone with SUFFIX of a decl named
-   NAME.  */
+/* Return a new assembler name for a numbered clone with SUFFIX of a
+   decl named IDENTIFIER.  */
 
 tree
-clone_function_name_1 (const char *name, const char *suffix)
+clone_function_name_1 (tree identifier, const char *suffix)
 {
+  const char *name = IDENTIFIER_POINTER (identifier);
   size_t len = strlen (name);
   char *tmp_name, *prefix;
 
@@ -527,7 +528,12 @@ clone_function_name_1 (const char *name, const char *suffix)
   memcpy (prefix, name, len);
   strcpy (prefix + len + 1, suffix);
   prefix[len] = symbol_table::symbol_suffix_separator ();
-  ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, clone_fn_id_num++);
+  /* Initialize the per-function counter hash table on first call */
+  if (!clone_fn_ids)
+    clone_fn_ids = hash_map<const char *, unsigned>::create_ggc (64);
+  unsigned int &suffix_counter = clone_fn_ids->get_or_insert(name);
+  ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, suffix_counter);
+  suffix_counter++;
   return get_identifier (tmp_name);
 }
 
@@ -536,8 +542,7 @@ clone_function_name_1 (const char *name, const char *suffix)
 tree
 clone_function_name (tree decl, const char *suffix)
 {
-  tree name = DECL_ASSEMBLER_NAME (decl);
-  return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+  return clone_function_name_1 (DECL_ASSEMBLER_NAME (decl), suffix);
 }
 
 
diff --git gcc/lto/lto-partition.c gcc/lto/lto-partition.c
index c7a5710..e3efa71 100644
--- gcc/lto/lto-partition.c
+++ gcc/lto/lto-partition.c
@@ -962,11 +962,12 @@ privatize_symbol_name_1 (symtab_node *node, tree decl)
   if (must_not_rename (node, name))
     return false;
 
-  name = maybe_rewrite_identifier (name);
+  tree identifier = get_identifier (maybe_rewrite_identifier (name));
   symtab->change_decl_assembler_name (decl,
-				      clone_function_name_1 (name,
+				      clone_function_name_1 (identifier,
 							     "lto_priv"));
 
+  name = IDENTIFIER_POINTER (identifier);
   if (node->lto_file_data)
     lto_record_renamed_decl (node->lto_file_data, name,
 			     IDENTIFIER_POINTER
diff --git gcc/testsuite/gcc.dg/independent-cloneids-1.c gcc/testsuite/gcc.dg/independent-cloneids-1.c
new file mode 100644
index 0000000..3203895
--- /dev/null
+++ gcc/testsuite/gcc.dg/independent-cloneids-1.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fipa-cp -fipa-cp-clone"  } */
+
+extern int printf (const char *, ...);
+
+static int __attribute__ ((noinline))
+foo (int arg)
+{
+  return 7 * arg;
+}
+
+static int __attribute__ ((noinline))
+bar (int arg)
+{
+  return arg * arg;
+}
+
+int
+baz (int arg)
+{
+  printf("%d\n", bar (3));
+  printf("%d\n", bar (4));
+  printf("%d\n", foo (5));
+  printf("%d\n", foo (6));
+  /* adding or removing the following call should not affect foo
+     function's clone numbering */
+  printf("%d\n", bar (7));
+  return foo (8);
+}
+
+/* { dg-final { scan-assembler-times {(?n)\m_*bar[.$_]constprop[.$_]0:} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)\m_*bar[.$_]constprop[.$_]1:} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)\m_*bar[.$_]constprop[.$_]2:} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)\m_*foo[.$_]constprop[.$_]0:} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)\m_*foo[.$_]constprop[.$_]1:} 1 } } */
+/* { dg-final { scan-assembler-times {(?n)\m_*foo[.$_]constprop[.$_]2:} 1 } } */
+/* { dg-final { scan-assembler-not {(?n)\m_*foo[.$_]constprop[.$_]3:} } } */
+/* { dg-final { scan-assembler-not {(?n)\m_*foo[.$_]constprop[.$_]4:} } } */
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-Rename-clone_function_name_1-and-clone_function_name.patch --]
[-- Type: text/x-patch; name="0002-Rename-clone_function_name_1-and-clone_function_name.patch", Size: 8895 bytes --]

From bbea3705e59e23289cdba72da143f89b3a08b78c 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 2/4] Rename clone_function_name_1 and clone_function_name to
 clarify usage.

gcc:
2018-08-02  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         | 14 +++++++-------
 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, 21 insertions(+), 21 deletions(-)

diff --git gcc/cgraph.h gcc/cgraph.h
index fadc107..322df1e 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 (tree identifier, const char *);
-tree clone_function_name (tree decl, const char *);
+tree numbered_clone_function_name_1 (tree identifier, 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 fb81fbd..08fc922 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);
@@ -518,7 +518,7 @@ static GTY(()) hash_map<const char *, unsigned> *clone_fn_ids;
    decl named IDENTIFIER.  */
 
 tree
-clone_function_name_1 (tree identifier, const char *suffix)
+numbered_clone_function_name_1 (tree identifier, const char *suffix)
 {
   const char *name = IDENTIFIER_POINTER (identifier);
   size_t len = strlen (name);
@@ -537,12 +537,12 @@ clone_function_name_1 (tree identifier, 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 with numbered SUFFIX.  */
 
 tree
-clone_function_name (tree decl, const char *suffix)
+numbered_clone_function_name (tree decl, const char *suffix)
 {
-  return clone_function_name_1 (DECL_ASSEMBLER_NAME (decl), suffix);
+  return numbered_clone_function_name_1 (DECL_ASSEMBLER_NAME (decl), suffix);
 }
 
 
@@ -590,7 +590,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,
@@ -969,7 +969,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 e3efa71..c872325 100644
--- gcc/lto/lto-partition.c
+++ gcc/lto/lto-partition.c
@@ -964,8 +964,8 @@ privatize_symbol_name_1 (symtab_node *node, tree decl)
 
   tree identifier = get_identifier (maybe_rewrite_identifier (name));
   symtab->change_decl_assembler_name (decl,
-				      clone_function_name_1 (identifier,
-							     "lto_priv"));
+				      numbered_clone_function_name_1 (identifier,
+								      "lto_priv"));
 
   name = IDENTIFIER_POINTER (identifier);
   if (node->lto_file_data)
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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-Cold-sections-don-t-need-to-be-numbered.patch --]
[-- Type: text/x-patch; name="0003-Cold-sections-don-t-need-to-be-numbered.patch", Size: 5781 bytes --]

From f6a5f30d8975214c564ec3740163b6156ca2ad97 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 3/4] Cold sections don't need to be numbered.

gcc:
2018-08-02  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-08-02  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 322df1e..836ceb1 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 decl, const char *);
 tree numbered_clone_function_name_1 (tree identifier, const char *);
 tree numbered_clone_function_name (tree decl, const char *);
 
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 08fc922..784f1df 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -514,6 +514,22 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count,
 
 static GTY(()) hash_map<const char *, unsigned> *clone_fn_ids;
 
+/* Return a new assembler name for a clone of DECL with SUFFIX.  */
+
+tree
+suffixed_function_name (tree decl, const char *suffix)
+{
+  const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
+  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 a new assembler name for a numbered clone with SUFFIX of a
    decl named IDENTIFIER.  */
 
diff --git gcc/final.c gcc/final.c
index 842e5e0..ac83417 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 (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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.5: 0004-Use-suffixed_function_name-in-cgraph_node-create_vir.patch --]
[-- Type: text/x-patch; name="0004-Use-suffixed_function_name-in-cgraph_node-create_vir.patch", Size: 1722 bytes --]

From d96bedcf51c289f3c80a12a681a7831d9e287874 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 4/4] Use suffixed_function_name in
 cgraph_node::create_virtual_clone.

gcc:
2018-08-02  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 784f1df..876b7fa 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -576,9 +576,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);
@@ -600,12 +599,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 (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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  parent reply	other threads:[~2018-08-31 18:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-16 19:38 [PATCH] " Michael Ploujnikov
2018-07-17  6:10 ` Bernhard Reutner-Fischer
2018-07-17 10:03   ` Richard Biener
2018-07-17 20:25     ` Michael Ploujnikov
2018-07-20  2:49       ` Michael Ploujnikov
2018-07-20 10:05         ` Richard Biener
2018-07-24 13:57           ` Michael Ploujnikov
2018-07-26 17:27             ` Michael Ploujnikov
2018-07-31 17:40               ` Michael Ploujnikov
2018-08-01 10:38                 ` Richard Biener
2018-08-02 19:05                   ` Michael Ploujnikov
2018-08-13 23:58                     ` [PING][PATCH] " Michael Ploujnikov
2018-08-20 19:47                       ` Jeff Law
2018-08-31 18:49                       ` Michael Ploujnikov [this message]
2018-09-03 10:02                         ` [PING v2][PATCH] " Martin Jambor
2018-09-03 11:08                           ` Richard Biener
2018-09-03 13:15                             ` Martin Jambor
2018-09-05  3:24                               ` Michael Ploujnikov
2018-11-28 21:09                                 ` [PATCH v3] " Michael Ploujnikov
2018-11-28 22:49                                   ` Segher Boessenkool
2018-11-29 14:13                                     ` Michael Ploujnikov
2018-11-30  7:26                                   ` Richard Biener
2018-11-30 21:11                                     ` Michael Ploujnikov
2018-12-01 16:31                                       ` H.J. Lu
2018-12-03 17:00                                         ` Michael Ploujnikov
2018-12-04  3:06                                           ` Michael Ploujnikov
2018-12-04 14:07                                             ` Jan Hubicka
2018-09-05  3:32                           ` [PING v2][PATCH] " Michael Ploujnikov
2018-12-04 12:48                             ` Martin Jambor
2018-12-04 13:22                               ` Michael Ploujnikov

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=078bf036-f02b-3bdb-545d-3f5ad67bc786@oracle.com \
    --to=michael.ploujnikov@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).