public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Disable iteration in early inliner
@ 2013-01-23 10:18 Jan Hubicka
  2013-01-23 10:29 ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2013-01-23 10:18 UTC (permalink / raw)
  To: gcc-patches, rguenther, jakub

Hi,
this patch disables iteration of early inliner. I added the code in early days
of indirect inlining mostly to catch things that are not well handled by IPA
inliner.  We got better on devirtualization and indirect inlining and I think
we now handle pretty much all interesting cases there.

Disabling iteration prevents problem with unbounded recursive inlining of
strongly connected components Richi observed. On the other hand we still loose
optimization on dead code removal: DCE past early inlining usually removes
&function statement leading to the offline copy of the function to be optimized
out for simple chains of more than one indirect inlining.
This is rather infrequent scenario though - usually you have one simple wrapper
with calback, not simple wrapper with callback and simple callback with another callback.

Bootstrapped/regtested x86_64-linux, compensated the 3 testcases in tesuite testing
the feature to both test the feature with parameter increased and thest that the
optimization happens anyway at IPA level by default.

I also benchmarke SPEC2k/mozilla/spec2k6/C++ benchmark and found no regressions.
Will commit it tomorrow if there are no complains.

Honza

	* g++.dg/tree-ssa/inline-1.C: Update max-inliner-iterations.
	* g++.dg/tree-ssa/inline-2.C: Update max-inliner-iterations.
	* g++.dg/tree-ssa/inline-3.C: Update max-inliner-iterations.
	* g++.dg/ipa/inline-1.C: New testcase.
	* g++.dg/ipa/inline-2.C: New testcase.
	* g++.dg/ipa/inline-3.C: New testcase.
	* params.def (PARAM_EARLY_INLINER_MAX_ITERATIONS): Drop to 1.
Index: testsuite/g++.dg/tree-ssa/inline-2.C
===================================================================
*** testsuite/g++.dg/tree-ssa/inline-2.C	(revision 195370)
--- testsuite/g++.dg/tree-ssa/inline-2.C	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline" } */
  /* { dg-add-options bind_pic_locally } */
  
  namespace std {
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline --param max-early-inliner-iterations=3" } */
  /* { dg-add-options bind_pic_locally } */
  
  namespace std {
Index: testsuite/g++.dg/tree-ssa/inline-3.C
===================================================================
*** testsuite/g++.dg/tree-ssa/inline-3.C	(revision 195370)
--- testsuite/g++.dg/tree-ssa/inline-3.C	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline" } */
  /* { dg-add-options bind_pic_locally } */
  
  #include <algorithm>
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline --param max-early-inliner-iterations=3" } */
  /* { dg-add-options bind_pic_locally } */
  
  #include <algorithm>
Index: testsuite/g++.dg/tree-ssa/inline-1.C
===================================================================
*** testsuite/g++.dg/tree-ssa/inline-1.C	(revision 195370)
--- testsuite/g++.dg/tree-ssa/inline-1.C	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline" } */
  /* { dg-add-options bind_pic_locally } */
  
  namespace std {
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-einline --param max-early-inliner-iterations=3" } */
  /* { dg-add-options bind_pic_locally } */
  
  namespace std {
Index: testsuite/g++.dg/torture/20070621-1.C
===================================================================
*** testsuite/g++.dg/torture/20070621-1.C	(revision 195370)
--- testsuite/g++.dg/torture/20070621-1.C	(working copy)
***************
*** 1,3 ****
--- 1,4 ----
+ // { dg-do compile }
  /* Reduced from libstdc++-v3/testsuite/25_algorithms/equal/1.cc
  
  1.2.ii: In function 'void test1()':
Index: testsuite/g++.dg/ipa/inline-1.C
===================================================================
*** testsuite/g++.dg/ipa/inline-1.C	(revision 0)
--- testsuite/g++.dg/ipa/inline-1.C	(revision 0)
***************
*** 0 ****
--- 1,36 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-ipa-inline --param max-early-inliner-iterations=1" } */
+ /* { dg-add-options bind_pic_locally } */
+ 
+ namespace std {
+   extern "C" void puts(const char *s);
+ }
+ 
+ template <class T, class E> void
+ foreach (T b, T e, void (*ptr)(E))
+ {
+   for (; b != e; b++)
+     ptr(*b);
+ }
+ 
+ void
+ inline_me (char *x)
+ {
+   std::puts(x);
+ }
+ 
+ static void
+ inline_me_too (char *x)
+ {
+   std::puts(x);
+ }
+ 
+ int main(int argc, char **argv)
+ {
+   foreach (argv, argv + argc, inline_me);
+   foreach (argv, argv + argc, inline_me_too);
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "Considering void inline_me\\(" 1 "inline"} } */
+ /* { dg-final { scan-tree-dump-times "Considering void inline_me_too\\(" 1 "inline"} } */
+ /* { dg-final { cleanup-tree-dump "einline" } } */
Index: testsuite/g++.dg/ipa/inline-2.C
===================================================================
*** testsuite/g++.dg/ipa/inline-2.C	(revision 0)
--- testsuite/g++.dg/ipa/inline-2.C	(revision 0)
***************
*** 0 ****
--- 1,36 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-ipa-inline --param max-early-inliner-iterations=1" } */
+ /* { dg-add-options bind_pic_locally } */
+ 
+ namespace std {
+   extern "C" void puts(const char *s);
+ }
+ 
+ template <class T, class E> void
+ foreach (T b, T e, E ptr)
+ {
+   for (; b != e; b++)
+     ptr(*b);
+ }
+ 
+ void
+ inline_me (char *x)
+ {
+   std::puts(x);
+ }
+ 
+ static void
+ inline_me_too (char *x)
+ {
+   std::puts(x);
+ }
+ 
+ int main(int argc, char **argv)
+ {
+   foreach (argv, argv + argc, inline_me);
+   foreach (argv, argv + argc, inline_me_too);
+ }
+ 
+ /* { dg-final { scan-ipa-dump-times "Considering void inline_me\\(" 1 "inline"} } */
+ /* { dg-final { scan-ipa-dump-times "Considering void inline_me_too\\(" 1 "inline"} } */
+ /* { dg-final { cleanup-ipa-dump "inline" } } */
Index: testsuite/g++.dg/ipa/inline-3.C
===================================================================
*** testsuite/g++.dg/ipa/inline-3.C	(revision 0)
--- testsuite/g++.dg/ipa/inline-3.C	(revision 0)
***************
*** 0 ****
--- 1,29 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-ipa-inline --param max-early-inliner-iterations=1" } */
+ /* { dg-add-options bind_pic_locally } */
+ 
+ #include <algorithm>
+ 
+ void foo(const char *s);
+ 
+ void
+ inline_me (char *x)
+ {
+   foo(x);
+ }
+ 
+ static void
+ inline_me_too (char *x)
+ {
+   foo(x);
+ }
+ 
+ int main(int argc, char **argv)
+ {
+   std::for_each (argv, argv + argc, inline_me);
+   std::for_each (argv, argv + argc, inline_me_too);
+ }
+ 
+ /* { dg-final { scan-ipa-dump-times "Considering void inline_me\\(" 1 "inline"} } */
+ /* { dg-final { scan-ipa-dump-times "Considering void inline_me_too\\(" 1 "inline"} } */
+ /* { dg-final { cleanup-tree-dump "inline" } } */
Index: lto-streamer-in.c
===================================================================
*** lto-streamer-in.c	(revision 195370)
--- lto-streamer-in.c	(working copy)
*************** lto_read_tree (struct lto_input_block *i
*** 1032,1037 ****
--- 1032,1039 ----
        && TREE_CODE (result) != TRANSLATION_UNIT_DECL)
      DECL_INITIAL (result) = stream_read_tree (ib, data_in);
  
+   gcc_assert (TREE_CODE (result) != BLOCK || !flag_wpa);
+ 
    /* We should never try to instantiate an MD or NORMAL builtin here.  */
    if (TREE_CODE (result) == FUNCTION_DECL)
      gcc_assert (!streamer_handle_as_builtin_p (result));
Index: params.def
===================================================================
*** params.def	(revision 195370)
--- params.def	(working copy)
*************** DEFPARAM (PARAM_MIN_INLINE_RECURSIVE_PRO
*** 109,115 ****
  DEFPARAM (PARAM_EARLY_INLINER_MAX_ITERATIONS,
  	  "max-early-inliner-iterations",
  	  "The maximum number of nested indirect inlining performed by early inliner",
! 	  10, 0, 0)
  
  /* Limit on probability of entry BB.  */
  DEFPARAM (PARAM_COMDAT_SHARING_PROBABILITY,
--- 109,115 ----
  DEFPARAM (PARAM_EARLY_INLINER_MAX_ITERATIONS,
  	  "max-early-inliner-iterations",
  	  "The maximum number of nested indirect inlining performed by early inliner",
! 	  1, 0, 0)
  
  /* Limit on probability of entry BB.  */
  DEFPARAM (PARAM_COMDAT_SHARING_PROBABILITY,

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Disable iteration in early inliner
  2013-01-23 10:18 Disable iteration in early inliner Jan Hubicka
@ 2013-01-23 10:29 ` Jakub Jelinek
  2013-01-23 13:15   ` Jan Hubicka
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2013-01-23 10:29 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, rguenther

On Wed, Jan 23, 2013 at 11:18:31AM +0100, Jan Hubicka wrote:
> Bootstrapped/regtested x86_64-linux, compensated the 3 testcases in tesuite testing
> the feature to both test the feature with parameter increased and thest that the
> optimization happens anyway at IPA level by default.

Not objecting to the patch, just wondering, if the fix is only about
changing a default of a param, won't the PR55797 testcase still ICE
if (to the -O -fno-guess-branch-probability -fno-tree-forwprop --param=early-inlining-insns=176
options) one also adds --param=max-early-inliner-iterations=2 ?
With --param=max-early-inliner-iterations=1 it indeed no longer ICEs.

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Disable iteration in early inliner
  2013-01-23 10:29 ` Jakub Jelinek
@ 2013-01-23 13:15   ` Jan Hubicka
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Hubicka @ 2013-01-23 13:15 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches, rguenther

> On Wed, Jan 23, 2013 at 11:18:31AM +0100, Jan Hubicka wrote:
> > Bootstrapped/regtested x86_64-linux, compensated the 3 testcases in tesuite testing
> > the feature to both test the feature with parameter increased and thest that the
> > optimization happens anyway at IPA level by default.
> 
> Not objecting to the patch, just wondering, if the fix is only about
> changing a default of a param, won't the PR55797 testcase still ICE
> if (to the -O -fno-guess-branch-probability -fno-tree-forwprop --param=early-inlining-insns=176
> options) one also adds --param=max-early-inliner-iterations=2 ?
> With --param=max-early-inliner-iterations=1 it indeed no longer ICEs.

It is not meant to be complette fix for PR55797, only the code quality issue Richi observed while
debugging it.  I am looking into the ICE now, too.  

Honza

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-01-23 13:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-23 10:18 Disable iteration in early inliner Jan Hubicka
2013-01-23 10:29 ` Jakub Jelinek
2013-01-23 13:15   ` Jan Hubicka

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).