public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: PR bootstrap/50709 (bootstrap miscompare)
Date: Thu, 20 Oct 2011 13:08:00 -0000	[thread overview]
Message-ID: <20111020122317.GA1489@kam.mff.cuni.cz> (raw)
In-Reply-To: <20111019164149.GG18858@kam.mff.cuni.cz>

> @@ -1392,16 +1393,20 @@ inline_small_functions (void)
>        if (!edge->inline_failed)
>  	continue;
>  
> -      /* Be sure that caches are maintained consistent.  */
>  #ifdef ENABLE_CHECKING
> +      /* Be sure that caches are maintained conservatively consistent.
> +	 This means that cached badness is allways smaller or equal
> +	 to the real badness.  */
> +      cached_badness = edge_badness (edge, false);
> +#endif
>        reset_edge_growth_cache (edge);
>        reset_node_growth_cache (edge->callee);
> -#endif
>  
>        /* When updating the edge costs, we only decrease badness in the keys.
>  	 Increases of badness are handled lazilly; when we see key with out
>  	 of date value on it, we re-insert it now.  */
>        current_badness = edge_badness (edge, false);
> +      gcc_assert (cached_badness == -1 || cached_badness <= current_badness);

This new check actually cathes a bug that is in tree since introduction of new
ipa-inline-analysis code.  The inliner assume that when it produce a new inline
copy, the overall growth estimates for all callees can only degrade.  This is
not quite true: when a new knowledge is propagated, the callees might actually
become cheaper and reduce the growth.

This patch takes the easy but expensive way to plug the problem by forcing
updating of all keys in the queue.  It increases LTO compile time of Mozilla to
10 minutes, so I will need to develop better sollution. (the trick saving
recomputation was originally introduced to reduce copmile time particularly on
this testcase) Just I should not keep tree ICEing on many C++ sources until I
am done.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 180247)
+++ ChangeLog	(working copy)
@@ -1,5 +1,10 @@
 2011-10-19  Jan Hubicka  <jh@suse.cz>
 
+	* ipa-inline.c (inline_small_functions): Always update all calles after
+	inlining.
+
+2011-10-19  Jan Hubicka  <jh@suse.cz>
+
 	PR bootstrap/50709
 	* ipa-inline.c (inline_small_functions): Fix checking code to not make
 	effect on fibheap stability.
Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 180247)
+++ ipa-inline.c	(working copy)
@@ -1515,8 +1515,13 @@ inline_small_functions (void)
 
 	  /* We inlined last offline copy to the body.  This might lead
 	     to callees of function having fewer call sites and thus they
-	     may need updating.  */
-	  if (callee->global.inlined_to)
+	     may need updating. 
+
+	     FIXME: the callee size could also shrink because more information
+	     is propagated from caller.  We don't track when this happen and
+	     thus we need to recompute everything all the time.  Once this is
+	     solved, "|| 1" should go away.  */
+	  if (callee->global.inlined_to || 1)
 	    update_all_callee_keys (heap, callee, updated_nodes);
 	  else
 	    update_callee_keys (heap, edge->callee, updated_nodes);

  reply	other threads:[~2011-10-20 12:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-19 17:15 Jan Hubicka
2011-10-20 13:08 ` Jan Hubicka [this message]
2011-10-25 16:47   ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111020122317.GA1489@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).