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.ploujni...@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.ploujni...@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.ploujni...@oracle.com> >>>>> >>>>> * lto-partition.c (privatize_symbol_name_1): Use >>>>> clone_function_name_numbered. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> 2018-10-26 Michael Ploujnikov <michael.ploujni...@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
From 13d6c5446f807d8691ad2c554eda6fef26dc12b0 Mon Sep 17 00:00:00 2001
From: Michael Ploujnikov <michael.ploujni...@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.ploujni...@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.ploujni...@oracle.com>
* lto-partition.c (privatize_symbol_name_1): Use
clone_function_name_numbered.
gcc/testsuite/ChangeLog:
2018-10-26 Michael Ploujnikov <michael.ploujni...@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
signature.asc
Description: OpenPGP digital signature
