public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR ipa/63576: Process speculative edges in ICF
@ 2014-10-27 15:41 Ilya Palachev
  2014-10-27 15:46 ` Jiong Wang
  0 siblings, 1 reply; 3+ messages in thread
From: Ilya Palachev @ 2014-10-27 15:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: Vyacheslav Barinov, Ilya Palachev

Hi all,

The attached patch is an attempt to fix the bug PR ipa/63576.
As it is said in the comment to the bug,

Jan Hubicka wrote:
> THen you need to sum counts (instead of taking ones from BB) and
> turn them back to frequencies (because it is profile only counts
> should be non-0)

It seems that counts and frequencies are gathered in some special
manner, and this patch simply adds counts from speculative edges and
from basic blocks. Of course, I don't know whether this way is proper
one, so please correct me or redirect to right place where it is
documented :)

Anyway, the patch was bootstrapped and regtested on
x86_64-unknown-linux-gnu.

Ok for trunk? 

gcc/

2014-10-22  Ilya Palachev  <i.palachev@samsung.com>

	* ipa-utils.c (compute_edge_count_and_frequency): New function
	(ipa_merge_profiles): handle speculative case
---
 gcc/ipa-utils.c | 52 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 552071e..335ab05 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -380,6 +380,46 @@ get_base_var (tree t)
   return t;
 }
 
+/* Computes count and frequency for edges.  */
+
+static void
+compute_edge_count_and_frequency(struct cgraph_edge *e,
+				 struct cgraph_node *dst)
+{
+  basic_block bb = gimple_bb (e->call_stmt);
+  if (!e->speculative)
+    {
+      e->count = bb->count;
+      e->frequency = compute_call_stmt_bb_frequency (dst->decl, bb);
+    }
+  else
+    {
+      if (profile_status_for_fn (DECL_STRUCT_FUNCTION (dst->decl))
+		      == PROFILE_ABSENT)
+        {
+	  e->count = bb->count;
+	  e->frequency = CGRAPH_FREQ_BASE;
+	}
+      else
+        {
+	  gcc_assert (e->count >= 0);
+	  e->count += bb->count;
+	  gcc_assert (e->frequency >= 0);
+
+	  int entry_freq = ENTRY_BLOCK_PTR_FOR_FN
+  				(DECL_STRUCT_FUNCTION (dst->decl))->frequency;
+	  int freq = e->frequency + bb->frequency;
+
+	  if (!entry_freq)
+	    entry_freq = 1, freq++;
+
+	  freq = freq * CGRAPH_FREQ_BASE / entry_freq;
+	  if (freq > CGRAPH_FREQ_MAX)
+	    freq = CGRAPH_FREQ_MAX;
+	  e->frequency = freq;
+	}
+    }
+}
 
 /* SRC and DST are going to be merged.  Take SRC's profile and merge it into
    DST so it is not going to be lost.  Destroy SRC's body on the way.  */
@@ -537,19 +577,11 @@ ipa_merge_profiles (struct cgraph_node *dst,
       pop_cfun ();
       for (e = dst->callees; e; e = e->next_callee)
 	{
-	  gcc_assert (!e->speculative);
-	  e->count = gimple_bb (e->call_stmt)->count;
-	  e->frequency = compute_call_stmt_bb_frequency
-			     (dst->decl,
-			      gimple_bb (e->call_stmt));
+	  compute_edge_count_and_frequency(e, dst);
 	}
       for (e = dst->indirect_calls; e; e = e->next_callee)
 	{
-	  gcc_assert (!e->speculative);
-	  e->count = gimple_bb (e->call_stmt)->count;
-	  e->frequency = compute_call_stmt_bb_frequency
-			     (dst->decl,
-			      gimple_bb (e->call_stmt));
+	  compute_edge_count_and_frequency(e, dst);
 	}
       src->release_body ();
       inline_update_overall_summary (dst);
-- 
2.1.1

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

* Re: [PATCH] PR ipa/63576: Process speculative edges in ICF
  2014-10-27 15:41 [PATCH] PR ipa/63576: Process speculative edges in ICF Ilya Palachev
@ 2014-10-27 15:46 ` Jiong Wang
  2014-10-29 14:58   ` [PATCHv2] " Ilya Palachev
  0 siblings, 1 reply; 3+ messages in thread
From: Jiong Wang @ 2014-10-27 15:46 UTC (permalink / raw)
  To: Ilya Palachev, gcc-patches; +Cc: Vyacheslav Barinov


On 27/10/14 15:30, Ilya Palachev wrote:
> Hi all,
>
> The attached patch is an attempt to fix the bug PR ipa/63576.
> As it is said in the comment to the bug,
>
> Jan Hubicka wrote:
>> THen you need to sum counts (instead of taking ones from BB) and
>> turn them back to frequencies (because it is profile only counts
>> should be non-0)
> It seems that counts and frequencies are gathered in some special
> manner, and this patch simply adds counts from speculative edges and
> from basic blocks. Of course, I don't know whether this way is proper
> one, so please correct me or redirect to right place where it is
> documented :)
>
> Anyway, the patch was bootstrapped and regtested on
> x86_64-unknown-linux-gnu.
>
> Ok for trunk?
>
> gcc/
>
> 2014-10-22  Ilya Palachev  <i.palachev@samsung.com>
>
> 	* ipa-utils.c (compute_edge_count_and_frequency): New function
> 	(ipa_merge_profiles): handle speculative case
> ---
>   gcc/ipa-utils.c | 52 ++++++++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
> index 552071e..335ab05 100644
> --- a/gcc/ipa-utils.c
> +++ b/gcc/ipa-utils.c
> @@ -380,6 +380,46 @@ get_base_var (tree t)
>     return t;
>   }
>   
> +/* Computes count and frequency for edges.  */
> +
> +static void
> +compute_edge_count_and_frequency(struct cgraph_edge *e,
> +				 struct cgraph_node *dst)
> +{
> +  basic_block bb = gimple_bb (e->call_stmt);
> +  if (!e->speculative)
> +    {
> +      e->count = bb->count;
> +      e->frequency = compute_call_stmt_bb_frequency (dst->decl, bb);
> +    }
> +  else
> +    {
> +      if (profile_status_for_fn (DECL_STRUCT_FUNCTION (dst->decl))
> +		      == PROFILE_ABSENT)
> +        {
> +	  e->count = bb->count;
> +	  e->frequency = CGRAPH_FREQ_BASE;
> +	}
> +      else
> +        {
> +	  gcc_assert (e->count >= 0);
> +	  e->count += bb->count;
> +	  gcc_assert (e->frequency >= 0);
> +
> +	  int entry_freq = ENTRY_BLOCK_PTR_FOR_FN
> +  				(DECL_STRUCT_FUNCTION (dst->decl))->frequency;
> +	  int freq = e->frequency + bb->frequency;
> +
> +	  if (!entry_freq)
> +	    entry_freq = 1, freq++;
> +
> +	  freq = freq * CGRAPH_FREQ_BASE / entry_freq;
> +	  if (freq > CGRAPH_FREQ_MAX)
> +	    freq = CGRAPH_FREQ_MAX;
> +	  e->frequency = freq;
> +	}
> +    }
> +}

how about using early exit for above code, something like:

   if (!e->speculative
       || profile_status_for_fn (DECL_STRUCT_FUNCTION (dst->decl))
		             == PROFILE_ABSEN))
     {
       e->count = bb->count;
       e->frequency = (e->speculative ? CGRAPH_FREQ_BASE
                       : compute_call_stmt_bb_frequency (dst->decl, bb));
       return;
     }
  
   gcc_assert (e->count >= 0);
   ...
   ...


>   
>   /* SRC and DST are going to be merged.  Take SRC's profile and merge it into
>      DST so it is not going to be lost.  Destroy SRC's body on the way.  */
> @@ -537,19 +577,11 @@ ipa_merge_profiles (struct cgraph_node *dst,
>         pop_cfun ();
>         for (e = dst->callees; e; e = e->next_callee)
>   	{
> -	  gcc_assert (!e->speculative);
> -	  e->count = gimple_bb (e->call_stmt)->count;
> -	  e->frequency = compute_call_stmt_bb_frequency
> -			     (dst->decl,
> -			      gimple_bb (e->call_stmt));
> +	  compute_edge_count_and_frequency(e, dst);
>   	}
>         for (e = dst->indirect_calls; e; e = e->next_callee)
>   	{
> -	  gcc_assert (!e->speculative);
> -	  e->count = gimple_bb (e->call_stmt)->count;
> -	  e->frequency = compute_call_stmt_bb_frequency
> -			     (dst->decl,
> -			      gimple_bb (e->call_stmt));
> +	  compute_edge_count_and_frequency(e, dst);
>   	}
>         src->release_body ();
>         inline_update_overall_summary (dst);


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

* [PATCHv2] PR ipa/63576: Process speculative edges in ICF
  2014-10-27 15:46 ` Jiong Wang
@ 2014-10-29 14:58   ` Ilya Palachev
  0 siblings, 0 replies; 3+ messages in thread
From: Ilya Palachev @ 2014-10-29 14:58 UTC (permalink / raw)
  To: gcc-patches; +Cc: Vyacheslav Barinov, Ilya Palachev, Jiong Wang, Jan Hubicka

Hi all,

This patch is an attempt to fix bug PR ipa/63576, corrected according
to note made by Jiong Wang:

On 27.10.2014 18:41, Jiong Wang wrote:
> how about using early exit for above code, something like:
>
>   if (!e->speculative
>       || profile_status_for_fn (DECL_STRUCT_FUNCTION (dst->decl))
>                      == PROFILE_ABSEN))
>     {
>       e->count = bb->count;
>       e->frequency = (e->speculative ? CGRAPH_FREQ_BASE
>                       : compute_call_stmt_bb_frequency (dst->decl, bb));
>       return;
>     }
>  
>   gcc_assert (e->count >= 0);
>   ...
>   ...

Ok, that's right idea.

Jan Hubicka wrote:
> THen you need to sum counts (instead of taking ones from BB) and
> turn them back to frequencies (because it is profile only counts
> should be non-0)

It seems that counts and frequencies are gathered in some special
manner, and this patch simply adds counts from speculative edges and
from basic blocks. Of course, I don't know whether this way is proper
one, so please correct me or redirect to right place where it is
documented.

Honza, can you explain your comment to the bug?

I've changed the patch, bootstrapped and
regtested on x86_64-unknown-linux-gnu again.

Ok for trunk?

--
Best regards,
Ilya Palachev

-------------------------------------------------------------------------------

gcc/

2014-10-27  Ilya Palachev  <i.palachev@samsung.com>

	* ipa-utils.c (compute_edge_count_and_frequency): New function
	(ipa_merge_profiles): handle speculative case
---
 gcc/ipa-utils.c | 43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index e4ea84c..177a170 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -390,6 +390,37 @@ get_base_var (tree t)
   return t;
 }
 
+/* Computes count and frequency for edges.  */
+
+static void
+compute_edge_count_and_frequency (struct cgraph_edge *e,
+				  struct cgraph_node *dst)
+{
+  basic_block bb = gimple_bb (e->call_stmt);
+  if (!e->speculative
+      || profile_status_for_fn (DECL_STRUCT_FUNCTION (dst->decl))
+		== PROFILE_ABSENT)
+    {
+      e->count = bb->count;
+      e->frequency = compute_call_stmt_bb_frequency (dst->decl, bb);
+      return;
+    }
+  gcc_assert (e->count >= 0);
+  e->count += bb->count;
+  gcc_assert (e->frequency >= 0);
+
+  int entry_freq = ENTRY_BLOCK_PTR_FOR_FN
+			(DECL_STRUCT_FUNCTION (dst->decl))->frequency;
+  int freq = e->frequency + bb->frequency;
+
+  if (!entry_freq)
+    entry_freq = 1, freq++;
+
+  freq = freq * CGRAPH_FREQ_BASE / entry_freq;
+  if (freq > CGRAPH_FREQ_MAX)
+    freq = CGRAPH_FREQ_MAX;
+  e->frequency = freq;
+}
 
 /* SRC and DST are going to be merged.  Take SRC's profile and merge it into
    DST so it is not going to be lost.  Destroy SRC's body on the way.  */
@@ -547,19 +578,11 @@ ipa_merge_profiles (struct cgraph_node *dst,
       pop_cfun ();
       for (e = dst->callees; e; e = e->next_callee)
 	{
-	  gcc_assert (!e->speculative);
-	  e->count = gimple_bb (e->call_stmt)->count;
-	  e->frequency = compute_call_stmt_bb_frequency
-			     (dst->decl,
-			      gimple_bb (e->call_stmt));
+	  compute_edge_count_and_frequency (e, dst);
 	}
       for (e = dst->indirect_calls; e; e = e->next_callee)
 	{
-	  gcc_assert (!e->speculative);
-	  e->count = gimple_bb (e->call_stmt)->count;
-	  e->frequency = compute_call_stmt_bb_frequency
-			     (dst->decl,
-			      gimple_bb (e->call_stmt));
+	  compute_edge_count_and_frequency (e, dst);
 	}
       src->release_body ();
       inline_update_overall_summary (dst);
-- 
2.1.1

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

end of thread, other threads:[~2014-10-29 14:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-27 15:41 [PATCH] PR ipa/63576: Process speculative edges in ICF Ilya Palachev
2014-10-27 15:46 ` Jiong Wang
2014-10-29 14:58   ` [PATCHv2] " Ilya Palachev

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