diff mbox

Fix order of ENTRY and EXIT in reverse post order

Message ID alpine.LSU.2.20.1508271621080.30229@wotan.suse.de
State New
Headers show

Commit Message

Michael Matz Aug. 27, 2015, 2:33 p.m. UTC
Hello,

I've this in my tree since some time already.  In-tree there is only one 
user of pre_and_rev_post_order_compute{,_fn} that actually wants entry and 
exit included, and that one is just a debug routine 
(draw_cfg_nodes_no_loops); so this bug right now is harmless.  But I've 
used this for some other patches, and in particular the hsa branch now 
introduces a use that's really interested in entry and exit blocks (only 
for code quality, but hey).

So, to demonstrate, given such CFG:

   EN -> 1 -> 2 -> EX
         |--> 3 /

One pre order would be:
EN, 1, 2, 3, EX

One post order would be:
EX, 2, 3, 1, EN

And therefore a reverse post order would be:
EN, 1, 3, 2, EX

In particular in a reverse post order the entry block comes first, and the 
exit block last.  The routine contains a thinko in that it places the 
entry block last, and the exit block first.  The bug exists since the 
introduction of the include_entry_exit parameter.

I'm regstrapping with this in-tree since many moons, but just started 
another one; of course, given that there aren't non-debug users right now, 
no changes in testsuites are expected.

Okay for trunk?


Ciao,
Michael.
------------------
	* cfganal.c (pre_and_rev_post_order_compute_fn): Correctly
	enter entry and exit blocks for reverse post order.

Comments

Richard Biener Aug. 27, 2015, 2:57 p.m. UTC | #1
On Thu, Aug 27, 2015 at 4:33 PM, Michael Matz <matz@suse.de> wrote:
> Hello,
>
> I've this in my tree since some time already.  In-tree there is only one
> user of pre_and_rev_post_order_compute{,_fn} that actually wants entry and
> exit included, and that one is just a debug routine
> (draw_cfg_nodes_no_loops); so this bug right now is harmless.  But I've
> used this for some other patches, and in particular the hsa branch now
> introduces a use that's really interested in entry and exit blocks (only
> for code quality, but hey).
>
> So, to demonstrate, given such CFG:
>
>    EN -> 1 -> 2 -> EX
>          |--> 3 /
>
> One pre order would be:
> EN, 1, 2, 3, EX
>
> One post order would be:
> EX, 2, 3, 1, EN
>
> And therefore a reverse post order would be:
> EN, 1, 3, 2, EX
>
> In particular in a reverse post order the entry block comes first, and the
> exit block last.  The routine contains a thinko in that it places the
> entry block last, and the exit block first.  The bug exists since the
> introduction of the include_entry_exit parameter.
>
> I'm regstrapping with this in-tree since many moons, but just started
> another one; of course, given that there aren't non-debug users right now,
> no changes in testsuites are expected.
>
> Okay for trunk?

Ok.

Richard.

>
> Ciao,
> Michael.
> ------------------
>         * cfganal.c (pre_and_rev_post_order_compute_fn): Correctly
>         enter entry and exit blocks for reverse post order.
>
> Index: cfganal.c
> ===================================================================
> --- cfganal.c   (revision 227259)
> +++ cfganal.c   (working copy)
> @@ -925,7 +925,7 @@ pre_and_rev_post_order_compute_fn (struc
>         pre_order[pre_order_num] = ENTRY_BLOCK;
>        pre_order_num++;
>        if (rev_post_order)
> -       rev_post_order[rev_post_order_num--] = ENTRY_BLOCK;
> +       rev_post_order[rev_post_order_num--] = EXIT_BLOCK;
>      }
>    else
>      rev_post_order_num -= NUM_FIXED_BLOCKS;
> @@ -996,7 +996,7 @@ pre_and_rev_post_order_compute_fn (struc
>         pre_order[pre_order_num] = EXIT_BLOCK;
>        pre_order_num++;
>        if (rev_post_order)
> -       rev_post_order[rev_post_order_num--] = EXIT_BLOCK;
> +       rev_post_order[rev_post_order_num--] = ENTRY_BLOCK;
>      }
>
>    return pre_order_num;
diff mbox

Patch

Index: cfganal.c
===================================================================
--- cfganal.c	(revision 227259)
+++ cfganal.c	(working copy)
@@ -925,7 +925,7 @@  pre_and_rev_post_order_compute_fn (struc
 	pre_order[pre_order_num] = ENTRY_BLOCK;
       pre_order_num++;
       if (rev_post_order)
-	rev_post_order[rev_post_order_num--] = ENTRY_BLOCK;
+	rev_post_order[rev_post_order_num--] = EXIT_BLOCK;
     }
   else
     rev_post_order_num -= NUM_FIXED_BLOCKS;
@@ -996,7 +996,7 @@  pre_and_rev_post_order_compute_fn (struc
 	pre_order[pre_order_num] = EXIT_BLOCK;
       pre_order_num++;
       if (rev_post_order)
-	rev_post_order[rev_post_order_num--] = EXIT_BLOCK;
+	rev_post_order[rev_post_order_num--] = ENTRY_BLOCK;
     }
 
   return pre_order_num;