diff mbox series

[PR,87525] Zero local estimated benefit for cloning extern inline function

Message ID ri6va15moet.fsf@suse.cz
State New
Headers show
Series [PR,87525] Zero local estimated benefit for cloning extern inline function | expand

Commit Message

Martin Jambor Feb. 27, 2019, noon UTC 
Hi,

in the discussion in PR 87525 Honza noted that IPA-CP should not
estimate any local time benefits from cloning an extern inline function,
that any benefits this might bring about have to come from other
specializations such cloning might enable.

While the patch is only a heuristics change and so does not really fix
the issue (which itself is a part of more general set of problems with
-D_FORTIFY_SOURCE and LTO), it should make it much less likely and is
sensible change on its own.

Bootstrapped and tested on x86_54-linux, OK for trunk and the
gcc-8-branch?

Thanks,

Martin




2019-02-25  Martin Jambor  <mjambor@suse.cz>

	PR lto/87525
	* ipa-cp.c (perform_estimation_of_a_value): Account zero time benefit
	for extern inline functions.

	testsuite/
	* gcc.dg/ipa/ipcp-5.c: New test.
---
 gcc/ipa-cp.c                      | 17 ++++++++----
 gcc/testsuite/gcc.dg/ipa/ipcp-5.c | 45 +++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-5.c

Comments

Jan Hubicka March 7, 2019, 4:07 p.m. UTC | #1 
Dne 2019-02-27 13:00, Martin Jambor napsal:
> Hi,
> 
> in the discussion in PR 87525 Honza noted that IPA-CP should not
> estimate any local time benefits from cloning an extern inline 
> function,
> that any benefits this might bring about have to come from other
> specializations such cloning might enable.
> 
> While the patch is only a heuristics change and so does not really fix
> the issue (which itself is a part of more general set of problems with
> -D_FORTIFY_SOURCE and LTO), it should make it much less likely and is
> sensible change on its own.
> 
> Bootstrapped and tested on x86_54-linux, OK for trunk and the
> gcc-8-branch?
> 
> Thanks,
> 
> Martin
> 
> 
> 
> 
> 2019-02-25  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR lto/87525
> 	* ipa-cp.c (perform_estimation_of_a_value): Account zero time benefit
> 	for extern inline functions.
> 
> 	testsuite/
> 	* gcc.dg/ipa/ipcp-5.c: New test.

OK,
please wait a week before backporting to gcc 8. I believe gcc 7 suffers 
from
same issue so backporting there makes sense too.

Honza
diff mbox series

Patch

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 442d5c63eff..59b15fa7362 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -2818,11 +2818,18 @@  perform_estimation_of_a_value (cgraph_node *node, vec<tree> known_csts,
   base_time -= time;
   if (base_time > 65535)
     base_time = 65535;
-  time_benefit = base_time.to_int ()
-    + devirtualization_time_bonus (node, known_csts, known_contexts,
-				   known_aggs_ptrs)
-    + hint_time_bonus (hints)
-    + removable_params_cost + est_move_cost;
+
+  /* Extern inline functions have no cloning local time benefits because they
+     will be inlined anyway.  The only reason to clone them is if it enables
+     optimization in any of the functions they call.  */
+  if (DECL_EXTERNAL (node->decl) && DECL_DECLARED_INLINE_P (node->decl))
+    time_benefit = 0;
+  else
+    time_benefit = base_time.to_int ()
+      + devirtualization_time_bonus (node, known_csts, known_contexts,
+				     known_aggs_ptrs)
+      + hint_time_bonus (hints)
+      + removable_params_cost + est_move_cost;
 
   gcc_checking_assert (size >=0);
   /* The inliner-heuristics based estimates may think that in certain
diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-5.c b/gcc/testsuite/gcc.dg/ipa/ipcp-5.c
new file mode 100644
index 00000000000..6786c514543
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipcp-5.c
@@ -0,0 +1,45 @@ 
+/* Test that estimated local cloning time benefit of extern inline functions is
+   zero.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-ipa-cp -fno-early-inlining"  } */
+/* { dg-add-options bind_pic_locally } */
+
+extern int get_int (void);
+extern void use_stuff (int);
+
+int arr[10];
+
+inline void
+f (int a)
+{
+  arr[0] += a + 5;
+  arr[1] += a + 50;
+  arr[2] += a - 3;
+  arr[3] += a;
+  arr[4] += a + 21;
+  arr[5] += a + 900;
+  arr[6] += a + 2;
+  arr[7] += a + 3456;
+  arr[8] += a + 3;
+  arr[9] += a + 32;
+  use_stuff (a);
+}
+
+
+int
+entry (void)
+{
+  int i;
+  for (i = 0; i < 100; i++)
+    f (7);
+  for (i = 0; i < 100; i++)
+    f (get_int ());
+  return 0;
+}
+
+
+/* { dg-final { scan-ipa-dump "loc_time: 0" "cp" } } */
+/* { dg-final { scan-ipa-dump-not "replacing param.*with const" "cp"  } } */
+
+