Hello,

On 09.08.2012 17:19, Alexander Monakov wrote:


On Thu, 9 Aug 2012, Andrey Belevantsev wrote:

Hello,

The problem in question is uncovered by the recent speculation patch, it is in
the handling of expressions blocked by bookkeeping.  Those are expressions
that become unavailable due to the newly created bookkeeping copies.  In the
original algorithm the supported insns and transformations cannot lead to this
result, but when handling non-separable insns or creating speculative checks
that unpredictably block certain insns the situation can arise.  We just
filter out all such expressions from the final availability set for
correctness.

The PR happens because the expression being filtered out can be transformed
while being moved up, thus we need to look up not only its exact pattern but
also all its previous forms saved in its history of changes.  The patch does
exactly that, I also clarified the comments w.r.t. this situation.

Bootstrapped and tested on ia64 and x86-64, the PR testcase is minimized, too.
OK for trunk?  Also need to backport this to 4.7 with PR 53975, say on the
next week.

This is OK.

The below is the port of this patch to 4.7, took longer than expected but still. Will commit after retesting on x86-64 (testing on ia64 is already fine) and with the fix for PR 53975.

Andrey

2012-10-16  Andrey Belevantsev  <a...@ispras.ru>

        Backport from mainline
        2012-08-09  Andrey Belevantsev  <a...@ispras.ru>

        PR rtl-optimization/53701
        * sel-sched.c (vinsn_vec_has_expr_p): Clarify function comment.
        Process not only expr's vinsns but all old vinsns from expr's
        history of changes.
        (update_and_record_unavailable_insns): Clarify comment. 

testsuite:
2012-10-16  Andrey Belevantsev  <a...@ispras.ru>

        Backport from mainline
        2012-08-09  Andrey Belevantsev  <a...@ispras.ru>

        PR rtl-optimization/53701
        * gcc.dg/pr53701.c: New test.


Index: gcc/testsuite/gcc.dg/pr53701.c
===================================================================
*** gcc/testsuite/gcc.dg/pr53701.c      (revision 0)
--- gcc/testsuite/gcc.dg/pr53701.c      (revision 0)
***************
*** 0 ****
--- 1,59 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O3 -fselective-scheduling2 -fsel-sched-pipelining" } */
+ typedef unsigned short int uint16_t;
+ typedef unsigned long int uintptr_t;
+ typedef struct GFX_VTABLE
+ {
+   int color_depth;
+   unsigned char *line[];
+ }
+ BITMAP;
+ extern int _drawing_mode;
+ extern BITMAP *_drawing_pattern;
+ extern int _drawing_y_anchor;
+ extern unsigned int _drawing_x_mask;
+ extern unsigned int _drawing_y_mask;
+ extern uintptr_t bmp_write_line (BITMAP *, int);
+   void
+ _linear_hline15 (BITMAP * dst, int dx1, int dy, int dx2, int color)
+ {
+   int w;
+   if (_drawing_mode == 0)
+   {
+     int x, curw;
+     unsigned short *sline =
+       (unsigned short *) (_drawing_pattern->
+           line[((dy) -
+             _drawing_y_anchor) & _drawing_y_mask]);
+     unsigned short *s;
+     unsigned short *d =
+       ((unsigned short *) (bmp_write_line (dst, dy)) + (dx1));
+     s = ((unsigned short *) (sline) + (x));
+     if (_drawing_mode == 2)
+     {
+     }
+     else if (_drawing_mode == 3)
+     {
+       do
+       {
+         w -= curw;
+         do
+         {
+           unsigned long c = (*(s));
+           if (!((unsigned long) (c) == 0x7C1F))
+           {
+             (*((uint16_t *) ((uintptr_t) (d))) = ((color)));
+           }
+           ((s)++);
+         }
+         while (--curw > 0);
+         s = sline;
+         curw =
+           (((w) <
+             ((int) _drawing_x_mask +
+              1)) ? (w) : ((int) _drawing_x_mask + 1));
+       }
+       while (curw > 0);
+     }
+   }
+ }
Index: gcc/sel-sched.c
===================================================================
*** gcc/sel-sched.c     (revision 192488)
--- gcc/sel-sched.c     (working copy)
*************** process_use_exprs (av_set_t *av_ptr)
*** 3567,3595 ****
    return NULL;
  }

! /* Lookup EXPR in VINSN_VEC and return TRUE if found.  */
  static bool
  vinsn_vec_has_expr_p (vinsn_vec_t vinsn_vec, expr_t expr)
  {
!   vinsn_t vinsn;
    int n;

!   FOR_EACH_VEC_ELT (vinsn_t, vinsn_vec, n, vinsn)
!     if (VINSN_SEPARABLE_P (vinsn))
!       {
!         if (vinsn_equal_p (vinsn, EXPR_VINSN (expr)))
!           return true;
!       }
!     else
!       {
!         /* For non-separable instructions, the blocking insn can have
!            another pattern due to substitution, and we can't choose
!            different register as in the above case.  Check all registers
!            being written instead.  */
!         if (bitmap_intersect_p (VINSN_REG_SETS (vinsn),
!                                 VINSN_REG_SETS (EXPR_VINSN (expr))))
!           return true;
!       }

    return false;
  }
--- 3567,3607 ----
    return NULL;
  }

! /* Lookup EXPR in VINSN_VEC and return TRUE if found. Also check patterns from
!    EXPR's history of changes.  */
  static bool
  vinsn_vec_has_expr_p (vinsn_vec_t vinsn_vec, expr_t expr)
  {
!   vinsn_t vinsn, expr_vinsn;
    int n;
+   unsigned i;

!   /* Start with checking expr itself and then proceed with all the old forms
!      of expr taken from its history vector.  */
!   for (i = 0, expr_vinsn = EXPR_VINSN (expr);
!        expr_vinsn;
!        expr_vinsn = (i < VEC_length (expr_history_def,
!                                    EXPR_HISTORY_OF_CHANGES (expr))
!                    ? VEC_index (expr_history_def,
!                                 EXPR_HISTORY_OF_CHANGES (expr),
!                                 i++)->old_expr_vinsn
!                    : NULL))
!     FOR_EACH_VEC_ELT (vinsn_t, vinsn_vec, n, vinsn)
!       if (VINSN_SEPARABLE_P (vinsn))
!       {
!         if (vinsn_equal_p (vinsn, expr_vinsn))
!           return true;
!       }
!       else
!       {
!         /* For non-separable instructions, the blocking insn can have
!            another pattern due to substitution, and we can't choose
!            different register as in the above case.  Check all registers
!            being written instead.  */
!         if (bitmap_intersect_p (VINSN_REG_SETS (vinsn),
!                                 VINSN_REG_SETS (expr_vinsn)))
!           return true;
!       }

    return false;
  }
*************** update_and_record_unavailable_insns (bas
*** 5697,5704 ****
                || EXPR_TARGET_AVAILABLE (new_expr)
                 != EXPR_TARGET_AVAILABLE (cur_expr))
            /* Unfortunately, the below code could be also fired up on
!              separable insns.
!              FIXME: add an example of how this could happen.  */
              vinsn_vec_add (&vec_bookkeeping_blocked_vinsns, cur_expr);
          }

--- 5709,5716 ----
                || EXPR_TARGET_AVAILABLE (new_expr)
                 != EXPR_TARGET_AVAILABLE (cur_expr))
            /* Unfortunately, the below code could be also fired up on
!              separable insns, e.g. when moving insns through the new
!              speculation check as in PR 53701.  */
              vinsn_vec_add (&vec_bookkeeping_blocked_vinsns, cur_expr);
          }


Reply via email to