diff mbox

Fix another fallout of partial inlining change

Message ID 20130906092630.GA5140@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Sept. 6, 2013, 9:26 a.m. UTC 
> Hi,
> 
> see http://gcc.gnu.org/ml/gcc/2013-09/msg00028.html for the context.
> The patch sets DECL_NO_INLINE_WARNING_P on the non-inlinable part after 
> splitting (an alternative would be to clear DECL_DECLARED_INLINE_P).

Sorry, I missed your mail and it seems that my original mail did not
hit the mailing list.  I am attaching what I wrote back then for a record.
The patch fixes situation where function is externaly visible and called
once.  In this case it makes sense to partially inline it into the
one caller.  Previous heuristic was wrong assuming that the function is
static and thus preventing splitting because it makes more sense to inline
it always.
> 
> Tested on x86_64-suse-linux, OK for the mainline?
> 
> 
> 2013-09-06  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* ipa-split.c (split_function): Set DECL_NO_INLINE_WARNING_P on the
> 	non-inlinable part.

Yes, this is OK.
Thinking about it, we ought to prvent splitting always_inline functions: those
may contain something that relies on inlining.  Either you can include in your
change or I will fix it as a followup.

Thank you,
Honza


Hi,
while analyzing profile issues with Martin I noticed that the testcases bellow
demonstrate three different bugs in profling and splitting.

This patch fixes first of it - the function t() should be split and partially
inlinined, while it is not.

Bootstrapped/regtested ppc64-linux, will commit it shortly.
Honza

Comments

Eric Botcazou Sept. 6, 2013, 9:36 a.m. UTC | #1 
> Sorry, I missed your mail and it seems that my original mail did not
> hit the mailing list.  I am attaching what I wrote back then for a record.
> The patch fixes situation where function is externaly visible and called
> once.  In this case it makes sense to partially inline it into the
> one caller.  Previous heuristic was wrong assuming that the function is
> static and thus preventing splitting because it makes more sense to inline
> it always.

OK, thanks for the explanation.

> Thinking about it, we ought to prvent splitting always_inline functions:
> those may contain something that relies on inlining.  Either you can
> include in your change or I will fix it as a followup.

I'll let you make the change.
diff mbox

Patch

Index: ipa-split.c
===================================================================
--- ipa-split.c	(revision 202153)
+++ ipa-split.c	(working copy)
@@ -1537,7 +1538,9 @@  execute_split_functions (void)
      Note that we are not completely conservative about disqualifying functions
      called once.  It is possible that the caller is called more then once and
      then inlining would still benefit.  */
-  if ((!node->callers || !node->callers->next_caller)
+  if ((!node->callers
+       /* Local functions called once will be completely inlined most of time.  */
+       || (!node->callers->next_caller && node->local.local))
       && !node->symbol.address_taken
       && (!flag_lto || !node->symbol.externally_visible))
     {
Index: testsuite/gcc.dg/tree-ssa/fnsplit-1.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/fnsplit-1.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/fnsplit-1.c	(working copy)
@@ -0,0 +1,23 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fnsplit" } */
+#include <stdio.h>
+int a[1000];
+
+void
+t(int a)
+{
+  if (a)
+    printf ("I Am Completely Operational,"),
+    printf ("And All My Circuits Are Functioning Perfectly\n");
+}
+int
+main(void)
+{
+  int i;
+  for (i = 0; i < 1000; i++)
+    t(a[i]);
+  return 0;
+}
+/* { dg-final { scan-tree-dump-times "Splitting function at:" 1 "fnsplit"} } */
+
+/* { dg-final { cleanup-tree-dump "fnsplit" } } */