diff mbox series

Make function clone name numbering independent.

Message ID a8a7ea06-98f3-1b7c-d0bb-4e428cb97cf3@oracle.com
State New
Headers show
Series Make function clone name numbering independent. | expand

Commit Message

Michael Ploujnikov July 16, 2018, 7:38 p.m. UTC
Hi,

This patch is a small part of the work I'm doing to make function codegen/assembly independent from one another as mentioned in: https://gcc.gnu.org/ml/gcc/2018-07/msg00210.html. It deals with clone_fn_id_num rather than object UIDs and I figured it's better to make my first submission with a smaller, simpler and self-contained patch.

This changes clone_function_name_1 such that clone names are based on a per-function rather than a global counter so that the number of clones of one function doesn't affect the numbering of clone names of other functions.

This should have minimal impact as the only user of the clone names that I found (https://gcc.gnu.org/ml/gcc/2013-03/msg00268.html) doesn't actually care about the specific numeric values.


Thanks
- Michael

gcc:
2018-07-16  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

       Make function clone name numbering independent.
       * cgraphclones.c: Replace clone_fn_id_num with clone_fn_ids.
       (clone_function_name_1): Use it.

testsuite:
2018-07-16  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

	Clone id counters should be completely independent from one another.
	* gcc/testsuite/gcc.dg/independent-cloneids-1.c: New test.


---
 gcc/cgraphclones.c                            | 11 ++++++--
 gcc/testsuite/gcc.dg/independent-cloneids-1.c | 38 +++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/independent-cloneids-1.c

Comments

Bernhard Reutner-Fischer July 17, 2018, 6:10 a.m. UTC | #1
On 16 July 2018 21:38:36 CEST, Michael Ploujnikov <michael.ploujnikov@oracle.com> wrote:
>Hi,
>

>+    clone_fn_ids = hash_map<const char *, unsigned>::create_ggc
>(1000);

Isn't 1000 a bit excessive? What about 64 or thereabouts? 

thanks,
Richard Biener July 17, 2018, 10:02 a.m. UTC | #2
On Tue, Jul 17, 2018 at 8:10 AM Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
>
> On 16 July 2018 21:38:36 CEST, Michael Ploujnikov <michael.ploujnikov@oracle.com> wrote:
> >Hi,
> >
>
> >+    clone_fn_ids = hash_map<const char *, unsigned>::create_ggc
> >(1000);
>
> Isn't 1000 a bit excessive? What about 64 or thereabouts?

I'm not sure we should throw memory at this "problem" in this way.
What specific issue
does this address?

Iff then I belive forgoing the automatic counter addidion is the way
to go and hand
control of that to the callers (for example the caller in
lto/lto-partition.c doesn't
even seem to need that counter.

You also assume the string you key on persists - luckily the
lto-partition.c caller
leaks it but this makes your approach quite fragile in my eye (put the logic
into clone_function_name instead, where you at least know you are dealing
with a string from an indentifier which are never collected).

Richard.

> thanks,
diff mbox series

Patch

diff --git gcc/cgraphclones.c gcc/cgraphclones.c
index 69572b9..c5a40bd 100644
--- gcc/cgraphclones.c
+++ gcc/cgraphclones.c
@@ -528,7 +528,7 @@  cgraph_node::create_clone (tree new_decl, gcov_type gcov_count, int freq,
   return new_node;
 }
 
-static GTY(()) unsigned int clone_fn_id_num;
+static GTY(()) hash_map<const char *, unsigned> *clone_fn_ids;
 
 /* Return a new assembler name for a clone with SUFFIX of a decl named
    NAME.  */
@@ -543,7 +543,14 @@  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++);
+  unsigned int *suffix_counter;
+  if (!clone_fn_ids) {
+    /* Initialize the per-function counter hash table if this is the first call */
+    clone_fn_ids = hash_map<const char *, unsigned>::create_ggc (1000);
+  }
+  suffix_counter = &clone_fn_ids->get_or_insert(name);
+  ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, *suffix_counter);
+  *suffix_counter = *suffix_counter + 1;
   return get_identifier (tmp_name);
 }
 
diff --git gcc/testsuite/gcc.dg/independent-cloneids-1.c gcc/testsuite/gcc.dg/independent-cloneids-1.c
new file mode 100644
index 0000000..d723e20
--- /dev/null
+++ gcc/testsuite/gcc.dg/independent-cloneids-1.c
@@ -0,0 +1,38 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp"  } */
+
+extern int printf (const char *, ...);
+
+static int __attribute__ ((noinline))
+foo (int arg)
+{
+  return 7 * arg;
+}
+
+static int __attribute__ ((noinline))
+bar (int arg)
+{
+  return arg * arg;
+}
+
+int
+baz (int arg)
+{
+  printf("%d\n", bar (3));
+  printf("%d\n", bar (4));
+  printf("%d\n", foo (5));
+  printf("%d\n", foo (6));
+  /* adding or removing the following call should not affect foo
+     function's clone numbering */
+  printf("%d\n", bar (7));
+  return foo (8);
+}
+
+/* { dg-final { scan-ipa-dump "Function bar.constprop.0" "cp" } } */
+/* { dg-final { scan-ipa-dump "Function bar.constprop.1" "cp" } } */
+/* { dg-final { scan-ipa-dump "Function bar.constprop.3" "cp" } } */
+/* { dg-final { scan-ipa-dump "Function foo.constprop.0" "cp" } } */
+/* { dg-final { scan-ipa-dump "Function foo.constprop.1" "cp" } } */
+/* { dg-final { scan-ipa-dump "Function foo.constprop.2" "cp" } } */
+/* { dg-final { scan-ipa-dump-not "Function foo.constprop.3" "cp" } } */
+/* { dg-final { scan-ipa-dump-not "Function foo.constprop.4" "cp" } } */