Patchwork Fix PR 48144

login
register
mail settings
Submitter Andrey Belevantsev
Date March 24, 2011, 10:50 a.m.
Message ID <4D8B21E8.7050402@ispras.ru>
Download mbox | patch
Permalink /patch/88165/
State New
Headers show

Comments

Andrey Belevantsev - March 24, 2011, 10:50 a.m.
Hello,

As noted in the PR audit trail, this is a case when we fail to find a 
transformed insn due to incomplete transformation history attached to it. 
The earlier fixes of this issue worked only for bookkeeping copies, but now 
we need a more general mechanism.  Fixed by simply picking up additional 
transformation history from the av sets of basic blocks on the way.  The 
problem can only happen on the bookkeeping blocks which have somewhat newer 
av sets available.

Bootstrapping and testing on x86-64 and ia64 is in progress, ok if it succeeds?

Andrey

2011-03-24  Andrey Belevantsev  <abel@ispras.ru>

	gcc/
	PR rtl-optimization/48144
	* sel-sched-ir.c (merge_history_vect): Factor out from ...
	(merge_expr_data): ... here.
	(av_set_intersect): Rename to av_set_code_motion_filter.
	Update all callers.  Call merge_history_vect when an expression
	is found in both sets.
	* sel-sched-ir.h (av_set_code_motion_filter): Add prototype.

	gcc/testsuite
	PR rtl-optimization/48144
	* gcc.dg/pr48144.c: New test.
Vladimir Makarov - March 24, 2011, 8:15 p.m.
On 03/24/2011 06:50 AM, Andrey Belevantsev wrote:
> Hello,
>
> As noted in the PR audit trail, this is a case when we fail to find a 
> transformed insn due to incomplete transformation history attached to 
> it. The earlier fixes of this issue worked only for bookkeeping 
> copies, but now we need a more general mechanism.  Fixed by simply 
> picking up additional transformation history from the av sets of basic 
> blocks on the way.  The problem can only happen on the bookkeeping 
> blocks which have somewhat newer av sets available.
>
> Bootstrapping and testing on x86-64 and ia64 is in progress, ok if it 
> succeeds?
>

Ok, thanks.

> 2011-03-24  Andrey Belevantsev <abel@ispras.ru>
>
>     gcc/
>     PR rtl-optimization/48144
>     * sel-sched-ir.c (merge_history_vect): Factor out from ...
>     (merge_expr_data): ... here.
>     (av_set_intersect): Rename to av_set_code_motion_filter.
>     Update all callers.  Call merge_history_vect when an expression
>     is found in both sets.
>     * sel-sched-ir.h (av_set_code_motion_filter): Add prototype.
>
>     gcc/testsuite
>     PR rtl-optimization/48144
>     * gcc.dg/pr48144.c: New test.
>
>
>

Patch

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index b88dad1..61f3ffb 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -1564,6 +1564,20 @@  free_history_vect (VEC (expr_history_def, heap) **pvect)
   *pvect = NULL;
 }
 
+/* Merge vector FROM to PVECT.  */
+static void
+merge_history_vect (VEC (expr_history_def, heap) **pvect,
+		    VEC (expr_history_def, heap) *from)
+{
+  expr_history_def *phist;
+  int i;
+
+  /* We keep this vector sorted.  */
+  for (i = 0; VEC_iterate (expr_history_def, from, i, phist); i++)
+    insert_in_history_vect (pvect, phist->uid, phist->type,
+                            phist->old_expr_vinsn, phist->new_expr_vinsn,
+                            phist->spec_ds);
+}
 
 /* Compare two vinsns as rhses if possible and as vinsns otherwise.  */
 bool
@@ -1796,9 +1810,6 @@  update_speculative_bits (expr_t to, expr_t from, insn_t split_point)
 void
 merge_expr_data (expr_t to, expr_t from, insn_t split_point)
 {
-  int i;
-  expr_history_def *phist;
-
   /* For now, we just set the spec of resulting expr to be minimum of the specs
      of merged exprs.  */
   if (EXPR_SPEC (to) > EXPR_SPEC (from))
@@ -1822,20 +1833,12 @@  merge_expr_data (expr_t to, expr_t from, insn_t split_point)
   EXPR_ORIG_SCHED_CYCLE (to) = MIN (EXPR_ORIG_SCHED_CYCLE (to),
                                     EXPR_ORIG_SCHED_CYCLE (from));
 
-  /* We keep this vector sorted.  */
-  for (i = 0;
-       VEC_iterate (expr_history_def, EXPR_HISTORY_OF_CHANGES (from),
-                    i, phist);
-       i++)
-    insert_in_history_vect (&EXPR_HISTORY_OF_CHANGES (to),
-                            phist->uid, phist->type,
-                            phist->old_expr_vinsn, phist->new_expr_vinsn,
-                            phist->spec_ds);
-
   EXPR_WAS_SUBSTITUTED (to) |= EXPR_WAS_SUBSTITUTED (from);
   EXPR_WAS_RENAMED (to) |= EXPR_WAS_RENAMED (from);
   EXPR_CANT_MOVE (to) |= EXPR_CANT_MOVE (from);
 
+  merge_history_vect (&EXPR_HISTORY_OF_CHANGES (to),
+		      EXPR_HISTORY_OF_CHANGES (from));
   update_target_availability (to, from, split_point);
   update_speculative_bits (to, from, split_point);
 }
@@ -2328,16 +2331,24 @@  av_set_split_usefulness (av_set_t av, int prob, int all_prob)
 }
 
 /* Leave in AVP only those expressions, which are present in AV,
-   and return it.  */
+   and return it, merging history expressions.  */
 void
-av_set_intersect (av_set_t *avp, av_set_t av)
+av_set_code_motion_filter (av_set_t *avp, av_set_t av)
 {
   av_set_iterator i;
-  expr_t expr;
+  expr_t expr, expr2;
 
   FOR_EACH_EXPR_1 (expr, i, avp)
-    if (av_set_lookup (av, EXPR_VINSN (expr)) == NULL)
+    if ((expr2 = av_set_lookup (av, EXPR_VINSN (expr))) == NULL)
       av_set_iter_remove (&i);
+    else
+      /* When updating av sets in bookkeeping blocks, we can add more insns
+	 there which will be transformed but the upper av sets will not
+	 reflect those transformations.  We then fail to undo those
+	 when searching for such insns.  So merge the history saved
+	 in the av set of the block we are processing.  */
+      merge_history_vect (&EXPR_HISTORY_OF_CHANGES (expr),
+			  EXPR_HISTORY_OF_CHANGES (expr2));
 }
 
 
diff --git a/gcc/sel-sched-ir.h b/gcc/sel-sched-ir.h
index 1f3dec4..5516da9 100644
--- a/gcc/sel-sched-ir.h
+++ b/gcc/sel-sched-ir.h
@@ -1565,7 +1565,7 @@  extern void av_set_leave_one_nonspec (av_set_t *);
 extern expr_t av_set_element (av_set_t, int);
 extern void av_set_substract_cond_branches (av_set_t *);
 extern void av_set_split_usefulness (av_set_t, int, int);
-extern void av_set_intersect (av_set_t *, av_set_t);
+extern void av_set_code_motion_filter (av_set_t *, av_set_t);
 
 extern void sel_save_haifa_priorities (void);
 
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index e26ddac..9179249 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -6481,7 +6481,7 @@  code_motion_path_driver (insn_t insn, av_set_t orig_ops, ilist_t path,
 
   /* Filter the orig_ops set.  */
   if (AV_SET_VALID_P (insn))
-    av_set_intersect (&orig_ops, AV_SET (insn));
+    av_set_code_motion_filter (&orig_ops, AV_SET (insn));
 
   /* If no more original ops, return immediately.  */
   if (!orig_ops)
diff --git a/gcc/testsuite/gcc.dg/pr48144.c b/gcc/testsuite/gcc.dg/pr48144.c
new file mode 100644
index 0000000..030202d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr48144.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O -frerun-cse-after-loop -fschedule-insns2 -fselective-scheduling2 -fno-tree-ch -funroll-loops --param=max-sched-extend-regions-iters=2 --param=max-sched-region-blocks=15" } */
+extern void *memcpy(void *dest, const void *src, __SIZE_TYPE__ n);
+
+void bar (void *, void *, void *);
+
+void foo
+  (void *p, char *data, unsigned data_len)
+{
+  int buffer[8];
+  int buf2[8];
+  unsigned i;
+  for (i = 0; i + 8 <= data_len; i += 8)
+    bar (p, buffer, data + i);
+  memcpy (buf2, data + i, data_len);
+}