diff mbox

Fix PR 46521 and 46522 (remnants of PR45352)

Message ID 4D10CE93.9090300@ispras.ru
State New
Headers show

Commit Message

Andrey Belevantsev Dec. 21, 2010, 3:58 p.m. UTC 
Hello,

After the patch for PR45352 there were more test cases that revealed two 
more problems.  First, we need to propagate the "rescheduling" bits from 
the current block to the next block more carefully, also through the empty 
blocks.  Second, I thought that if we stall for more cycles that DFA tells 
us to, then the insn would be surely ready for issuing, but this is not the 
case, so I accounted for this in the function that resets scheduling cycles.

Both problems fixed by the below patch, I have checked it on all tests from 
PRs 45352, 46521, and 46522 with all option variants found by Zdenek.  The 
patch was bootstrapped and tested on x86-64 with selective scheduling 
enabled with -O2, and bootstrapped on ia64 again with selective scheduling 
enabled with -O2.  Ok for trunk and active branches if testing on ia64 passes?

Andrey


2010-12-21  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/45352
	PR rtl-optimization/46521
	PR rtl-optimization/46522
	* sel-sched.c (reset_sched_cycles_in_current_ebb): Recheck the DFA state
	on the last iteration of the advancing loop.
	(sel_sched_region_1): Propagate the rescheduling bit to the next block
	also for empty blocks.

	gcc.dg/pr46521.c: New.
	gcc.dg/pr46522.c: New.

Comments

Vladimir Makarov Dec. 21, 2010, 5:31 p.m. UTC | #1 
On 12/21/2010 10:58 AM, Andrey Belevantsev wrote:
> Hello,
>
> After the patch for PR45352 there were more test cases that revealed 
> two more problems.  First, we need to propagate the "rescheduling" 
> bits from the current block to the next block more carefully, also 
> through the empty blocks.  Second, I thought that if we stall for more 
> cycles that DFA tells us to, then the insn would be surely ready for 
> issuing, but this is not the case, so I accounted for this in the 
> function that resets scheduling cycles.
>
> Both problems fixed by the below patch, I have checked it on all tests 
> from PRs 45352, 46521, and 46522 with all option variants found by 
> Zdenek.  The patch was bootstrapped and tested on x86-64 with 
> selective scheduling enabled with -O2, and bootstrapped on ia64 again 
> with selective scheduling enabled with -O2.  Ok for trunk and active 
> branches if testing on ia64 passes?
>

Ok.  Thanks, Andrey.

> 2010-12-21  Andrey Belevantsev <abel@ispras.ru>
>
>     PR rtl-optimization/45352
>     PR rtl-optimization/46521
>     PR rtl-optimization/46522
>     * sel-sched.c (reset_sched_cycles_in_current_ebb): Recheck the DFA 
> state
>     on the last iteration of the advancing loop.
>     (sel_sched_region_1): Propagate the rescheduling bit to the next 
> block
>     also for empty blocks.
>



>     gcc.dg/pr46521.c: New.
>     gcc.dg/pr46522.c: New.
>
I guess you are going to put it into different changelog file 
testsuite/ChangeLog.  I am writing it just to be sure.
diff mbox

Patch

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 3b5603c..edd6cb9 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -7053,7 +7053,17 @@  reset_sched_cycles_in_current_ebb (void)
                   && haifa_cost > 0
                   && estimate_insn_cost (insn, curr_state) == 0)
                 break;
-	    }
+
+              /* When the data dependency stall is longer than the DFA stall,
+                 it could be that after the longer stall the insn will again
+                 become unavailable  to the DFA restrictions.  Looks strange
+                 but happens e.g. on x86-64.  So recheck DFA on the last
+                 iteration.  */
+              if (after_stall
+                  && real_insn
+                  && haifa_cost == 0)
+                haifa_cost = estimate_insn_cost (insn, curr_state);
+            }
 
 	  haifa_clock += i;
           if (sched_verbose >= 2)
@@ -7504,21 +7514,23 @@  sel_sched_region_1 (void)
             {
               basic_block bb = EBB_FIRST_BB (i);
 
-              if (sel_bb_empty_p (bb))
-                {
-                  bitmap_clear_bit (blocks_to_reschedule, bb->index);
-                  continue;
-                }
-
               if (bitmap_bit_p (blocks_to_reschedule, bb->index))
                 {
+                  if (! bb_ends_ebb_p (bb))
+                    bitmap_set_bit (blocks_to_reschedule, bb_next_bb (bb)->index);
+                  if (sel_bb_empty_p (bb))
+                    {
+                      bitmap_clear_bit (blocks_to_reschedule, bb->index);
+                      continue;
+                    }
                   clear_outdated_rtx_info (bb);
                   if (sel_insn_is_speculation_check (BB_END (bb))
                       && JUMP_P (BB_END (bb)))
                     bitmap_set_bit (blocks_to_reschedule,
                                     BRANCH_EDGE (bb)->dest->index);
                 }
-              else if (INSN_SCHED_TIMES (sel_bb_head (bb)) <= 0)
+              else if (! sel_bb_empty_p (bb)
+                       && INSN_SCHED_TIMES (sel_bb_head (bb)) <= 0)
                 bitmap_set_bit (blocks_to_reschedule, bb->index);
             }
 
diff --git a/gcc/testsuite/gcc.dg/pr46521.c b/gcc/testsuite/gcc.dg/pr46521.c
new file mode 100644
index 0000000..0c41c43
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr46521.c
@@ -0,0 +1,20 @@ 
+/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+/* { dg-options "-Os -fselective-scheduling2 -fsel-sched-pipelining -fprofile-generate -fno-early-inlining" } */
+
+static void bmp_iter_next (int *bi)
+{
+  *bi >>= 1;
+}
+
+int bmp_iter_set (int *, int);
+void bitmap_clear (void);
+void bitmap_initialize_stat (void);
+
+void df_md_alloc (int bi, int bb_index, int bb_info)
+{
+  for (; bmp_iter_set (&bi, bb_index); bmp_iter_next (&bi))
+    if (bb_info)
+      bitmap_clear ();
+    else
+      bitmap_initialize_stat ();
+}
diff --git a/gcc/testsuite/gcc.dg/pr46522.c b/gcc/testsuite/gcc.dg/pr46522.c
new file mode 100644
index 0000000..13a5aa9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr46522.c
@@ -0,0 +1,33 @@ 
+/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+/* { dg-options "-O3 -fkeep-inline-functions -fsel-sched-pipelining -fselective-scheduling2 -funroll-loops" } */
+
+struct S
+{
+  unsigned i, j;
+};
+
+static inline void
+bar (struct S *s)
+{
+  if (s->i++ == 1)
+    {
+      s->i = 0;
+      s->j++;
+    }
+}
+
+void
+foo1 (struct S *s)
+{
+  bar (s);
+}
+
+void
+foo2 (struct S s1, struct S s2, int i)
+{
+  while (s1.i != s2.i) {
+    if (i)
+      *(unsigned *) 0 |= (1U << s1.i);
+    bar (&s1);
+  }
+}