* Avoid unnecessarily numbered clone symbols
@ 2018-10-20 3:29 Michael Ploujnikov
2018-10-20 15:42 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-20 3:29 UTC (permalink / raw)
To: GCC Patches; +Cc: Sriraman Tallam, mliska
[-- Attachment #1.1.1: Type: text/plain, Size: 672 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Rename-clone_function_name_1-and-clone_function_name.patch --]
[-- Type: text/x-patch; name="0001-Rename-clone_function_name_1-and-clone_function_name.patch", Size: 9165 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.3: 0002-Cold-sections-don-t-need-to-be-numbered.patch --]
[-- Type: text/x-patch; name="0002-Cold-sections-don-t-need-to-be-numbered.patch", Size: 5758 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-Use-suffixed_function_name-in-cgraph_node-create_vir.patch --]
[-- Type: text/x-patch; name="0003-Use-suffixed_function_name-in-cgraph_node-create_vir.patch", Size: 1734 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.5: 0004-There-can-be-at-most-one-.localalias-clone-per-symbo.patch --]
[-- Type: text/x-patch; name="0004-There-can-be-at-most-one-.localalias-clone-per-symbo.patch", Size: 1105 bytes --]
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Avoid unnecessarily numbered clone symbols
2018-10-20 3:29 Avoid unnecessarily numbered clone symbols Michael Ploujnikov
@ 2018-10-20 15:42 ` Bernhard Reutner-Fischer
2018-10-21 8:06 ` Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Bernhard Reutner-Fischer @ 2018-10-20 15:42 UTC (permalink / raw)
To: gcc-patches, Michael Ploujnikov, GCC Patches; +Cc: Sriraman Tallam, mliska
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;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Avoid unnecessarily numbered clone symbols
2018-10-20 15:42 ` Bernhard Reutner-Fischer
@ 2018-10-21 8:06 ` Michael Ploujnikov
2018-10-22 6:14 ` [PATCH v2] " Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-21 8:06 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Sriraman Tallam, mliska
[-- Attachment #1.1: Type: text/plain, Size: 2471 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] Avoid unnecessarily numbered clone symbols
2018-10-21 8:06 ` Michael Ploujnikov
@ 2018-10-22 6:14 ` Michael Ploujnikov
2018-10-23 16:31 ` [PATCH v3] " Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-22 6:14 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Sriraman Tallam, mliska
[-- Attachment #1.1.1: Type: text/plain, Size: 643 bytes --]
Continuing from https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01258.html
Fixed up the code after the change to concat suggested by Bernhard
Reutner.
Outstanding question still remains:
To write an exact replacement for numbered_clone_function_name (apart
from the numbering) I also need to copy the double underscore
prefixing behaviour done by ASM_PN_FORMAT (right?) which is used by
ASM_FORMAT_PRIVATE_NAME. Does that mean that I can't use my
suffixed_function_name to replace the very similar looking code in
cgraph_node::create_virtual_clone? Or is it just missing the double
underscore prefix by mistake?
- Michael
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Rename-clone_function_name_1-and-clone_function_name.patch --]
[-- Type: text/x-patch; name="0001-Rename-clone_function_name_1-and-clone_function_name.patch", Size: 9206 bytes --]
From 74435e1d8c5984eaee766d7940eeffbe565fcc2e 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 | 22 +++++++++++++---------
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 | 3 ++-
9 files changed, 28 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..bc59dc2 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -316,7 +316,8 @@ 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 +515,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 +532,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 +588,8 @@ 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 +968,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..de4f299 100644
--- gcc/symtab.c
+++ gcc/symtab.c
@@ -1787,7 +1787,8 @@ 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.3: 0002-Cold-sections-don-t-need-to-be-numbered.patch --]
[-- Type: text/x-patch; name="0002-Cold-sections-don-t-need-to-be-numbered.patch", Size: 6073 bytes --]
From 4ff50b97b96e0a61e6bf2fc6c78cc0d325a19be6 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 | 26 ++++++++++++++++++++++
gcc/final.c | 3 ++-
.../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, 34 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 bc59dc2..f7751ea 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -515,6 +515,32 @@ 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)
+{
+ /* Since this is being used in the same places as
+ * numbered_clone_function_name it needs to start the string with
+ * leading underscores if the target machine requires it, just like
+ * ASM_PN_FORMAT. */
+ char *separator = XALLOCAVEC (char, 2);
+ separator[0] = symbol_table::symbol_suffix_separator ();
+ separator[1] = 0;
+#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
+ const char *prefix = "__";
+#else
+ const char *prefix = "";
+#endif
+ char *result = ACONCAT ((
+ prefix,
+ IDENTIFIER_POINTER (identifier),
+ separator,
+ suffix,
+ (char*)0));
+ return get_identifier(result);
+}
+
/* Return NAME appended with string SUFFIX and a unique unspecified
number. */
diff --git gcc/final.c gcc/final.c
index 842e5e0..6ca8085 100644
--- gcc/final.c
+++ gcc/final.c
@@ -2203,7 +2203,8 @@ 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-Use-suffixed_function_name-in-cgraph_node-create_vir.patch --]
[-- Type: text/x-patch; name="0003-Use-suffixed_function_name-in-cgraph_node-create_vir.patch", Size: 1854 bytes --]
From 0de23451dc96ad2eef4707c5f02d33e327070fca 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 | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index f7751ea..edf5cfd 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -584,9 +584,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);
@@ -608,14 +607,9 @@ 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));
+ numbered_clone_function_name (old_decl, suffix));
SET_DECL_RTL (new_decl, NULL);
new_node = create_clone (new_decl, count, false,
--
2.7.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.5: 0004-There-can-be-at-most-one-.localalias-clone-per-symbo.patch --]
[-- Type: text/x-patch; name="0004-There-can-be-at-most-one-.localalias-clone-per-symbo.patch", Size: 1133 bytes --]
From ff92d9da5c7c0af2c8be09af51a1125b5db5b92b 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 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git gcc/symtab.c gcc/symtab.c
index de4f299..bd5a0dc 100644
--- gcc/symtab.c
+++ gcc/symtab.c
@@ -1787,8 +1787,8 @@ 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] Avoid unnecessarily numbered clone symbols
2018-10-22 6:14 ` [PATCH v2] " Michael Ploujnikov
@ 2018-10-23 16:31 ` Michael Ploujnikov
2018-10-26 4:39 ` [PATCH v4] Avoid unnecessarily numbering cloned symbols Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-23 16:31 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Sriraman Tallam, mliska, ebotcazou
[-- Attachment #1.1.1: Type: text/plain, Size: 895 bytes --]
On 2018-10-21 09:14 PM, Michael Ploujnikov wrote:
> Continuing from https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01258.html
>
> Fixed up the code after the change to concat suggested by Bernhard
> Reutner.
>
> Outstanding question still remains:
>
> To write an exact replacement for numbered_clone_function_name (apart
> from the numbering) I also need to copy the double underscore
> prefixing behaviour done by ASM_PN_FORMAT (right?) which is used by
> ASM_FORMAT_PRIVATE_NAME. Does that mean that I can't use my
> suffixed_function_name to replace the very similar looking code in
> cgraph_node::create_virtual_clone? Or is it just missing the double
> underscore prefix by mistake?
I found https://gcc.gnu.org/ml/gcc-patches/2013-08/msg01028.html which
answered my question so now I'm just simplifying
cgraph_node::create_virtual_clone with ACONCAT.
- Michael
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Rename-clone_function_name_1-and-clone_function_name.patch --]
[-- Type: text/x-patch; name="0001-Rename-clone_function_name_1-and-clone_function_name.patch", Size: 9207 bytes --]
From 383c64faa956c8b06e680808ef275acb6a746158 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-23 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 | 22 +++++++++++++---------
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 | 3 ++-
9 files changed, 28 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..4395806 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -316,7 +316,8 @@ 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 +515,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 +532,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 +588,8 @@ 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 +968,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..de4f299 100644
--- gcc/symtab.c
+++ gcc/symtab.c
@@ -1787,7 +1787,8 @@ 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.3: 0002-Cold-sections-don-t-need-to-be-numbered.patch --]
[-- Type: text/x-patch; name="0002-Cold-sections-don-t-need-to-be-numbered.patch", Size: 6115 bytes --]
From 6ae1d7c955e2f541a1009400db8cc609a7b9ff73 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-23 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-23 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 | 27 ++++++++++++++++++++++
gcc/final.c | 3 ++-
.../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, 35 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 4395806..bb8be79 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -515,6 +515,33 @@ 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. Uses
+ platform-specific separators. */
+
+tree
+suffixed_function_name (tree identifier, const char *suffix)
+{
+ /* Since this is being used in the same places as
+ * numbered_clone_function_name it needs to start the string with
+ * leading underscores if the target machine requires it, just like
+ * ASM_PN_FORMAT. */
+ char *separator = XALLOCAVEC (char, 2);
+ separator[0] = symbol_table::symbol_suffix_separator ();
+ separator[1] = 0;
+#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
+ const char *prefix = "__";
+#else
+ const char *prefix = "";
+#endif
+ char *result = ACONCAT ((
+ prefix,
+ IDENTIFIER_POINTER (identifier),
+ separator,
+ suffix,
+ (char*)0));
+ return get_identifier (result);
+}
+
/* Return NAME appended with string SUFFIX and a unique unspecified
number. */
diff --git gcc/final.c gcc/final.c
index 842e5e0..6ca8085 100644
--- gcc/final.c
+++ gcc/final.c
@@ -2203,7 +2203,8 @@ 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: 0003-There-can-be-at-most-one-.localalias-clone-per-symbo.patch --]
[-- Type: text/x-patch; name="0003-There-can-be-at-most-one-.localalias-clone-per-symbo.patch", Size: 1133 bytes --]
From 42f7d5cc5348565fa7d866b20d125ce74c14e1e4 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 3/4] There can be at most one .localalias clone per symbol.
gcc:
2018-10-23 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* symtab.c (symtab_node::noninterposable_alias): Use
suffixed_function_name.
---
gcc/symtab.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git gcc/symtab.c gcc/symtab.c
index de4f299..bd5a0dc 100644
--- gcc/symtab.c
+++ gcc/symtab.c
@@ -1787,8 +1787,8 @@ 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.5: 0004-Use-ACONCAT-in-cgraph_node-create_virtual_clone.patch --]
[-- Type: text/x-patch; name="0004-Use-ACONCAT-in-cgraph_node-create_virtual_clone.patch", Size: 1710 bytes --]
From 3efc4e1d96a382ed00f7e529d1f5a5a844c7fa69 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 ACONCAT in cgraph_node::create_virtual_clone.
gcc:
2018-10-23 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* cgraphclones.c (cgraph_node::create_virtual_clone): Use
ACONCAT for the DECL_NAME.
---
gcc/cgraphclones.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index bb8be79..c98b0ed 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -585,9 +585,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);
@@ -609,11 +608,8 @@ 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] = '.';
+ char *name = ACONCAT ((IDENTIFIER_POINTER (DECL_NAME (old_decl)),
+ ".", suffix, (char *)0));
DECL_NAME (new_decl) = get_identifier (name);
SET_DECL_ASSEMBLER_NAME (new_decl,
numbered_clone_function_name (old_decl, suffix));
--
2.7.4
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-23 16:31 ` [PATCH v3] " Michael Ploujnikov
@ 2018-10-26 4:39 ` Michael Ploujnikov
2018-10-26 8:56 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-26 4:39 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Sriraman Tallam, mliska, ebotcazou
[-- Attachment #1.1.1: Type: text/plain, Size: 1295 bytes --]
I've taken the advice from a discussion on IRC and re-wrote the patch
with more uniform function names and using overloading.
I think this function accomplished the following goals:
- remove clone numbering where it's not needed:
final.c:final_scan_insn_1 and
symtab.c:simd_symtab_node::noninterposable_alias.
- name and document the clone naming API such that future users won't
accidentally use the numbering when it's not necessary; if you need
numbering then you need to explicitly ask for it with the right
function
- provide a new function that allows users to specify a clone number
explicitly as an argument
My thoughts for future improvements:
- It's a bit unfortunate that lto-partition.c:privatize_symbol_name_1
has to break the decl abstraction and pass in a string that it
created into what I would consider the implementation-detail
function. The best way I can think of to make it uniform with the
rest of the users is to have it create a new empty decl with
DECL_ASSEMBLER_NAME set to the new string
- It's unfortunate that I have to duplicate the separator
concatenation in the numberless clone_function_name, but I think it
has to be like that unless ASM_FORMAT_PRIVATE_NAME making the
number optional.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Avoid-unnecessarily-numbering-cloned-symbols.patch --]
[-- Type: text/x-patch; name="0001-Avoid-unnecessarily-numbering-cloned-symbols.patch", Size: 15137 bytes --]
From 0df79d0ac6a9891b344f988c7157a54cebbc1cb8 Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Thu, 25 Oct 2018 13:16:36 -0400
Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
gcc/ChangeLog:
2018-10-25 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only. (clone_function_name):
Renamed to clone_function_name_numbered to be explicit about
numbering. (clone_function_name): New two-argument function
that does not number its output. (clone_function_name): New
three-argument function that takes a number to append to its
output.
* cgraphclones.c (duplicate_thunk_for_node):
(clone_function_name_1): Renamed.
(clone_function_name_numbered): Two new functions.
(clone_function_name): Improved documentation.
(cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
* config/rs6000/rs6000.c (make_resolver_func): Ditto.
* final.c (final_scan_insn_1): Use the new clone_function_name
without numbering.
* multiple_target.c (create_dispatcher_calls): Ditto.
(create_target_clone): Ditto.
* omp-expand.c (grid_expand_target_grid_body): Ditto.
* omp-low.c (create_omp_child_function_name): Ditto.
* omp-simd-clone.c (simd_clone_create): Ditto.
* symtab.c (simd_symtab_node::noninterposable_alias): Use the
new clone_function_name without numbering.
gcc/lto/ChangeLog:
2018-10-25 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* lto-partition.c (privatize_symbol_name_1): Use
clone_function_name_numbered.
gcc/testsuite/ChangeLog:
2018-10-25 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* gcc.dg/tree-prof/cold_partition_label.c: Update for cold
section names without numbers.
* 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 | 7 ++-
gcc/cgraphclones.c | 66 ++++++++++++++++++----
gcc/config/rs6000/rs6000.c | 2 +-
gcc/lto/lto-partition.c | 4 +-
gcc/multiple_target.c | 8 +--
gcc/omp-expand.c | 3 +-
gcc/omp-low.c | 4 +-
gcc/omp-simd-clone.c | 3 +-
.../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 +-
12 files changed, 79 insertions(+), 28 deletions(-)
diff --git gcc/cgraph.h gcc/cgraph.h
index a8b1b4c..971065d 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2368,8 +2368,11 @@ 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 clone_function_name_numbered (const char *name, const char *suffix);
+tree clone_function_name_numbered (tree decl, const char *suffix);
+tree clone_function_name (const char *name, const char *suffix,
+ unsigned long number);
+tree clone_function_name (tree decl, const char *suffix);
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..0e496d9 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -316,7 +316,8 @@ 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) = clone_function_name_numbered (thunk->decl,
+ "artificial_thunk");
SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
new_thunk = cgraph_node::create (new_decl);
@@ -514,11 +515,38 @@ 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 a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with a unique (for
+ each NAME) unspecified number. If clone numbering is not needed
+ then the two argument clone_function_name should be used instead.
+ Should not be called directly except for by
+ lto-partition.c:privatize_symbol_name_1. */
+tree
+clone_function_name_numbered (const char *name, const char *suffix)
+{
+ return clone_function_name (name, suffix, clone_fn_id_num++);
+}
+/* Return a new assembler name for a clone of DECL. Apart from string
+ SUFFIX, the new name will end with a unique (for each DECL
+ assembler name) unspecified number. If clone numbering is not
+ needed then the two argument clone_function_name should be used
+ instead. */
tree
-clone_function_name_1 (const char *name, const char *suffix)
+clone_function_name_numbered (tree decl, const char *suffix)
+{
+ tree name = DECL_ASSEMBLER_NAME (decl);
+ return clone_function_name_numbered (IDENTIFIER_POINTER (name),
+ suffix);
+}
+
+/* Return a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with the specified
+ number. If clone numbering is not needed then the two argument
+ clone_function_name should be used instead. */
+tree
+clone_function_name (const char *name, const char *suffix,
+ unsigned long number)
{
size_t len = strlen (name);
char *tmp_name, *prefix;
@@ -527,17 +555,34 @@ 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++);
+ ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, number);
return get_identifier (tmp_name);
}
-/* Return a new assembler name for a clone of DECL with SUFFIX. */
+/* Return a new assembler name ending with the string SUFFIX for a
+ clone of DECL. */
tree
clone_function_name (tree decl, const char *suffix)
{
- tree name = DECL_ASSEMBLER_NAME (decl);
- return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+ tree identifier = DECL_ASSEMBLER_NAME (decl);
+ /* For consistency this needs to behave the same way as
+ ASM_FORMAT_PRIVATE_NAME does, but without the final number
+ suffix. */
+ char *separator = XALLOCAVEC (char, 2);
+ separator[0] = symbol_table::symbol_suffix_separator ();
+ separator[1] = 0;
+#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
+ const char *prefix = "__";
+#else
+ const char *prefix = "";
+#endif
+ char *result = ACONCAT ((prefix,
+ IDENTIFIER_POINTER (identifier),
+ separator,
+ suffix,
+ (char*)0));
+ return get_identifier (result);
}
@@ -585,7 +630,8 @@ 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, clone_function_name_numbered (old_decl,
+ suffix));
SET_DECL_RTL (new_decl, NULL);
new_node = create_clone (new_decl, count, false,
@@ -964,7 +1010,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) = clone_function_name_numbered (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..c314739 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 = clone_function_name_numbered (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..24e7c23 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"));
+ clone_function_name_numbered (
+ 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..9bbfeee 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"));
+ clone_function_name_numbered (
+ 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));
+ clone_function_name_numbered (
+ node->decl, name));
}
return new_node;
}
diff --git gcc/omp-expand.c gcc/omp-expand.c
index d2a77c0..faff02b 100644
--- gcc/omp-expand.c
+++ gcc/omp-expand.c
@@ -7625,7 +7625,8 @@ 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) = clone_function_name_numbered (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..29539b3 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 clone_function_name_numbered (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..7bafe39 100644
--- gcc/omp-simd-clone.c
+++ gcc/omp-simd-clone.c
@@ -444,7 +444,8 @@ 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) = clone_function_name_numbered (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/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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-26 4:39 ` [PATCH v4] Avoid unnecessarily numbering cloned symbols Michael Ploujnikov
@ 2018-10-26 8:56 ` Martin Liška
2018-10-26 14:31 ` Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2018-10-26 8:56 UTC (permalink / raw)
To: Michael Ploujnikov, Bernhard Reutner-Fischer, gcc-patches
Cc: Sriraman Tallam, ebotcazou, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]
On 10/26/18 12:59 AM, Michael Ploujnikov wrote:
> I've taken the advice from a discussion on IRC and re-wrote the patch
> with more uniform function names and using overloading.
>
> I think this function accomplished the following goals:
> - remove clone numbering where it's not needed:
> final.c:final_scan_insn_1 and
> symtab.c:simd_symtab_node::noninterposable_alias.
> - name and document the clone naming API such that future users won't
> accidentally use the numbering when it's not necessary; if you need
> numbering then you need to explicitly ask for it with the right
> function
> - provide a new function that allows users to specify a clone number
> explicitly as an argument
Hello.
Thanks for reworking that.
>
> My thoughts for future improvements:
> - It's a bit unfortunate that lto-partition.c:privatize_symbol_name_1
> has to break the decl abstraction and pass in a string that it
> created into what I would consider the implementation-detail
> function. The best way I can think of to make it uniform with the
> rest of the users is to have it create a new empty decl with
> DECL_ASSEMBLER_NAME set to the new string
That's not nice to create artificial declaration. Having string variant
is fine for me.
> - It's unfortunate that I have to duplicate the separator
> concatenation in the numberless clone_function_name, but I think it
> has to be like that unless ASM_FORMAT_PRIVATE_NAME making the
> number optional.
>
That's also fine for me. I'm attaching small nits that I found.
And please reformat following chunk in ChangeLog entry:
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only. (clone_function_name):
Renamed to clone_function_name_numbered to be explicit about
numbering. (clone_function_name): New two-argument function
that does not number its output. (clone_function_name): New
three-argument function that takes a number to append to its
output.
into:
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only.
(clone_function_name): Renamed to clone_function_name_numbered
to be explicit about...
I'm adding Honza to CC, hope he can review it quickly.
Thanks,
Martin
[-- Attachment #2: nits.patch --]
[-- Type: text/x-patch, Size: 1772 bytes --]
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c896a5f60cb..9cba4c2c3a9 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -521,6 +521,7 @@ static GTY(()) unsigned int clone_fn_id_num;
then the two argument clone_function_name should be used instead.
Should not be called directly except for by
lto-partition.c:privatize_symbol_name_1. */
+
tree
clone_function_name_numbered (const char *name, const char *suffix)
{
@@ -532,6 +533,7 @@ clone_function_name_numbered (const char *name, const char *suffix)
assembler name) unspecified number. If clone numbering is not
needed then the two argument clone_function_name should be used
instead. */
+
tree
clone_function_name_numbered (tree decl, const char *suffix)
{
@@ -542,8 +544,9 @@ clone_function_name_numbered (tree decl, const char *suffix)
/* Return a new assembler name for a clone of decl named NAME. Apart
from the string SUFFIX, the new name will end with the specified
- number. If clone numbering is not needed then the two argument
+ NUMBER. If clone numbering is not needed then the two argument
clone_function_name should be used instead. */
+
tree
clone_function_name (const char *name, const char *suffix,
unsigned long number)
@@ -559,9 +562,9 @@ clone_function_name (const char *name, const char *suffix,
return get_identifier (tmp_name);
}
-
/* Return a new assembler name ending with the string SUFFIX for a
clone of DECL. */
+
tree
clone_function_name (tree decl, const char *suffix)
{
@@ -581,7 +584,7 @@ clone_function_name (tree decl, const char *suffix)
IDENTIFIER_POINTER (identifier),
separator,
suffix,
- (char*)0));
+ NULL));
return get_identifier (result);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-26 8:56 ` Martin Liška
@ 2018-10-26 14:31 ` Michael Ploujnikov
2018-10-26 15:11 ` Jan Hubicka
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-26 14:31 UTC (permalink / raw)
To: Martin Liška, Bernhard Reutner-Fischer, gcc-patches
Cc: Sriraman Tallam, ebotcazou, Jan Hubicka
[-- Attachment #1.1.1: Type: text/plain, Size: 3084 bytes --]
Hi Martin,
Thanks for the review.
On 2018-10-26 03:51 AM, Martin Liška wrote:
> On 10/26/18 12:59 AM, Michael Ploujnikov wrote:
>> I've taken the advice from a discussion on IRC and re-wrote the patch
>> with more uniform function names and using overloading.
>>
>> I think this function accomplished the following goals:
>> - remove clone numbering where it's not needed:
>> final.c:final_scan_insn_1 and
>> symtab.c:simd_symtab_node::noninterposable_alias.
>> - name and document the clone naming API such that future users won't
>> accidentally use the numbering when it's not necessary; if you need
>> numbering then you need to explicitly ask for it with the right
>> function
>> - provide a new function that allows users to specify a clone number
>> explicitly as an argument
>
> Hello.
>
> Thanks for reworking that.
>
>>
>> My thoughts for future improvements:
>> - It's a bit unfortunate that lto-partition.c:privatize_symbol_name_1
>> has to break the decl abstraction and pass in a string that it
>> created into what I would consider the implementation-detail
>> function. The best way I can think of to make it uniform with the
>> rest of the users is to have it create a new empty decl with
>> DECL_ASSEMBLER_NAME set to the new string
>
> That's not nice to create artificial declaration. Having string variant
> is fine for me.
Ok.
>
>> - It's unfortunate that I have to duplicate the separator
>> concatenation in the numberless clone_function_name, but I think it
>> has to be like that unless ASM_FORMAT_PRIVATE_NAME making the
>> number optional.
>>
>
> That's also fine for me. I'm attaching small nits that I found.
> And please reformat following chunk in ChangeLog entry:
>
> * cgraph.h (clone_function_name_1): Replaced by new
> clone_function_name_numbered that takes name as string; for
> privatize_symbol_name_1 use only. (clone_function_name):
> Renamed to clone_function_name_numbered to be explicit about
> numbering. (clone_function_name): New two-argument function
> that does not number its output. (clone_function_name): New
> three-argument function that takes a number to append to its
> output.
>
> into:
>
> * cgraph.h (clone_function_name_1): Replaced by new
> clone_function_name_numbered that takes name as string; for
> privatize_symbol_name_1 use only.
> (clone_function_name): Renamed to clone_function_name_numbered
> to be explicit about...
Fixed, assuming you wanted me to start each function on a new line.
> suffix,
> - (char*)0));
> + NULL));
> return get_identifier (result);
> }
I've actually been told that NULL isn't always the same on some
targets and that I should use (char*)0 instead. Note that
libiberty/concat.c itself uses (char*)0.
>
> I'm adding Honza to CC, hope he can review it quickly.
>
> Thanks,
> Martin
>
Thanks again,
Michael
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Avoid-unnecessarily-numbering-cloned-symbols.patch --]
[-- Type: text/x-patch; name="0001-Avoid-unnecessarily-numbering-cloned-symbols.patch", Size: 15052 bytes --]
From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Thu, 25 Oct 2018 13:16:36 -0400
Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
gcc/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only.
(clone_function_name): Renamed to
clone_function_name_numbered to be explicit about numbering.
(clone_function_name): New two-argument function that does
not number its output.
(clone_function_name): New three-argument function that
takes a number to append to its output.
* cgraphclones.c (duplicate_thunk_for_node):
(clone_function_name_1): Renamed.
(clone_function_name_numbered): Two new functions.
(clone_function_name): Improved documentation.
(cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
* config/rs6000/rs6000.c (make_resolver_func): Ditto.
* final.c (final_scan_insn_1): Use the new clone_function_name
without numbering.
* multiple_target.c (create_dispatcher_calls): Ditto.
(create_target_clone): Ditto.
* omp-expand.c (grid_expand_target_grid_body): Ditto.
* omp-low.c (create_omp_child_function_name): Ditto.
* omp-simd-clone.c (simd_clone_create): Ditto.
* symtab.c (simd_symtab_node::noninterposable_alias): Use the
new clone_function_name without numbering.
gcc/lto/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* lto-partition.c (privatize_symbol_name_1): Use
clone_function_name_numbered.
gcc/testsuite/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* gcc.dg/tree-prof/cold_partition_label.c: Update for cold
section names without numbers.
* 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 | 7 ++-
gcc/cgraphclones.c | 69 ++++++++++++++++++----
gcc/config/rs6000/rs6000.c | 2 +-
gcc/lto/lto-partition.c | 4 +-
gcc/multiple_target.c | 8 +--
gcc/omp-expand.c | 3 +-
gcc/omp-low.c | 4 +-
gcc/omp-simd-clone.c | 3 +-
.../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 +-
12 files changed, 82 insertions(+), 28 deletions(-)
diff --git gcc/cgraph.h gcc/cgraph.h
index a8b1b4c..971065d 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2368,8 +2368,11 @@ 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 clone_function_name_numbered (const char *name, const char *suffix);
+tree clone_function_name_numbered (tree decl, const char *suffix);
+tree clone_function_name (const char *name, const char *suffix,
+ unsigned long number);
+tree clone_function_name (tree decl, const char *suffix);
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..fd2967c 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -316,7 +316,8 @@ 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) = clone_function_name_numbered (thunk->decl,
+ "artificial_thunk");
SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
new_thunk = cgraph_node::create (new_decl);
@@ -514,11 +515,41 @@ 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 a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with a unique (for
+ each NAME) unspecified number. If clone numbering is not needed
+ then the two argument clone_function_name should be used instead.
+ Should not be called directly except for by
+ lto-partition.c:privatize_symbol_name_1. */
tree
-clone_function_name_1 (const char *name, const char *suffix)
+clone_function_name_numbered (const char *name, const char *suffix)
+{
+ return clone_function_name (name, suffix, clone_fn_id_num++);
+}
+
+/* Return a new assembler name for a clone of DECL. Apart from string
+ SUFFIX, the new name will end with a unique (for each DECL
+ assembler name) unspecified number. If clone numbering is not
+ needed then the two argument clone_function_name should be used
+ instead. */
+
+tree
+clone_function_name_numbered (tree decl, const char *suffix)
+{
+ tree name = DECL_ASSEMBLER_NAME (decl);
+ return clone_function_name_numbered (IDENTIFIER_POINTER (name),
+ suffix);
+}
+
+/* Return a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with the specified
+ NUMBER. If clone numbering is not needed then the two argument
+ clone_function_name should be used instead. */
+
+tree
+clone_function_name (const char *name, const char *suffix,
+ unsigned long number)
{
size_t len = strlen (name);
char *tmp_name, *prefix;
@@ -527,17 +558,34 @@ 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++);
+ ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, number);
return get_identifier (tmp_name);
}
-/* Return a new assembler name for a clone of DECL with SUFFIX. */
+/* Return a new assembler name ending with the string SUFFIX for a
+ clone of DECL. */
tree
clone_function_name (tree decl, const char *suffix)
{
- tree name = DECL_ASSEMBLER_NAME (decl);
- return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+ tree identifier = DECL_ASSEMBLER_NAME (decl);
+ /* For consistency this needs to behave the same way as
+ ASM_FORMAT_PRIVATE_NAME does, but without the final number
+ suffix. */
+ char *separator = XALLOCAVEC (char, 2);
+ separator[0] = symbol_table::symbol_suffix_separator ();
+ separator[1] = 0;
+#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
+ const char *prefix = "__";
+#else
+ const char *prefix = "";
+#endif
+ char *result = ACONCAT ((prefix,
+ IDENTIFIER_POINTER (identifier),
+ separator,
+ suffix,
+ (char*)0));
+ return get_identifier (result);
}
@@ -585,7 +633,8 @@ 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, clone_function_name_numbered (old_decl,
+ suffix));
SET_DECL_RTL (new_decl, NULL);
new_node = create_clone (new_decl, count, false,
@@ -964,7 +1013,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) = clone_function_name_numbered (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..c314739 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 = clone_function_name_numbered (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..24e7c23 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"));
+ clone_function_name_numbered (
+ 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..9bbfeee 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"));
+ clone_function_name_numbered (
+ 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));
+ clone_function_name_numbered (
+ node->decl, name));
}
return new_node;
}
diff --git gcc/omp-expand.c gcc/omp-expand.c
index d2a77c0..faff02b 100644
--- gcc/omp-expand.c
+++ gcc/omp-expand.c
@@ -7625,7 +7625,8 @@ 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) = clone_function_name_numbered (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..29539b3 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 clone_function_name_numbered (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..7bafe39 100644
--- gcc/omp-simd-clone.c
+++ gcc/omp-simd-clone.c
@@ -444,7 +444,8 @@ 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) = clone_function_name_numbered (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/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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-26 14:31 ` Michael Ploujnikov
@ 2018-10-26 15:11 ` Jan Hubicka
2018-10-27 19:53 ` Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Jan Hubicka @ 2018-10-26 15:11 UTC (permalink / raw)
To: Michael Ploujnikov
Cc: Martin Liška, Bernhard Reutner-Fischer, gcc-patches,
Sriraman Tallam, ebotcazou
> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
> From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
> Date: Thu, 25 Oct 2018 13:16:36 -0400
> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
>
> gcc/ChangeLog:
>
> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>
> * cgraph.h (clone_function_name_1): Replaced by new
> clone_function_name_numbered that takes name as string; for
> privatize_symbol_name_1 use only.
> (clone_function_name): Renamed to
> clone_function_name_numbered to be explicit about numbering.
> (clone_function_name): New two-argument function that does
> not number its output.
> (clone_function_name): New three-argument function that
> takes a number to append to its output.
> * cgraphclones.c (duplicate_thunk_for_node):
> (clone_function_name_1): Renamed.
> (clone_function_name_numbered): Two new functions.
> (clone_function_name): Improved documentation.
> (cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
> * config/rs6000/rs6000.c (make_resolver_func): Ditto.
> * final.c (final_scan_insn_1): Use the new clone_function_name
> without numbering.
> * multiple_target.c (create_dispatcher_calls): Ditto.
> (create_target_clone): Ditto.
> * omp-expand.c (grid_expand_target_grid_body): Ditto.
> * omp-low.c (create_omp_child_function_name): Ditto.
> * omp-simd-clone.c (simd_clone_create): Ditto.
> * symtab.c (simd_symtab_node::noninterposable_alias): Use the
> new clone_function_name without numbering.
>
> gcc/lto/ChangeLog:
>
> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>
> * lto-partition.c (privatize_symbol_name_1): Use
> clone_function_name_numbered.
>
> gcc/testsuite/ChangeLog:
>
> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>
> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold
> section names without numbers.
> * 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.
OK,
thanks!
Honza
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-26 15:11 ` Jan Hubicka
@ 2018-10-27 19:53 ` Michael Ploujnikov
2018-10-29 10:31 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-27 19:53 UTC (permalink / raw)
To: Jan Hubicka
Cc: Martin Liška, Bernhard Reutner-Fischer, gcc-patches,
Sriraman Tallam, ebotcazou
[-- Attachment #1.1: Type: text/plain, Size: 2403 bytes --]
Hi,
On 2018-10-26 10:25 a.m., Jan Hubicka wrote:
>> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
>> From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
>> Date: Thu, 25 Oct 2018 13:16:36 -0400
>> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
>>
>> gcc/ChangeLog:
>>
>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>
>> * cgraph.h (clone_function_name_1): Replaced by new
>> clone_function_name_numbered that takes name as string; for
>> privatize_symbol_name_1 use only.
>> (clone_function_name): Renamed to
>> clone_function_name_numbered to be explicit about numbering.
>> (clone_function_name): New two-argument function that does
>> not number its output.
>> (clone_function_name): New three-argument function that
>> takes a number to append to its output.
>> * cgraphclones.c (duplicate_thunk_for_node):
>> (clone_function_name_1): Renamed.
>> (clone_function_name_numbered): Two new functions.
>> (clone_function_name): Improved documentation.
>> (cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
>> * config/rs6000/rs6000.c (make_resolver_func): Ditto.
>> * final.c (final_scan_insn_1): Use the new clone_function_name
>> without numbering.
>> * multiple_target.c (create_dispatcher_calls): Ditto.
>> (create_target_clone): Ditto.
>> * omp-expand.c (grid_expand_target_grid_body): Ditto.
>> * omp-low.c (create_omp_child_function_name): Ditto.
>> * omp-simd-clone.c (simd_clone_create): Ditto.
>> * symtab.c (simd_symtab_node::noninterposable_alias): Use the
>> new clone_function_name without numbering.
>>
>> gcc/lto/ChangeLog:
>>
>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>
>> * lto-partition.c (privatize_symbol_name_1): Use
>> clone_function_name_numbered.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>
>> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold
>> section names without numbers.
>> * 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.
>
> OK,
> thanks!
> Honza
>
Thanks again for the review. This is my first patch and I don't have
commit access. What should I do?
- Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-27 19:53 ` Michael Ploujnikov
@ 2018-10-29 10:31 ` Martin Liška
2018-10-29 11:51 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2018-10-29 10:31 UTC (permalink / raw)
To: Michael Ploujnikov, Jan Hubicka
Cc: Bernhard Reutner-Fischer, gcc-patches, Sriraman Tallam, ebotcazou
On 10/27/18 6:15 PM, Michael Ploujnikov wrote:
> Hi,
>
> On 2018-10-26 10:25 a.m., Jan Hubicka wrote:
>>> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
>>> From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>> Date: Thu, 25 Oct 2018 13:16:36 -0400
>>> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>
>>> * cgraph.h (clone_function_name_1): Replaced by new
>>> clone_function_name_numbered that takes name as string; for
>>> privatize_symbol_name_1 use only.
>>> (clone_function_name): Renamed to
>>> clone_function_name_numbered to be explicit about numbering.
>>> (clone_function_name): New two-argument function that does
>>> not number its output.
>>> (clone_function_name): New three-argument function that
>>> takes a number to append to its output.
>>> * cgraphclones.c (duplicate_thunk_for_node):
>>> (clone_function_name_1): Renamed.
>>> (clone_function_name_numbered): Two new functions.
>>> (clone_function_name): Improved documentation.
>>> (cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
>>> * config/rs6000/rs6000.c (make_resolver_func): Ditto.
>>> * final.c (final_scan_insn_1): Use the new clone_function_name
>>> without numbering.
>>> * multiple_target.c (create_dispatcher_calls): Ditto.
>>> (create_target_clone): Ditto.
>>> * omp-expand.c (grid_expand_target_grid_body): Ditto.
>>> * omp-low.c (create_omp_child_function_name): Ditto.
>>> * omp-simd-clone.c (simd_clone_create): Ditto.
>>> * symtab.c (simd_symtab_node::noninterposable_alias): Use the
>>> new clone_function_name without numbering.
>>>
>>> gcc/lto/ChangeLog:
>>>
>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>
>>> * lto-partition.c (privatize_symbol_name_1): Use
>>> clone_function_name_numbered.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>
>>> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold
>>> section names without numbers.
>>> * 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.
>>
>> OK,
>> thanks!
>> Honza
>>
>
> Thanks again for the review. This is my first patch and I don't have
> commit access. What should I do?
I'm going to install the patch on your behalf. For write access you should
follow these intructions: https://www.gnu.org/software/gcc/svnwrite.html#policies
Martin
>
>
> - Michael
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-29 10:31 ` Martin Liška
@ 2018-10-29 11:51 ` Martin Liška
2018-10-29 18:45 ` Michael Ploujnikov
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2018-10-29 11:51 UTC (permalink / raw)
To: Michael Ploujnikov, Jan Hubicka
Cc: Bernhard Reutner-Fischer, gcc-patches, Sriraman Tallam, ebotcazou
On 10/29/18 9:40 AM, Martin Liška wrote:
> On 10/27/18 6:15 PM, Michael Ploujnikov wrote:
>> Hi,
>>
>> On 2018-10-26 10:25 a.m., Jan Hubicka wrote:
>>>> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
>>>> From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>> Date: Thu, 25 Oct 2018 13:16:36 -0400
>>>> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>
>>>> * cgraph.h (clone_function_name_1): Replaced by new
>>>> clone_function_name_numbered that takes name as string; for
>>>> privatize_symbol_name_1 use only.
>>>> (clone_function_name): Renamed to
>>>> clone_function_name_numbered to be explicit about numbering.
>>>> (clone_function_name): New two-argument function that does
>>>> not number its output.
>>>> (clone_function_name): New three-argument function that
>>>> takes a number to append to its output.
>>>> * cgraphclones.c (duplicate_thunk_for_node):
>>>> (clone_function_name_1): Renamed.
>>>> (clone_function_name_numbered): Two new functions.
>>>> (clone_function_name): Improved documentation.
>>>> (cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
>>>> * config/rs6000/rs6000.c (make_resolver_func): Ditto.
>>>> * final.c (final_scan_insn_1): Use the new clone_function_name
>>>> without numbering.
>>>> * multiple_target.c (create_dispatcher_calls): Ditto.
>>>> (create_target_clone): Ditto.
>>>> * omp-expand.c (grid_expand_target_grid_body): Ditto.
>>>> * omp-low.c (create_omp_child_function_name): Ditto.
>>>> * omp-simd-clone.c (simd_clone_create): Ditto.
>>>> * symtab.c (simd_symtab_node::noninterposable_alias): Use the
>>>> new clone_function_name without numbering.
>>>>
>>>> gcc/lto/ChangeLog:
>>>>
>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>
>>>> * lto-partition.c (privatize_symbol_name_1): Use
>>>> clone_function_name_numbered.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>
>>>> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold
>>>> section names without numbers.
>>>> * 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.
>>>
>>> OK,
>>> thanks!
>>> Honza
>>>
>>
>> Thanks again for the review. This is my first patch and I don't have
>> commit access. What should I do?
>
> I'm going to install the patch on your behalf. For write access you should
> follow these intructions: https://www.gnu.org/software/gcc/svnwrite.html#policies
>
> Martin
>
>>
>>
>> - Michael
>>
>
But first I see some failures when I tried to apply the patch:
$ patch -p0 --dry-run < ~/Downloads/0001-Avoid-unnecessarily-numbering-cloned-symbols.patch
checking file gcc/cgraph.h
Hunk #1 succeeded at 2382 with fuzz 1 (offset 14 lines).
checking file gcc/cgraphclones.c
Hunk #1 succeeded at 317 (offset 1 line).
checking file gcc/config/rs6000/rs6000.c
Hunk #1 succeeded at 36997 (offset 485 lines).
checking file gcc/lto/lto-partition.c
checking file gcc/multiple_target.c
checking file gcc/omp-expand.c
checking file gcc/omp-low.c
checking file gcc/omp-simd-clone.c
checking file gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
Hunk #1 FAILED at 42.
1 out of 1 hunk FAILED
checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
Hunk #1 FAILED at 41.
1 out of 1 hunk FAILED
checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
Hunk #1 FAILED at 42.
1 out of 1 hunk FAILED
Can you please rebase that on top of current trunk?
Thanks,
Martin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-29 11:51 ` Martin Liška
@ 2018-10-29 18:45 ` Michael Ploujnikov
2018-10-30 12:55 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ploujnikov @ 2018-10-29 18:45 UTC (permalink / raw)
To: Martin Liška, Jan Hubicka
Cc: Bernhard Reutner-Fischer, gcc-patches, Sriraman Tallam, ebotcazou
[-- Attachment #1.1.1: Type: text/plain, Size: 4332 bytes --]
On 2018-10-29 6:49 a.m., Martin Liška wrote:
> On 10/29/18 9:40 AM, Martin Liška wrote:
>> On 10/27/18 6:15 PM, Michael Ploujnikov wrote:
>>> Hi,
>>>
>>> On 2018-10-26 10:25 a.m., Jan Hubicka wrote:
>>>>> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001
>>>>> From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>> Date: Thu, 25 Oct 2018 13:16:36 -0400
>>>>> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>>
>>>>> * cgraph.h (clone_function_name_1): Replaced by new
>>>>> clone_function_name_numbered that takes name as string; for
>>>>> privatize_symbol_name_1 use only.
>>>>> (clone_function_name): Renamed to
>>>>> clone_function_name_numbered to be explicit about numbering.
>>>>> (clone_function_name): New two-argument function that does
>>>>> not number its output.
>>>>> (clone_function_name): New three-argument function that
>>>>> takes a number to append to its output.
>>>>> * cgraphclones.c (duplicate_thunk_for_node):
>>>>> (clone_function_name_1): Renamed.
>>>>> (clone_function_name_numbered): Two new functions.
>>>>> (clone_function_name): Improved documentation.
>>>>> (cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
>>>>> * config/rs6000/rs6000.c (make_resolver_func): Ditto.
>>>>> * final.c (final_scan_insn_1): Use the new clone_function_name
>>>>> without numbering.
>>>>> * multiple_target.c (create_dispatcher_calls): Ditto.
>>>>> (create_target_clone): Ditto.
>>>>> * omp-expand.c (grid_expand_target_grid_body): Ditto.
>>>>> * omp-low.c (create_omp_child_function_name): Ditto.
>>>>> * omp-simd-clone.c (simd_clone_create): Ditto.
>>>>> * symtab.c (simd_symtab_node::noninterposable_alias): Use the
>>>>> new clone_function_name without numbering.
>>>>>
>>>>> gcc/lto/ChangeLog:
>>>>>
>>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>>
>>>>> * lto-partition.c (privatize_symbol_name_1): Use
>>>>> clone_function_name_numbered.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>>>>>
>>>>> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold
>>>>> section names without numbers.
>>>>> * 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.
>>>>
>>>> OK,
>>>> thanks!
>>>> Honza
>>>>
>>>
>>> Thanks again for the review. This is my first patch and I don't have
>>> commit access. What should I do?
>>
>> I'm going to install the patch on your behalf. For write access you should
>> follow these intructions: https://www.gnu.org/software/gcc/svnwrite.html#policies
>>
>> Martin
>>
>>>
>>>
>>> - Michael
>>>
>>
>
> But first I see some failures when I tried to apply the patch:
>
> $ patch -p0 --dry-run < ~/Downloads/0001-Avoid-unnecessarily-numbering-cloned-symbols.patch
> checking file gcc/cgraph.h
> Hunk #1 succeeded at 2382 with fuzz 1 (offset 14 lines).
> checking file gcc/cgraphclones.c
> Hunk #1 succeeded at 317 (offset 1 line).
> checking file gcc/config/rs6000/rs6000.c
> Hunk #1 succeeded at 36997 (offset 485 lines).
> checking file gcc/lto/lto-partition.c
> checking file gcc/multiple_target.c
> checking file gcc/omp-expand.c
> checking file gcc/omp-low.c
> checking file gcc/omp-simd-clone.c
> checking file gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
> checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
> Hunk #1 FAILED at 42.
> 1 out of 1 hunk FAILED
> checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
> Hunk #1 FAILED at 41.
> 1 out of 1 hunk FAILED
> checking file gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
> Hunk #1 FAILED at 42.
> 1 out of 1 hunk FAILED
>
> Can you please rebase that on top of current trunk?
> Thanks,
> Martin
>
Sorry about that. Attached is the fixed patch. Note that I'm currently unable to run
those particular section-attr tests to verify the correctness of the new scan-assembler
expressions.
Thanks for installing the patch while I figure out the SVN access.
- Michael
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-Avoid-unnecessarily-numbering-cloned-symbols.patch --]
[-- Type: text/x-patch; name="0001-Avoid-unnecessarily-numbering-cloned-symbols.patch", Size: 15799 bytes --]
From 13d6c5446f807d8691ad2c554eda6fef26dc12b0 Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujnikov@oracle.com>
Date: Thu, 25 Oct 2018 13:16:36 -0400
Subject: [PATCH] Avoid unnecessarily numbering cloned symbols.
gcc/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* cgraph.h (clone_function_name_1): Replaced by new
clone_function_name_numbered that takes name as string; for
privatize_symbol_name_1 use only.
(clone_function_name): Renamed to
clone_function_name_numbered to be explicit about numbering.
(clone_function_name): New two-argument function that does
not number its output.
(clone_function_name): New three-argument function that
takes a number to append to its output.
* cgraphclones.c (duplicate_thunk_for_node):
(clone_function_name_1): Renamed.
(clone_function_name_numbered): Two new functions.
(clone_function_name): Improved documentation.
(cgraph_node::create_virtual_clone): Use clone_function_name_numbered.
* config/rs6000/rs6000.c (make_resolver_func): Ditto.
* final.c (final_scan_insn_1): Use the new clone_function_name
without numbering.
* multiple_target.c (create_dispatcher_calls): Ditto.
(create_target_clone): Ditto.
* omp-expand.c (grid_expand_target_grid_body): Ditto.
* omp-low.c (create_omp_child_function_name): Ditto.
* omp-simd-clone.c (simd_clone_create): Ditto.
* symtab.c (simd_symtab_node::noninterposable_alias): Use the
new clone_function_name without numbering.
gcc/lto/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* lto-partition.c (privatize_symbol_name_1): Use
clone_function_name_numbered.
gcc/testsuite/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujnikov@oracle.com>
* gcc.dg/tree-prof/cold_partition_label.c: Update for cold
section names without numbers.
* 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 | 7 +-
gcc/cgraphclones.c | 69 ++++++++++++++++---
gcc/config/rs6000/rs6000.c | 2 +-
gcc/lto/lto-partition.c | 4 +-
gcc/multiple_target.c | 8 +--
gcc/omp-expand.c | 3 +-
gcc/omp-low.c | 4 +-
gcc/omp-simd-clone.c | 3 +-
.../gcc.dg/tree-prof/cold_partition_label.c | 4 +-
.../gcc.dg/tree-prof/section-attr-1.c | 4 +-
.../gcc.dg/tree-prof/section-attr-2.c | 4 +-
.../gcc.dg/tree-prof/section-attr-3.c | 4 +-
12 files changed, 85 insertions(+), 31 deletions(-)
diff --git gcc/cgraph.h gcc/cgraph.h
index 71c54537b9..c13d79850f 100644
--- gcc/cgraph.h
+++ gcc/cgraph.h
@@ -2382,8 +2382,11 @@ tree thunk_adjust (gimple_stmt_iterator *, tree, bool, HOST_WIDE_INT, tree,
HOST_WIDE_INT);
/* In cgraphclones.c */
-tree clone_function_name_1 (const char *, const char *);
-tree clone_function_name (tree decl, const char *);
+tree clone_function_name_numbered (const char *name, const char *suffix);
+tree clone_function_name_numbered (tree decl, const char *suffix);
+tree clone_function_name (const char *name, const char *suffix,
+ unsigned long number);
+tree clone_function_name (tree decl, const char *suffix);
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 189cb31a5d..e17959c0ca 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -317,7 +317,8 @@ 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) = clone_function_name_numbered (thunk->decl,
+ "artificial_thunk");
SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
new_thunk = cgraph_node::create (new_decl);
@@ -514,11 +515,41 @@ 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 a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with a unique (for
+ each NAME) unspecified number. If clone numbering is not needed
+ then the two argument clone_function_name should be used instead.
+ Should not be called directly except for by
+ lto-partition.c:privatize_symbol_name_1. */
tree
-clone_function_name_1 (const char *name, const char *suffix)
+clone_function_name_numbered (const char *name, const char *suffix)
+{
+ return clone_function_name (name, suffix, clone_fn_id_num++);
+}
+
+/* Return a new assembler name for a clone of DECL. Apart from string
+ SUFFIX, the new name will end with a unique (for each DECL
+ assembler name) unspecified number. If clone numbering is not
+ needed then the two argument clone_function_name should be used
+ instead. */
+
+tree
+clone_function_name_numbered (tree decl, const char *suffix)
+{
+ tree name = DECL_ASSEMBLER_NAME (decl);
+ return clone_function_name_numbered (IDENTIFIER_POINTER (name),
+ suffix);
+}
+
+/* Return a new assembler name for a clone of decl named NAME. Apart
+ from the string SUFFIX, the new name will end with the specified
+ NUMBER. If clone numbering is not needed then the two argument
+ clone_function_name should be used instead. */
+
+tree
+clone_function_name (const char *name, const char *suffix,
+ unsigned long number)
{
size_t len = strlen (name);
char *tmp_name, *prefix;
@@ -527,17 +558,34 @@ 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++);
+ ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, number);
return get_identifier (tmp_name);
}
-/* Return a new assembler name for a clone of DECL with SUFFIX. */
+/* Return a new assembler name ending with the string SUFFIX for a
+ clone of DECL. */
tree
clone_function_name (tree decl, const char *suffix)
{
- tree name = DECL_ASSEMBLER_NAME (decl);
- return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
+ tree identifier = DECL_ASSEMBLER_NAME (decl);
+ /* For consistency this needs to behave the same way as
+ ASM_FORMAT_PRIVATE_NAME does, but without the final number
+ suffix. */
+ char *separator = XALLOCAVEC (char, 2);
+ separator[0] = symbol_table::symbol_suffix_separator ();
+ separator[1] = 0;
+#if defined (NO_DOT_IN_LABEL) && defined (NO_DOLLAR_IN_LABEL)
+ const char *prefix = "__";
+#else
+ const char *prefix = "";
+#endif
+ char *result = ACONCAT ((prefix,
+ IDENTIFIER_POINTER (identifier),
+ separator,
+ suffix,
+ (char*)0));
+ return get_identifier (result);
}
@@ -585,7 +633,8 @@ 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, clone_function_name_numbered (old_decl,
+ suffix));
SET_DECL_RTL (new_decl, NULL);
new_node = create_clone (new_decl, count, false,
@@ -964,7 +1013,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) = clone_function_name_numbered (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 a9d038829b..4f113cb025 100644
--- gcc/config/rs6000/rs6000.c
+++ gcc/config/rs6000/rs6000.c
@@ -36997,7 +36997,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 = clone_function_name_numbered (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 c7a5710f77..24e7c23859 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"));
+ clone_function_name_numbered (
+ 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 2d892f201c..5225e46bf0 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"));
+ clone_function_name_numbered (
+ 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));
+ clone_function_name_numbered (
+ node->decl, name));
}
return new_node;
}
diff --git gcc/omp-expand.c gcc/omp-expand.c
index e8abde413d..1185a26619 100644
--- gcc/omp-expand.c
+++ gcc/omp-expand.c
@@ -7625,7 +7625,8 @@ 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) = clone_function_name_numbered (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 bbcbc121ba..b06ddb3857 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 clone_function_name_numbered (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 b15adf0bad..7bafe39b17 100644
--- gcc/omp-simd-clone.c
+++ gcc/omp-simd-clone.c
@@ -444,7 +444,8 @@ 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) = clone_function_name_numbered (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/testsuite/gcc.dg/tree-prof/cold_partition_label.c gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
index 52518cf40c..450308d640 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 1f99b3128c..0cbd2de0cb 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-1.c
@@ -46,5 +46,5 @@ 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,__text_cold\.\*\[\\n\\r\]+_foo\.cold\.0" { target *-*-darwin* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*__TEXT,__text_cold\*\[\\n\\r\]+_foo\.cold" { target *-*-darwin* } } } */
diff --git gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
index 9bdc63a1b0..75a4d8a2c9 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-2.c
@@ -45,5 +45,5 @@ 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,__text_cold\.\*\[\\n\\r\]+_foo\.cold\.0:" { target *-*-darwin* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*__TEXT,__text_cold\*\[\\n\\r\]+_foo\.cold:" { target *-*-darwin* } } } */
diff --git gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
index 29eee4587d..c243b18b1c 100644
--- gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
+++ gcc/testsuite/gcc.dg/tree-prof/section-attr-3.c
@@ -46,5 +46,5 @@ 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,__text_cold\.\*\[\\n\\r\]+_foo\.cold\.0:" { target *-*-darwin* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "\.section\[\t \]*__TEXT,__text_cold\*\[\\n\\r\]+_foo\.cold:" { target *-*-darwin* } } } */
--
2.19.1
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
2018-10-29 18:45 ` Michael Ploujnikov
@ 2018-10-30 12:55 ` Martin Liška
0 siblings, 0 replies; 14+ messages in thread
From: Martin Liška @ 2018-10-30 12:55 UTC (permalink / raw)
To: Michael Ploujnikov, Jan Hubicka
Cc: Bernhard Reutner-Fischer, gcc-patches, Sriraman Tallam, ebotcazou
On 10/29/18 6:43 PM, Michael Ploujnikov wrote:
> Thanks for installing the patch while I figure out the SVN access.
> - Michael
Installed as r265621.
Martin
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-10-30 11:40 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-20 3:29 Avoid unnecessarily numbered clone symbols Michael Ploujnikov
2018-10-20 15:42 ` Bernhard Reutner-Fischer
2018-10-21 8:06 ` Michael Ploujnikov
2018-10-22 6:14 ` [PATCH v2] " Michael Ploujnikov
2018-10-23 16:31 ` [PATCH v3] " Michael Ploujnikov
2018-10-26 4:39 ` [PATCH v4] Avoid unnecessarily numbering cloned symbols Michael Ploujnikov
2018-10-26 8:56 ` Martin Liška
2018-10-26 14:31 ` Michael Ploujnikov
2018-10-26 15:11 ` Jan Hubicka
2018-10-27 19:53 ` Michael Ploujnikov
2018-10-29 10:31 ` Martin Liška
2018-10-29 11:51 ` Martin Liška
2018-10-29 18:45 ` Michael Ploujnikov
2018-10-30 12:55 ` Martin Liška
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).