Patchwork [PR45354] sel-sched: don't make EDGE_CROSSING edges fallthrough

login
register
mail settings
Submitter Alexander Monakov
Date Nov. 30, 2010, 1:31 p.m.
Message ID <alpine.LNX.2.00.1011301607320.6883@monoid.intra.ispras.ru>
Download mbox | patch
Permalink /patch/73600/
State New
Headers show

Comments

Alexander Monakov - Nov. 30, 2010, 1:31 p.m.
Hi,

We should not delete jumps if they cross hot/cold partition boundaries.  The
patch adds the corresponding check in bb_has_removable_jump_to_p (which is
renamed from jump_leads_only_to_bb_p, making the name a bit closer to the
purpose).

Bootstrapped and regtested on ia64-linux, x86_64-linux.  OK for trunk?

2010-11-30  Alexander Monakov  <amonakov@ispras.ru>

	PR rtl-optimization/45354
	* sel-sched-ir.c (jump_leads_only_to_bb_p): Rename to ...
	(bb_has_removable_jump_to_p): This.  Update all callers. Make static.
	Allow BBs ending with a conditional jump.  Forbid EDGE_CROSSING jumps.
	* sel-sched-ir.h (jump_leads_only_to_bb_p): Delete prototype.

testsuite:
	* gcc.dg/tree-prof/pr45354.c: New.
Vladimir Makarov - Dec. 2, 2010, 10:18 p.m.
On 11/30/2010 08:31 AM, Alexander Monakov wrote:
> Hi,
>
> We should not delete jumps if they cross hot/cold partition boundaries.  The
> patch adds the corresponding check in bb_has_removable_jump_to_p (which is
> renamed from jump_leads_only_to_bb_p, making the name a bit closer to the
> purpose).
>
> Bootstrapped and regtested on ia64-linux, x86_64-linux.  OK for trunk?
>
Yes.  Thanks for the patch, Alexander.
> 2010-11-30  Alexander Monakov<amonakov@ispras.ru>
>
> 	PR rtl-optimization/45354
> 	* sel-sched-ir.c (jump_leads_only_to_bb_p): Rename to ...
> 	(bb_has_removable_jump_to_p): This.  Update all callers. Make static.
> 	Allow BBs ending with a conditional jump.  Forbid EDGE_CROSSING jumps.
> 	* sel-sched-ir.h (jump_leads_only_to_bb_p): Delete prototype.
H.J. Lu - Dec. 9, 2010, 7:29 p.m.
On Tue, Nov 30, 2010 at 5:31 AM, Alexander Monakov <amonakov@ispras.ru> wrote:
> Hi,
>
> We should not delete jumps if they cross hot/cold partition boundaries.  The
> patch adds the corresponding check in bb_has_removable_jump_to_p (which is
> renamed from jump_leads_only_to_bb_p, making the name a bit closer to the
> purpose).
>
> Bootstrapped and regtested on ia64-linux, x86_64-linux.  OK for trunk?
>
> 2010-11-30  Alexander Monakov  <amonakov@ispras.ru>
>
>        PR rtl-optimization/45354
>        * sel-sched-ir.c (jump_leads_only_to_bb_p): Rename to ...
>        (bb_has_removable_jump_to_p): This.  Update all callers. Make static.
>        Allow BBs ending with a conditional jump.  Forbid EDGE_CROSSING jumps.
>        * sel-sched-ir.h (jump_leads_only_to_bb_p): Delete prototype.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46875

Patch

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 7956cd8..f7099fa 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -156,6 +156,7 @@  static void move_bb_info (basic_block, basic_block);
 static void remove_empty_bb (basic_block, bool);
 static void sel_merge_blocks (basic_block, basic_block);
 static void sel_remove_loop_preheader (void);
+static bool bb_has_removable_jump_to_p (basic_block, basic_block);
 
 static bool insn_is_the_only_one_in_bb_p (insn_t);
 static void create_initial_data_sets (basic_block);
@@ -3675,7 +3676,7 @@  tidy_control_flow (basic_block xbb, bool full_tidying)
     return changed;
 
   /* Check if there is a unnecessary jump after insn left.  */
-  if (jump_leads_only_to_bb_p (BB_END (xbb), xbb->next_bb)
+  if (bb_has_removable_jump_to_p (xbb, xbb->next_bb)
       && INSN_SCHED_TIMES (BB_END (xbb)) == 0
       && !IN_CURRENT_FENCE_P (BB_END (xbb)))
     {
@@ -3716,7 +3717,7 @@  tidy_control_flow (basic_block xbb, bool full_tidying)
       /* And unconditional jump in previous basic block leads to
          next basic block of XBB and this jump can be safely removed.  */
       && in_current_region_p (xbb->prev_bb)
-      && jump_leads_only_to_bb_p (BB_END (xbb->prev_bb), xbb->next_bb)
+      && bb_has_removable_jump_to_p (xbb->prev_bb, xbb->next_bb)
       && INSN_SCHED_TIMES (BB_END (xbb->prev_bb)) == 0
       /* Also this jump is not at the scheduling boundary.  */
       && !IN_CURRENT_FENCE_P (BB_END (xbb->prev_bb)))
@@ -6104,22 +6105,19 @@  sel_is_loop_preheader_p (basic_block bb)
   return false;
 }
 
-/* Checks whether JUMP leads to basic block DEST_BB and no other blocks.  */
-bool
-jump_leads_only_to_bb_p (insn_t jump, basic_block dest_bb)
+/* Check whether JUMP_BB ends with a jump insn that leads only to DEST_BB and
+   can be removed, making the corresponding edge fallthrough (assuming that
+   all basic blocks between JUMP_BB and DEST_BB are empty).  */
+static bool
+bb_has_removable_jump_to_p (basic_block jump_bb, basic_block dest_bb)
 {
-  basic_block jump_bb = BLOCK_FOR_INSN (jump);
-
-  /* It is not jump, jump with side-effects or jump can lead to several
-     basic blocks.  */
-  if (!onlyjump_p (jump)
-      || !any_uncondjump_p (jump))
+  if (!onlyjump_p (BB_END (jump_bb)))
     return false;
 
   /* Several outgoing edges, abnormal edge or destination of jump is
      not DEST_BB.  */
   if (EDGE_COUNT (jump_bb->succs) != 1
-      || EDGE_SUCC (jump_bb, 0)->flags & EDGE_ABNORMAL
+      || EDGE_SUCC (jump_bb, 0)->flags & (EDGE_ABNORMAL | EDGE_CROSSING)
       || EDGE_SUCC (jump_bb, 0)->dest != dest_bb)
     return false;
 
@@ -6199,7 +6197,7 @@  sel_remove_loop_preheader (void)
                  basic block if it becomes empty.  */
 	      if (next_bb->prev_bb == prev_bb
                   && prev_bb != ENTRY_BLOCK_PTR
-                  && jump_leads_only_to_bb_p (BB_END (prev_bb), next_bb))
+                  && bb_has_removable_jump_to_p (prev_bb, next_bb))
                 {
                   redirect_edge_and_branch (EDGE_SUCC (prev_bb, 0), next_bb);
                   if (BB_END (prev_bb) == bb_note (prev_bb))
diff --git a/gcc/sel-sched-ir.h b/gcc/sel-sched-ir.h
index 9ab0466..1f3dec4 100644
--- a/gcc/sel-sched-ir.h
+++ b/gcc/sel-sched-ir.h
@@ -1590,7 +1590,6 @@  extern bool sel_remove_insn (insn_t, bool, bool);
 extern bool bb_header_p (insn_t);
 extern void sel_init_invalid_data_sets (insn_t);
 extern bool insn_at_boundary_p (insn_t);
-extern bool jump_leads_only_to_bb_p (insn_t, basic_block);
 
 /* Basic block and CFG functions.  */
 
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr45354.c b/gcc/testsuite/gcc.dg/tree-prof/pr45354.c
index e69de29..b30ad77 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/pr45354.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr45354.c
@@ -0,0 +1,43 @@ 
+/* { dg-require-effective-target freorder } */
+/* { dg-options "-O -freorder-blocks-and-partition -fschedule-insns -fselective-scheduling" { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+
+extern void abort (void);
+
+int ifelse_val2;
+
+int __attribute__((noinline))
+test_ifelse2 (int i)
+{
+  int result = 0;
+  if (!i)				/* count(6) */
+    result = 1;				/* count(1) */
+  if (i == 1)				/* count(6) */
+    result = 1024;
+  if (i == 2)				/* count(6) */
+    result = 2;				/* count(3) */
+  if (i == 3)				/* count(6) */
+    return 8;				/* count(2) */
+  if (i == 4)				/* count(4) */
+    return 2048;
+  return result;			/* count(4) */
+}
+
+void __attribute__((noinline))
+call_ifelse ()
+{
+  ifelse_val2 += test_ifelse2 (0);
+  ifelse_val2 += test_ifelse2 (2);
+  ifelse_val2 += test_ifelse2 (2);
+  ifelse_val2 += test_ifelse2 (2);
+  ifelse_val2 += test_ifelse2 (3);
+  ifelse_val2 += test_ifelse2 (3);
+}
+
+int
+main()
+{
+  call_ifelse ();
+  if (ifelse_val2 != 23)
+    abort ();
+  return 0;
+}