[PATCH] Fix -fcompare-debug issue in tree profiler (PR debug/46255)

[PATCH] Fix -fcompare-debug issue in tree profiler (PR debug/46255)

[Jakub Jelinek]

Hi!

While debug stmts are never considered to need fake edges, it currently
(incorrectly) makes a difference whether a stmt that needs fake edges
was the last one or not.  Fixed by disregarding any debug stmts
at the end of bb.

Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?

2010-11-01  Jakub Jelinek  <jakub@redhat.com>

	PR debug/46255
	* tree-cfg.c (gimple_flow_call_edges_add): Use gsi_last_nondebug_bb
	instead of gsi_last_bb.

	* gcc.dg/pr46255.c: New test.

--- gcc/tree-cfg.c.jj	2010-11-01 09:07:24.000000000 +0100
+++ gcc/tree-cfg.c	2010-11-01 14:03:05.000000000 +0100
@@ -6749,7 +6749,7 @@ gimple_flow_call_edges_add (sbitmap bloc
   if (check_last_block)
     {
       basic_block bb = EXIT_BLOCK_PTR->prev_bb;
-      gimple_stmt_iterator gsi = gsi_last_bb (bb);
+      gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb);
       gimple t = NULL;
 
       if (!gsi_end_p (gsi))
@@ -6783,7 +6783,7 @@ gimple_flow_call_edges_add (sbitmap bloc
       if (blocks && !TEST_BIT (blocks, i))
 	continue;
 
-      gsi = gsi_last_bb (bb);
+      gsi = gsi_last_nondebug_bb (bb);
       if (!gsi_end_p (gsi))
 	{
 	  last_stmt = gsi_stmt (gsi);
--- gcc/testsuite/gcc.dg/pr46255.c.jj	2010-11-01 14:04:55.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46255.c	2010-11-01 14:04:34.000000000 +0100
@@ -0,0 +1,12 @@
+/* PR debug/46255 */
+/* { dg-do compile } */
+/* { dg-options "-fcompare-debug -fprofile-generate -O" } */
+
+int bar (void);
+
+void
+foo (int i)
+{
+  while (i)
+    i = bar ();
+}

	Jakub

Re: [PATCH] Fix -fcompare-debug issue in tree profiler (PR debug/46255)

[Richard Guenther]

On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> While debug stmts are never considered to need fake edges, it currently
> (incorrectly) makes a difference whether a stmt that needs fake edges
> was the last one or not.  Fixed by disregarding any debug stmts
> at the end of bb.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?

Hmm, you mean that the stmt needing fake edges doesn't end the BB?
So your patch fixes that by splitting the BB at the appropriate point?

If so, it's ok.

Thanks,
Richard.

> 2010-11-01  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/46255
>        * tree-cfg.c (gimple_flow_call_edges_add): Use gsi_last_nondebug_bb
>        instead of gsi_last_bb.
>
>        * gcc.dg/pr46255.c: New test.
>
> --- gcc/tree-cfg.c.jj   2010-11-01 09:07:24.000000000 +0100
> +++ gcc/tree-cfg.c      2010-11-01 14:03:05.000000000 +0100
> @@ -6749,7 +6749,7 @@ gimple_flow_call_edges_add (sbitmap bloc
>   if (check_last_block)
>     {
>       basic_block bb = EXIT_BLOCK_PTR->prev_bb;
> -      gimple_stmt_iterator gsi = gsi_last_bb (bb);
> +      gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb);
>       gimple t = NULL;
>
>       if (!gsi_end_p (gsi))
> @@ -6783,7 +6783,7 @@ gimple_flow_call_edges_add (sbitmap bloc
>       if (blocks && !TEST_BIT (blocks, i))
>        continue;
>
> -      gsi = gsi_last_bb (bb);
> +      gsi = gsi_last_nondebug_bb (bb);
>       if (!gsi_end_p (gsi))
>        {
>          last_stmt = gsi_stmt (gsi);
> --- gcc/testsuite/gcc.dg/pr46255.c.jj   2010-11-01 14:04:55.000000000 +0100
> +++ gcc/testsuite/gcc.dg/pr46255.c      2010-11-01 14:04:34.000000000 +0100
> @@ -0,0 +1,12 @@
> +/* PR debug/46255 */
> +/* { dg-do compile } */
> +/* { dg-options "-fcompare-debug -fprofile-generate -O" } */
> +
> +int bar (void);
> +
> +void
> +foo (int i)
> +{
> +  while (i)
> +    i = bar ();
> +}
>
>        Jakub
>

Re: [PATCH] Fix -fcompare-debug issue in tree profiler (PR debug/46255)

[Jakub Jelinek]

On Mon, Nov 01, 2010 at 11:29:57PM +0100, Richard Guenther wrote:
> On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > While debug stmts are never considered to need fake edges, it currently
> > (incorrectly) makes a difference whether a stmt that needs fake edges
> > was the last one or not.  Fixed by disregarding any debug stmts
> > at the end of bb.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?
> 
> Hmm, you mean that the stmt needing fake edges doesn't end the BB?

Yes.

> So your patch fixes that by splitting the BB at the appropriate point?

No, that was what the code was doing before and that caused the
-fcompare-debug differences, because without -g the bb was not split, with
-g it was split.
profile.c uses block_ends_with_call_p, which for gimple uses
/* Return true if BB ends with a call, possibly followed by some
   instructions that must stay with the call.  Return false,
   otherwise.  */

static bool
gimple_block_ends_with_call_p (basic_block bb)
{
  gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb);
  return !gsi_end_p (gsi) && is_gimple_call (gsi_stmt (gsi));
}

Having the fake edge gong not immediately from the call, but
from the end of bb where the call is followed only by debug stmts,
seems to work just fine, all it means is the profiling stuff
gets emitted before or after those debug stmts which really result in
no code being generated.

	Jakub

Re: [PATCH] Fix -fcompare-debug issue in tree profiler (PR debug/46255)

[Richard Guenther]

On Tue, Nov 2, 2010 at 12:07 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Nov 01, 2010 at 11:29:57PM +0100, Richard Guenther wrote:
>> On Mon, Nov 1, 2010 at 8:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > While debug stmts are never considered to need fake edges, it currently
>> > (incorrectly) makes a difference whether a stmt that needs fake edges
>> > was the last one or not.  Fixed by disregarding any debug stmts
>> > at the end of bb.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?
>>
>> Hmm, you mean that the stmt needing fake edges doesn't end the BB?
>
> Yes.
>
>> So your patch fixes that by splitting the BB at the appropriate point?
>
> No, that was what the code was doing before and that caused the
> -fcompare-debug differences, because without -g the bb was not split, with
> -g it was split.

Hmm, I remember we drop DEBUG statements after calls that throw
to be able to split the BB at the appropriate point.  Do we?

> profile.c uses block_ends_with_call_p, which for gimple uses
> /* Return true if BB ends with a call, possibly followed by some
>   instructions that must stay with the call.  Return false,
>   otherwise.  */
>
> static bool
> gimple_block_ends_with_call_p (basic_block bb)
> {
>  gimple_stmt_iterator gsi = gsi_last_nondebug_bb (bb);
>  return !gsi_end_p (gsi) && is_gimple_call (gsi_stmt (gsi));
> }
>
> Having the fake edge gong not immediately from the call, but
> from the end of bb where the call is followed only by debug stmts,
> seems to work just fine, all it means is the profiling stuff
> gets emitted before or after those debug stmts which really result in
> no code being generated.

I guess as they are fake edges and only profiling uses it (but it's
a CFG hook also implemented on RTL ... huh...) it probably doesn't matter.

So, the patch is ok.  It would be nice to make this localized though
and remove the hookization of this functionality.

Richard.

>        Jakub
>