* [patch] Fix another fallout of partial inlining change
@ 2013-09-06 9:09 Eric Botcazou
2013-09-06 9:26 ` Jan Hubicka
0 siblings, 1 reply; 3+ messages in thread
From: Eric Botcazou @ 2013-09-06 9:09 UTC (permalink / raw)
To: gcc-patches; +Cc: Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 560 bytes --]
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).
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.
2013-09-06 Eric Botcazou <ebotcazou@adacore.com>
* gnat.dg/warn10.ad[sb]: New test.
* gnat.dg/warn10_pkg.ads: New helper.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 671 bytes --]
Index: ipa-split.c
===================================================================
--- ipa-split.c (revision 202287)
+++ ipa-split.c (working copy)
@@ -1222,6 +1222,9 @@ split_function (struct split_point *spli
DECL_BUILT_IN_CLASS (node->symbol.decl) = NOT_BUILT_IN;
DECL_FUNCTION_CODE (node->symbol.decl) = (enum built_in_function) 0;
}
+ /* If the original function is declared inline, there is no point in issuing
+ a warning for the non-inlinable part. */
+ DECL_NO_INLINE_WARNING_P (node->symbol.decl) = 1;
cgraph_node_remove_callees (cur_node);
ipa_remove_all_references (&cur_node->symbol.ref_list);
if (!split_part_return_p)
[-- Attachment #3: warn10.adb --]
[-- Type: text/x-adasrc, Size: 225 bytes --]
-- { dg-do compile }
-- { dg-options "-O3 -gnatn -Winline" }
package body Warn10 is
procedure Do_Something(Driver : My_Driver) is
X : Float;
begin
X := Get_Input_Value( Driver, 1, 1);
end;
end Warn10;
[-- Attachment #4: warn10.ads --]
[-- Type: text/x-adasrc, Size: 193 bytes --]
with Warn10_Pkg; use Warn10_Pkg;
package Warn10 is
type My_Driver is new Root with record
Extra : Natural;
end record;
procedure Do_Something(Driver : My_Driver);
end Warn10;
[-- Attachment #5: warn10_pkg.ads --]
[-- Type: text/x-adasrc, Size: 281 bytes --]
package Warn10_Pkg is
Size : constant Natural := 100;
type My_Array is array(1..Size, 1..Size) of Float;
type Root is tagged record
Input_Values : My_Array;
end record;
function Get_Input_Value( Driver : Root; I, J : Natural) return Float;
end Warn10_Pkg;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix another fallout of partial inlining change
2013-09-06 9:09 [patch] Fix another fallout of partial inlining change Eric Botcazou
@ 2013-09-06 9:26 ` Jan Hubicka
2013-09-06 9:37 ` Eric Botcazou
0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2013-09-06 9:26 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Jan Hubicka
> 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
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" } } */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix another fallout of partial inlining change
2013-09-06 9:26 ` Jan Hubicka
@ 2013-09-06 9:37 ` Eric Botcazou
0 siblings, 0 replies; 3+ messages in thread
From: Eric Botcazou @ 2013-09-06 9:37 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches
> 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.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-06 9:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-06 9:09 [patch] Fix another fallout of partial inlining change Eric Botcazou
2013-09-06 9:26 ` Jan Hubicka
2013-09-06 9:37 ` Eric Botcazou
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).