[PATCH] Fix PR45352

[PATCH] Fix PR45352

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

Hello,

As explained in the audit trail, this problem report has a number of tests 
for which we have failed to put correct scheduling cycles on the pipelined 
code in line with the Haifa scheduler.  There were two reasons for that, 
first is the number of assumptions that we couldn't issue more than 
issue_rate insns, and second is that the EBB boundaries for the resetting 
scheduling cycle process did not always correspond with the boundaries of 
the rescheduled region.  In the latter case, there was also a small bug due 
to the recent change of bitmap_bit_p to bitmap_clear_bit and me not 
noticing that the bits were still needed after the clearing.  The hunks for 
the latter case can also be committed separately.

I have also removed the assert that we do not issue more than issue_rate 
insns on the same fence.  This assert can be put back after Jie's fix of 
max_issue will go in; I can wait until the consensus of his patch and 
remove the according hunk or leave it in.

Bootstrapped and tested on ia64 and x86-64, all new tests pass.
OK for trunk and 4.5?

Thanks, Andrey


2010-10-22  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/45352
	* sel-sched.c (invoke_reorder_hooks): New parameter pran_hook.
	Use it to remember whether any of the hooks was run.
	(choose_best_insn): New parameter ran_hook.  Set can_issue to 1
	when no hook was run.  Update comment.
	(find_best_expr): Update comment.  Try issuing when no hooks
	were run.  Indicate resource stall with writing -1 to pneed_stall.
	Do not set pneed_stall when the variable_issue hook is not implemented.
	(advance_state_on_fence): Do not assert that we cannot issue more than
	issue_rate insns.
	(stall_for_cycles): Update comment.  Handle the negative n parameter.
	Set AFTER_STALL_P only for dependency stalls.
	(fill_insns): Remove dead variable stall_iterations.  Account for
	need_stall being negative.
	(init_seqno_1): Force EBB start for resetting sched cycles on any
	successor blocks of the rescheduled region.
	(sel_sched_region_1): Use bitmap_bit_p instead of bitmap_clear_bit.
	(reset_sched_cycles_in_current_ebb): Add debug printing. 		

	gcc.dg/pr45352.c, gcc.dg/pr45352-1.c, gcc.dg/pr45352-2.c: New tests.
	gcc.target/i386/pr45352.c, gcc.target/i386/pr45352-1.c,
	gcc.target/i386/pr45352-2.c: New tests.


[-- Attachment #2: pr45352.diff --]
[-- Type: text/x-patch, Size: 20178 bytes --]

Index: gcc/testsuite/gcc.target/i386/pr45352-2.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352-2.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352-2.c	(revision 0)
***************
*** 0 ****
--- 1,108 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O1 -mtune=amdfam10 -fexpensive-optimizations -fgcse -foptimize-register-move -freorder-blocks -fschedule-insns2 -funswitch-loops -fgcse-las -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ 
+ typedef char uint8_t;
+ typedef uint32_t;
+ typedef vo_frame_t;
+ struct vo_frame_s
+ {
+     uint8_t base[3];
+   int pitches[3];};
+ typedef struct
+ {
+ void
+     (*proc_macro_block)
+     (void);
+ }
+ xine_xvmc_t;
+ typedef struct
+ {
+   uint8_t ref[2][3];
+ int pmv;
+ }
+ motion_t;
+ typedef struct
+ {
+   uint32_t bitstream_buf;
+   int bitstream_bits;
+     uint8_t * bitstream_ptr;
+     uint8_t dest[3];
+   int pitches[3];
+   int offset;
+     motion_t b_motion;
+     motion_t f_motion;
+   int v_offset;
+   int coded_picture_width;
+   int picture_structure;
+ struct vo_frame_s *current_frame;}
+ picture_t;
+ typedef struct
+ {
+ int xvmc_last_slice_code;}
+ mpeg2dec_accel_t;
+ static bitstream_init (picture_t * picture, void *start)
+ {
+   picture->bitstream_ptr = start;
+ }
+ static slice_xvmc_init (picture_t * picture, int code)
+ {
+   int offset;
+   struct vo_frame_s *forward_reference_frame;
+   offset = picture->picture_structure == 2;
+   picture->pitches[0] = picture->current_frame->pitches[0];
+   picture->pitches[1] = picture->current_frame->pitches[1];
+   if (picture)
+     picture->f_motion.ref
+       [0]
+       [0]
+       = forward_reference_frame->base + (offset ? picture->pitches[0] : 0);
+   picture->f_motion.ref[0][1] = (offset);
+   if (picture->picture_structure)
+       picture->pitches[0] <<= picture->pitches[1] <<= 1;
+   offset = 0;
+   while (1)
+     {
+       if (picture->bitstream_buf >= 0x08000000)
+ 	  break;
+       switch (picture->bitstream_buf >> 12)
+ 	{
+ 	case 8:
+ 	  offset += 33;
+ 		picture->bitstream_buf
+ 		  |=
+ 		  picture->bitstream_ptr[1] << picture->bitstream_bits;
+ 	}
+     }
+   picture->offset = (offset);
+   while (picture->offset - picture->coded_picture_width >= 0)
+     {
+       picture->offset -= picture->coded_picture_width;
+       if (picture->current_frame)
+ 	{
+ 	  picture->dest[0] += picture->pitches[0];
+ 	  picture->dest[1] += picture->pitches[1];
+ 	}
+       picture->v_offset += 16;
+     }
+ }
+ 
+ void
+ mpeg2_xvmc_slice
+   (mpeg2dec_accel_t * accel, picture_t * picture, int code, uint8_t buffer,int mba_inc)
+ {
+   xine_xvmc_t * xvmc = bitstream_init (picture, buffer);
+   slice_xvmc_init (picture, code);
+     while (1)
+       {
+ 	if (picture)
+ 	    break;
+ 	switch (picture->bitstream_buf)
+ 	  {
+ 	  case 8:
+ 	    mba_inc += accel->xvmc_last_slice_code = code;
+ 		  xvmc->proc_macro_block   ();
+ 	    while (mba_inc)
+ 	      ;
+ 	  }
+       }
+ }
Index: gcc/testsuite/gcc.target/i386/pr45352.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352.c	(revision 0)
***************
*** 0 ****
--- 1,25 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3 -march=amdfam10 -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ 
+ struct S
+ {
+   struct
+   {
+     int i;
+   } **p;
+   int x;
+   int y;
+ };
+ 
+ extern int baz (void);
+ extern int bar (void *, int, int);
+ 
+ void
+ foo (struct S *s)
+ {
+   int i;
+   for (i = 0; i < s->x; i++)
+     bar (s->p[i], baz (), s->y);
+   for (i = 0; i < s->x; i++)
+     s->p[i]->i++;
+ }
Index: gcc/testsuite/gcc.target/i386/pr45352-1.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352-1.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352-1.c	(revision 0)
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do compile } */
+ /* { dg-options "-mtune=amdfam10 -O3 -fpeel-loops -fselective-scheduling2 -fsel-sched-pipelining -fPIC" } */
+ 
+ static int FIR_Tab_16[16][16];
+ 
+ void
+ V_Pass_Avrg_16_C_ref (int *Dst, int *Src, int W, int BpS, int Rnd)
+ {
+   while (W-- > 0)
+     {
+       int i, k;
+       int Sums[16] = { };
+       for (i = 0; i < 16; ++i)
+ 	for (k = 0; k < 16; ++k)
+ 	  Sums[k] += FIR_Tab_16[i][k] * Src[i];
+       for (i = 0; i < 16; ++i)
+ 	Dst[i] = Sums[i] + Src[i];
+     }
+ }
Index: gcc/testsuite/gcc.dg/pr45352-1.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352-1.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352-1.c	(revision 0)
***************
*** 0 ****
--- 1,13 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O3 -fschedule-insns -fschedule-insns2 -fselective-scheduling2 -fsel-sched-pipelining -funroll-loops -fprefetch-loop-arrays" } */
+ 
+ void main1 (float *pa, float *pc)
+ {
+   int i;
+   float b[256];
+   float c[256];
+   for (i = 0; i < 256; i++)
+     b[i] = c[i] = pc[i];
+   for (i = 0; i < 256; i++)
+     pa[i] = b[i] * c[i];
+ }
Index: gcc/testsuite/gcc.dg/pr45352-2.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352-2.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352-2.c	(revision 0)
***************
*** 0 ****
--- 1,17 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O1 -freorder-blocks -fschedule-insns2 -funswitch-loops -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ void
+ foo1 (int *s)
+ {
+   s[0] = s[1];
+   while (s[6] - s[8])
+     {
+       s[6] -= s[8];
+       if (s[8] || s[0])
+ 	{
+ 	  s[3] += s[0];
+ 	  s[4] += s[1];
+ 	}
+       s[7]++;
+     }
+ }
Index: gcc/testsuite/gcc.dg/pr45352.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352.c	(revision 0)
***************
*** 0 ****
--- 1,24 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-Os -fselective-scheduling2 -fsel-sched-pipelining -fprofile-generate" } */
+ 
+ static inline void
+ bmp_iter_next (int *bi, int *bit_no)
+ {
+   *bi >>= 1;
+   *bit_no += 1;
+ }
+ 
+ int bmp_iter_set (int *bi, int *bit_no);
+ void bitmap_initialize_stat (int, ...);
+ void bitmap_clear (void);
+ 
+ void
+ df_md_alloc (int bi, int bb_index, void *bb_info)
+ {
+   for (; bmp_iter_set (&bi, &bb_index); bmp_iter_next (&bi, &bb_index))
+ 
+     if (bb_info)
+       bitmap_clear ();
+     else
+       bitmap_initialize_stat (0);
+ }
Index: gcc/sel-sched.c
===================================================================
*** gcc/sel-sched.c	(revision 165769)
--- gcc/sel-sched.c	(working copy)
*************** sel_dfa_new_cycle (insn_t insn, fence_t 
*** 4049,4060 ****
  }
  
  /* Invoke reorder* target hooks on the ready list.  Return the number of insns
!    we can issue.  FENCE is the current fence.  */
  static int
! invoke_reorder_hooks (fence_t fence)
  {
    int issue_more;
!   bool ran_hook = false;
  
    /* Call the reorder hook at the beginning of the cycle, and call
       the reorder2 hook in the middle of the cycle.  */
--- 4049,4062 ----
  }
  
  /* Invoke reorder* target hooks on the ready list.  Return the number of insns
!    we can issue.  FENCE is the current fence.  Write true in *PRAN_HOOK whether
!    we have actually run any hook.  */
  static int
! invoke_reorder_hooks (fence_t fence, bool *pran_hook)
  {
    int issue_more;
! 
!   *pran_hook = false;
  
    /* Call the reorder hook at the beginning of the cycle, and call
       the reorder2 hook in the middle of the cycle.  */
*************** invoke_reorder_hooks (fence_t fence)
*** 4077,4083 ****
            if (pipelining_p)
              ++ready.n_ready;
  
!           ran_hook = true;
          }
        else
          /* Initialize can_issue_more for variable_issue.  */
--- 4079,4085 ----
            if (pipelining_p)
              ++ready.n_ready;
  
!           *pran_hook = true;
          }
        else
          /* Initialize can_issue_more for variable_issue.  */
*************** invoke_reorder_hooks (fence_t fence)
*** 4106,4119 ****
              ++ready.n_ready;
          }
  
!       ran_hook = true;
      }
    else
      issue_more = FENCE_ISSUE_MORE (fence);
  
    /* Ensure that ready list and vec_av_set are in line with each other,
       i.e. vec_av_set[i] == ready_element (&ready, i).  */
!   if (issue_more && ran_hook)
      {
        int i, j, n;
        rtx *arr = ready.vec;
--- 4108,4121 ----
              ++ready.n_ready;
          }
  
!       *pran_hook = true;
      }
    else
      issue_more = FENCE_ISSUE_MORE (fence);
  
    /* Ensure that ready list and vec_av_set are in line with each other,
       i.e. vec_av_set[i] == ready_element (&ready, i).  */
!   if (issue_more && *pran_hook)
      {
        int i, j, n;
        rtx *arr = ready.vec;
*************** get_expr_cost (expr_t expr, fence_t fenc
*** 4311,4319 ****
  }
  
  /* Find the best insn for scheduling, either via max_issue or just take
!    the most prioritized available.  */
  static int
! choose_best_insn (fence_t fence, int privileged_n, int *index)
  {
    int can_issue = 0;
  
--- 4313,4325 ----
  }
  
  /* Find the best insn for scheduling, either via max_issue or just take
!    the most prioritized available.  FENCE is the current fence,
!    PRIVILEGED_N is the number of privileged insns for max_issue call,
!    RAN_HOOK indicates that any preparational hooks were run.  
!    Write in *INDEX the number of selected instruction or -1 if neither
!    instruction of the ready list can be selected.  */
  static int
! choose_best_insn (fence_t fence, int privileged_n, bool ran_hook, int *index)
  {
    int can_issue = 0;
  
*************** choose_best_insn (fence_t fence, int pri
*** 4338,4343 ****
--- 4344,4351 ----
  	  if (get_expr_cost (expr, fence) < 1)
  	    {
  	      can_issue = can_issue_more;
+ 	      if (!ran_hook && !can_issue)
+ 		can_issue = 1;
  	      *index = i;
  
  	      if (sched_verbose >= 2)
*************** choose_best_insn (fence_t fence, int pri
*** 4360,4371 ****
  /* Choose the best expr from *AV_VLIW_PTR and a suitable register for it.
     BNDS and FENCE are current boundaries and scheduling fence respectively.
     Return the expr found and NULL if nothing can be issued atm.
!    Write to PNEED_STALL the number of cycles to stall if no expr was found.  */
  static expr_t
  find_best_expr (av_set_t *av_vliw_ptr, blist_t bnds, fence_t fence,
                  int *pneed_stall)
  {
    expr_t best;
  
    /* Choose the best insn for scheduling via:
       1) sorting the ready list based on priority;
--- 4368,4382 ----
  /* Choose the best expr from *AV_VLIW_PTR and a suitable register for it.
     BNDS and FENCE are current boundaries and scheduling fence respectively.
     Return the expr found and NULL if nothing can be issued atm.
!    Write to PNEED_STALL the number of cycles to stall if no expr was found.
!    The positive number of cycles means a data dependency stall, while
!    the negative one means a functional stall (DFA stall).  */
  static expr_t
  find_best_expr (av_set_t *av_vliw_ptr, blist_t bnds, fence_t fence,
                  int *pneed_stall)
  {
    expr_t best;
+   bool ran_hook;
  
    /* Choose the best insn for scheduling via:
       1) sorting the ready list based on priority;
*************** find_best_expr (av_set_t *av_vliw_ptr, b
*** 4376,4383 ****
      {
        int privileged_n, index;
  
!       can_issue_more = invoke_reorder_hooks (fence);
!       if (can_issue_more > 0)
          {
            /* Try choosing the best insn until we find one that is could be
               scheduled due to liveness restrictions on its destination register.
--- 4387,4394 ----
      {
        int privileged_n, index;
  
!       can_issue_more = invoke_reorder_hooks (fence, &ran_hook);
!       if (can_issue_more > 0 || !ran_hook)
          {
            /* Try choosing the best insn until we find one that is could be
               scheduled due to liveness restrictions on its destination register.
*************** find_best_expr (av_set_t *av_vliw_ptr, b
*** 4385,4391 ****
               in the order of their priority.  */
            invoke_dfa_lookahead_guard ();
            privileged_n = calculate_privileged_insns ();
!           can_issue_more = choose_best_insn (fence, privileged_n, &index);
            if (can_issue_more)
              best = find_expr_for_ready (index, true);
          }
--- 4396,4402 ----
               in the order of their priority.  */
            invoke_dfa_lookahead_guard ();
            privileged_n = calculate_privileged_insns ();
!           can_issue_more = choose_best_insn (fence, privileged_n, ran_hook, &index);
            if (can_issue_more)
              best = find_expr_for_ready (index, true);
          }
*************** find_best_expr (av_set_t *av_vliw_ptr, b
*** 4394,4400 ****
        if (can_issue_more == 0)
          {
            best = NULL;
!           *pneed_stall = 1;
          }
      }
  
--- 4405,4411 ----
        if (can_issue_more == 0)
          {
            best = NULL;
!           *pneed_stall = -1;
          }
      }
  
*************** find_best_expr (av_set_t *av_vliw_ptr, b
*** 4402,4409 ****
      {
        can_issue_more = invoke_aftermath_hooks (fence, EXPR_INSN_RTX (best),
                                                 can_issue_more);
!       if (can_issue_more == 0)
!         *pneed_stall = 1;
      }
  
    if (sched_verbose >= 2)
--- 4413,4421 ----
      {
        can_issue_more = invoke_aftermath_hooks (fence, EXPR_INSN_RTX (best),
                                                 can_issue_more);
!       if (targetm.sched.variable_issue
! 	  && can_issue_more == 0)
!         *pneed_stall = -1;
      }
  
    if (sched_verbose >= 2)
*************** advance_state_on_fence (fence_t fence, i
*** 5282,5294 ****
        gcc_assert (res < 0);
  
        if (memcmp (temp_state, FENCE_STATE (fence), dfa_state_size))
!         {
!           FENCE_ISSUED_INSNS (fence)++;
! 
!           /* We should never issue more than issue_rate insns.  */
!           if (FENCE_ISSUED_INSNS (fence) > issue_rate)
!             gcc_unreachable ();
!         }
      }
    else
      {
--- 5294,5300 ----
        gcc_assert (res < 0);
  
        if (memcmp (temp_state, FENCE_STATE (fence), dfa_state_size))
! 	FENCE_ISSUED_INSNS (fence)++;
      }
    else
      {
*************** schedule_expr_on_boundary (bnd_t bnd, ex
*** 5471,5483 ****
    return insn;
  }
  
! /* Stall for N cycles on FENCE.  */
  static void
  stall_for_cycles (fence_t fence, int n)
  {
    int could_more;
  
!   could_more = n > 1 || FENCE_ISSUED_INSNS (fence) < issue_rate;
    while (n--)
      advance_one_cycle (fence);
    if (could_more)
--- 5477,5495 ----
    return insn;
  }
  
! /* Stall for N cycles on FENCE.  N > 0 means we want a stall because
!    of an unsatisfied data dependence, N < 0 means the DFA stall.
!    The difference is that we need to set the AFTER_STALL_P bit only
!    for a data dependence stall.  */
  static void
  stall_for_cycles (fence_t fence, int n)
  {
    int could_more;
  
!   if (n > 0)
!     could_more = 1;
!   else
!     n = -n;
    while (n--)
      advance_one_cycle (fence);
    if (could_more)
*************** fill_insns (fence_t fence, int seqno, il
*** 5510,5516 ****
        blist_t *bnds_tailp1, *bndsp;
        expr_t expr_vliw;
        int need_stall;
!       int was_stall = 0, scheduled_insns = 0, stall_iterations = 0;
        int max_insns = pipelining_p ? issue_rate : 2 * issue_rate;
        int max_stall = pipelining_p ? 1 : 3;
        bool last_insn_was_debug = false;
--- 5522,5528 ----
        blist_t *bnds_tailp1, *bndsp;
        expr_t expr_vliw;
        int need_stall;
!       int was_stall = 0, scheduled_insns = 0;
        int max_insns = pipelining_p ? issue_rate : 2 * issue_rate;
        int max_stall = pipelining_p ? 1 : 3;
        bool last_insn_was_debug = false;
*************** fill_insns (fence_t fence, int seqno, il
*** 5529,5545 ****
        do
          {
            expr_vliw = find_best_expr (&av_vliw, bnds, fence, &need_stall);
!           if (!expr_vliw && need_stall)
              {
                /* All expressions required a stall.  Do not recompute av sets
                   as we'll get the same answer (modulo the insns between
                   the fence and its boundary, which will not be available for
!                  pipelining).  */
!               gcc_assert (! expr_vliw && stall_iterations < 2);
!               was_stall++;
! 	      /* If we are going to stall for too long, break to recompute av
  		 sets and bring more insns for pipelining.  */
! 	      if (need_stall <= 3)
  		stall_for_cycles (fence, need_stall);
  	      else
  		{
--- 5541,5556 ----
        do
          {
            expr_vliw = find_best_expr (&av_vliw, bnds, fence, &need_stall);
!           if (! expr_vliw && need_stall != 0)
              {
                /* All expressions required a stall.  Do not recompute av sets
                   as we'll get the same answer (modulo the insns between
                   the fence and its boundary, which will not be available for
!                  pipelining).
! 		 If we are going to stall for too long, break to recompute av
  		 sets and bring more insns for pipelining.  */
!               was_stall++;
! 	      if (need_stall < 0 || need_stall <= 3)
  		stall_for_cycles (fence, need_stall);
  	      else
  		{
*************** fill_insns (fence_t fence, int seqno, il
*** 5548,5554 ****
  		}
              }
          }
!       while (! expr_vliw && need_stall);
  
        /* Now either we've selected expr_vliw or we have nothing to schedule.  */
        if (!expr_vliw)
--- 5559,5565 ----
  		}
              }
          }
!       while (! expr_vliw && need_stall != 0);
  
        /* Now either we've selected expr_vliw or we have nothing to schedule.  */
        if (!expr_vliw)
*************** init_seqno_1 (basic_block bb, sbitmap vi
*** 6711,6716 ****
--- 6722,6729 ----
  
  	  init_seqno_1 (succ, visited_bbs, blocks_to_reschedule);
  	}
+       else if (blocks_to_reschedule)
+         bitmap_set_bit (forced_ebb_heads, succ->index);
      }
  
    for (insn = BB_END (bb); insn != note; insn = PREV_INSN (insn))
*************** reset_sched_cycles_in_current_ebb (void)
*** 7046,7051 ****
--- 7059,7066 ----
  	    }
  
  	  haifa_clock += i;
+           if (sched_verbose >= 2)
+             sel_print ("haifa clock: %d\n", haifa_clock);
  	}
        else
  	gcc_assert (haifa_cost == 0);
*************** reset_sched_cycles_in_current_ebb (void)
*** 7064,7069 ****
--- 7079,7085 ----
                {
                  sel_print ("advance_state (dfa_new_cycle)\n");
                  debug_state (curr_state);
+ 		sel_print ("haifa clock: %d\n", haifa_clock + 1);
                }
            }
  
*************** reset_sched_cycles_in_current_ebb (void)
*** 7072,7079 ****
  	  cost = state_transition (curr_state, insn);
  
            if (sched_verbose >= 2)
!             debug_state (curr_state);
! 
  	  gcc_assert (cost < 0);
  	}
  
--- 7088,7098 ----
  	  cost = state_transition (curr_state, insn);
  
            if (sched_verbose >= 2)
! 	    {
! 	      sel_print ("scheduled insn %d, clock %d\n", INSN_UID (insn),
! 			 haifa_clock + 1);
!               debug_state (curr_state);
! 	    }
  	  gcc_assert (cost < 0);
  	}
  
*************** sel_sched_region_1 (void)
*** 7517,7523 ****
                    continue;
                  }
  
!               if (bitmap_clear_bit (blocks_to_reschedule, bb->index))
                  {
                    flist_tail_init (new_fences);
  
--- 7536,7542 ----
                    continue;
                  }
  
!               if (bitmap_bit_p (blocks_to_reschedule, bb->index))
                  {
                    flist_tail_init (new_fences);
  

Re: [PATCH] Fix PR45352

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]

Hello,

Now that we have agreed about the scheduler not issuing more than 
issue_rate insns, the PR can actually be solved easier.  I only need the 
loop resetting sched cycles be in line with the rest of the scheduler and 
advance state when we have issued issue_rate insns.  The only bits from the 
old patch that are needed are not setting need_stall to 1 when no 
variable_issue hook exists and properly synchronizing rescheduling regions 
and resetting cycles regions.  These do not make us trying to issue more 
than issue_rate insns.

The patch fixes all test cases and in the process of bootstrap/test on 
ia64, ok if it passes?

Andrey

2010-10-22  Andrey Belevantsev  <abel@ispras.ru>

     PR rtl-optimization/45352
     * sel-sched.c (find_best_expr): Do not set pneed_stall when
     the variable_issue hook is not implemented.
     (fill_insns): Remove dead variable stall_iterations.
     (init_seqno_1): Force EBB start for resetting sched cycles on any
     successor blocks of the rescheduled region.
     (sel_sched_region_1): Use bitmap_bit_p instead of bitmap_clear_bit.
     (reset_sched_cycles_in_current_ebb): Add debug printing.
     New variable issued_insns.  Advance state when we have issued
     issue_rate insns.
	
     gcc.dg/pr45352.c, gcc.dg/pr45352-1.c, gcc.dg/pr45352-2.c: New tests.
     gcc.target/i386/pr45352.c, gcc.target/i386/pr45352-1.c,
     gcc.target/i386/pr45352-2.c: New tests.

[-- Attachment #2: pr45352-new.diff --]
[-- Type: text/x-patch, Size: 13045 bytes --]

Index: gcc/testsuite/gcc.target/i386/pr45352-2.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352-2.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352-2.c	(revision 0)
***************
*** 0 ****
--- 1,108 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O1 -mtune=amdfam10 -fexpensive-optimizations -fgcse -foptimize-register-move -freorder-blocks -fschedule-insns2 -funswitch-loops -fgcse-las -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ 
+ typedef char uint8_t;
+ typedef uint32_t;
+ typedef vo_frame_t;
+ struct vo_frame_s
+ {
+     uint8_t base[3];
+   int pitches[3];};
+ typedef struct
+ {
+ void
+     (*proc_macro_block)
+     (void);
+ }
+ xine_xvmc_t;
+ typedef struct
+ {
+   uint8_t ref[2][3];
+ int pmv;
+ }
+ motion_t;
+ typedef struct
+ {
+   uint32_t bitstream_buf;
+   int bitstream_bits;
+     uint8_t * bitstream_ptr;
+     uint8_t dest[3];
+   int pitches[3];
+   int offset;
+     motion_t b_motion;
+     motion_t f_motion;
+   int v_offset;
+   int coded_picture_width;
+   int picture_structure;
+ struct vo_frame_s *current_frame;}
+ picture_t;
+ typedef struct
+ {
+ int xvmc_last_slice_code;}
+ mpeg2dec_accel_t;
+ static bitstream_init (picture_t * picture, void *start)
+ {
+   picture->bitstream_ptr = start;
+ }
+ static slice_xvmc_init (picture_t * picture, int code)
+ {
+   int offset;
+   struct vo_frame_s *forward_reference_frame;
+   offset = picture->picture_structure == 2;
+   picture->pitches[0] = picture->current_frame->pitches[0];
+   picture->pitches[1] = picture->current_frame->pitches[1];
+   if (picture)
+     picture->f_motion.ref
+       [0]
+       [0]
+       = forward_reference_frame->base + (offset ? picture->pitches[0] : 0);
+   picture->f_motion.ref[0][1] = (offset);
+   if (picture->picture_structure)
+       picture->pitches[0] <<= picture->pitches[1] <<= 1;
+   offset = 0;
+   while (1)
+     {
+       if (picture->bitstream_buf >= 0x08000000)
+ 	  break;
+       switch (picture->bitstream_buf >> 12)
+ 	{
+ 	case 8:
+ 	  offset += 33;
+ 		picture->bitstream_buf
+ 		  |=
+ 		  picture->bitstream_ptr[1] << picture->bitstream_bits;
+ 	}
+     }
+   picture->offset = (offset);
+   while (picture->offset - picture->coded_picture_width >= 0)
+     {
+       picture->offset -= picture->coded_picture_width;
+       if (picture->current_frame)
+ 	{
+ 	  picture->dest[0] += picture->pitches[0];
+ 	  picture->dest[1] += picture->pitches[1];
+ 	}
+       picture->v_offset += 16;
+     }
+ }
+ 
+ void
+ mpeg2_xvmc_slice
+   (mpeg2dec_accel_t * accel, picture_t * picture, int code, uint8_t buffer,int mba_inc)
+ {
+   xine_xvmc_t * xvmc = bitstream_init (picture, buffer);
+   slice_xvmc_init (picture, code);
+     while (1)
+       {
+ 	if (picture)
+ 	    break;
+ 	switch (picture->bitstream_buf)
+ 	  {
+ 	  case 8:
+ 	    mba_inc += accel->xvmc_last_slice_code = code;
+ 		  xvmc->proc_macro_block   ();
+ 	    while (mba_inc)
+ 	      ;
+ 	  }
+       }
+ }
Index: gcc/testsuite/gcc.target/i386/pr45352.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352.c	(revision 0)
***************
*** 0 ****
--- 1,25 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3 -march=amdfam10 -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ 
+ struct S
+ {
+   struct
+   {
+     int i;
+   } **p;
+   int x;
+   int y;
+ };
+ 
+ extern int baz (void);
+ extern int bar (void *, int, int);
+ 
+ void
+ foo (struct S *s)
+ {
+   int i;
+   for (i = 0; i < s->x; i++)
+     bar (s->p[i], baz (), s->y);
+   for (i = 0; i < s->x; i++)
+     s->p[i]->i++;
+ }
Index: gcc/testsuite/gcc.target/i386/pr45352-1.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352-1.c	(revision 0)
--- gcc/testsuite/gcc.target/i386/pr45352-1.c	(revision 0)
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do compile } */
+ /* { dg-options "-mtune=amdfam10 -O3 -fpeel-loops -fselective-scheduling2 -fsel-sched-pipelining -fPIC" } */
+ 
+ static int FIR_Tab_16[16][16];
+ 
+ void
+ V_Pass_Avrg_16_C_ref (int *Dst, int *Src, int W, int BpS, int Rnd)
+ {
+   while (W-- > 0)
+     {
+       int i, k;
+       int Sums[16] = { };
+       for (i = 0; i < 16; ++i)
+ 	for (k = 0; k < 16; ++k)
+ 	  Sums[k] += FIR_Tab_16[i][k] * Src[i];
+       for (i = 0; i < 16; ++i)
+ 	Dst[i] = Sums[i] + Src[i];
+     }
+ }
Index: gcc/testsuite/gcc.dg/pr45352-1.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352-1.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352-1.c	(revision 0)
***************
*** 0 ****
--- 1,13 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O3 -fschedule-insns -fschedule-insns2 -fselective-scheduling2 -fsel-sched-pipelining -funroll-loops -fprefetch-loop-arrays" } */
+ 
+ void main1 (float *pa, float *pc)
+ {
+   int i;
+   float b[256];
+   float c[256];
+   for (i = 0; i < 256; i++)
+     b[i] = c[i] = pc[i];
+   for (i = 0; i < 256; i++)
+     pa[i] = b[i] * c[i];
+ }
Index: gcc/testsuite/gcc.dg/pr45352-2.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352-2.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352-2.c	(revision 0)
***************
*** 0 ****
--- 1,17 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O1 -freorder-blocks -fschedule-insns2 -funswitch-loops -fselective-scheduling2 -fsel-sched-pipelining -funroll-all-loops" } */
+ void
+ foo1 (int *s)
+ {
+   s[0] = s[1];
+   while (s[6] - s[8])
+     {
+       s[6] -= s[8];
+       if (s[8] || s[0])
+ 	{
+ 	  s[3] += s[0];
+ 	  s[4] += s[1];
+ 	}
+       s[7]++;
+     }
+ }
Index: gcc/testsuite/gcc.dg/pr45352.c
===================================================================
*** gcc/testsuite/gcc.dg/pr45352.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr45352.c	(revision 0)
***************
*** 0 ****
--- 1,24 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-Os -fselective-scheduling2 -fsel-sched-pipelining -fprofile-generate" } */
+ 
+ static inline void
+ bmp_iter_next (int *bi, int *bit_no)
+ {
+   *bi >>= 1;
+   *bit_no += 1;
+ }
+ 
+ int bmp_iter_set (int *bi, int *bit_no);
+ void bitmap_initialize_stat (int, ...);
+ void bitmap_clear (void);
+ 
+ void
+ df_md_alloc (int bi, int bb_index, void *bb_info)
+ {
+   for (; bmp_iter_set (&bi, &bb_index); bmp_iter_next (&bi, &bb_index))
+ 
+     if (bb_info)
+       bitmap_clear ();
+     else
+       bitmap_initialize_stat (0);
+ }
Index: gcc/sel-sched.c
===================================================================
*** gcc/sel-sched.c	(revision 166235)
--- gcc/sel-sched.c	(working copy)
*************** find_best_expr (av_set_t *av_vliw_ptr, b
*** 4403,4409 ****
      {
        can_issue_more = invoke_aftermath_hooks (fence, EXPR_INSN_RTX (best),
                                                 can_issue_more);
!       if (can_issue_more == 0)
          *pneed_stall = 1;
      }
  
--- 4403,4410 ----
      {
        can_issue_more = invoke_aftermath_hooks (fence, EXPR_INSN_RTX (best),
                                                 can_issue_more);
!       if (targetm.sched.variable_issue
! 	  && can_issue_more == 0)
          *pneed_stall = 1;
      }
  
*************** fill_insns (fence_t fence, int seqno, il
*** 5511,5517 ****
        blist_t *bnds_tailp1, *bndsp;
        expr_t expr_vliw;
        int need_stall;
!       int was_stall = 0, scheduled_insns = 0, stall_iterations = 0;
        int max_insns = pipelining_p ? issue_rate : 2 * issue_rate;
        int max_stall = pipelining_p ? 1 : 3;
        bool last_insn_was_debug = false;
--- 5512,5518 ----
        blist_t *bnds_tailp1, *bndsp;
        expr_t expr_vliw;
        int need_stall;
!       int was_stall = 0, scheduled_insns = 0;
        int max_insns = pipelining_p ? issue_rate : 2 * issue_rate;
        int max_stall = pipelining_p ? 1 : 3;
        bool last_insn_was_debug = false;
*************** fill_insns (fence_t fence, int seqno, il
*** 5530,5545 ****
        do
          {
            expr_vliw = find_best_expr (&av_vliw, bnds, fence, &need_stall);
!           if (!expr_vliw && need_stall)
              {
                /* All expressions required a stall.  Do not recompute av sets
                   as we'll get the same answer (modulo the insns between
                   the fence and its boundary, which will not be available for
!                  pipelining).  */
!               gcc_assert (! expr_vliw && stall_iterations < 2);
!               was_stall++;
! 	      /* If we are going to stall for too long, break to recompute av
  		 sets and bring more insns for pipelining.  */
  	      if (need_stall <= 3)
  		stall_for_cycles (fence, need_stall);
  	      else
--- 5531,5545 ----
        do
          {
            expr_vliw = find_best_expr (&av_vliw, bnds, fence, &need_stall);
!           if (! expr_vliw && need_stall)
              {
                /* All expressions required a stall.  Do not recompute av sets
                   as we'll get the same answer (modulo the insns between
                   the fence and its boundary, which will not be available for
!                  pipelining).
! 		 If we are going to stall for too long, break to recompute av
  		 sets and bring more insns for pipelining.  */
+               was_stall++;
  	      if (need_stall <= 3)
  		stall_for_cycles (fence, need_stall);
  	      else
*************** init_seqno_1 (basic_block bb, sbitmap vi
*** 6712,6717 ****
--- 6712,6719 ----
  
  	  init_seqno_1 (succ, visited_bbs, blocks_to_reschedule);
  	}
+       else if (blocks_to_reschedule)
+         bitmap_set_bit (forced_ebb_heads, succ->index);
      }
  
    for (insn = BB_END (bb); insn != note; insn = PREV_INSN (insn))
*************** reset_sched_cycles_in_current_ebb (void)
*** 6966,6971 ****
--- 6968,6974 ----
    int last_clock = 0;
    int haifa_last_clock = -1;
    int haifa_clock = 0;
+   int issued_insns = 0;
    insn_t insn;
  
    if (targetm.sched.init)
*************** reset_sched_cycles_in_current_ebb (void)
*** 7020,7033 ****
            haifa_cost = cost;
            after_stall = 1;
          }
! 
        if (haifa_cost > 0)
  	{
  	  int i = 0;
  
  	  while (haifa_cost--)
  	    {
! 	      advance_state (curr_state);
                i++;
  
  	      if (sched_verbose >= 2)
--- 7023,7038 ----
            haifa_cost = cost;
            after_stall = 1;
          }
!       if (haifa_cost == 0
! 	  && issued_insns == issue_rate)
! 	haifa_cost = 1;
        if (haifa_cost > 0)
  	{
  	  int i = 0;
  
  	  while (haifa_cost--)
  	    {
! 	      advance_state (curr_state), issued_insns = 0;
                i++;
  
  	      if (sched_verbose >= 2)
*************** reset_sched_cycles_in_current_ebb (void)
*** 7047,7052 ****
--- 7052,7059 ----
  	    }
  
  	  haifa_clock += i;
+           if (sched_verbose >= 2)
+             sel_print ("haifa clock: %d\n", haifa_clock);
  	}
        else
  	gcc_assert (haifa_cost == 0);
*************** reset_sched_cycles_in_current_ebb (void)
*** 7059,7080 ****
  					    haifa_last_clock, haifa_clock,
  					    &sort_p))
  	  {
! 	    advance_state (curr_state);
  	    haifa_clock++;
  	    if (sched_verbose >= 2)
                {
                  sel_print ("advance_state (dfa_new_cycle)\n");
                  debug_state (curr_state);
                }
            }
  
        if (real_insn)
  	{
  	  cost = state_transition (curr_state, insn);
  
            if (sched_verbose >= 2)
!             debug_state (curr_state);
! 
  	  gcc_assert (cost < 0);
  	}
  
--- 7066,7092 ----
  					    haifa_last_clock, haifa_clock,
  					    &sort_p))
  	  {
! 	    advance_state (curr_state), issued_insns = 0;
  	    haifa_clock++;
  	    if (sched_verbose >= 2)
                {
                  sel_print ("advance_state (dfa_new_cycle)\n");
                  debug_state (curr_state);
+ 		sel_print ("haifa clock: %d\n", haifa_clock + 1);
                }
            }
  
        if (real_insn)
  	{
  	  cost = state_transition (curr_state, insn);
+ 	  issued_insns++;
  
            if (sched_verbose >= 2)
! 	    {
! 	      sel_print ("scheduled insn %d, clock %d\n", INSN_UID (insn),
! 			 haifa_clock + 1);
!               debug_state (curr_state);
! 	    }
  	  gcc_assert (cost < 0);
  	}
  
*************** sel_sched_region_1 (void)
*** 7518,7524 ****
                    continue;
                  }
  
!               if (bitmap_clear_bit (blocks_to_reschedule, bb->index))
                  {
                    flist_tail_init (new_fences);
  
--- 7530,7536 ----
                    continue;
                  }
  
!               if (bitmap_bit_p (blocks_to_reschedule, bb->index))
                  {
                    flist_tail_init (new_fences);
  

Re: [PATCH] Fix PR45352

[Vladimir Makarov]

On 11/03/2010 08:18 AM, Andrey Belevantsev wrote:
> Hello,
>
> Now that we have agreed about the scheduler not issuing more than 
> issue_rate insns, the PR can actually be solved easier.  I only need 
> the loop resetting sched cycles be in line with the rest of the 
> scheduler and advance state when we have issued issue_rate insns.  The 
> only bits from the old patch that are needed are not setting 
> need_stall to 1 when no variable_issue hook exists and properly 
> synchronizing rescheduling regions and resetting cycles regions.  
> These do not make us trying to issue more than issue_rate insns.
>
> The patch fixes all test cases and in the process of bootstrap/test on 
> ia64, ok if it passes?
>
Ok. Just one small thing.  Could you instead of

  advance_state (curr_state), issued_insns = 0;

use

advance_state (curr_state);
issued_insns = 0;

There are two such places.

Thanks for working on PR and for the patch, Andrey.

> Andrey
>
> 2010-10-22  Andrey Belevantsev <abel@ispras.ru>
>
>     PR rtl-optimization/45352
>     * sel-sched.c (find_best_expr): Do not set pneed_stall when
>     the variable_issue hook is not implemented.
>     (fill_insns): Remove dead variable stall_iterations.
>     (init_seqno_1): Force EBB start for resetting sched cycles on any
>     successor blocks of the rescheduled region.
>     (sel_sched_region_1): Use bitmap_bit_p instead of bitmap_clear_bit.
>     (reset_sched_cycles_in_current_ebb): Add debug printing.
>     New variable issued_insns.  Advance state when we have issued
>     issue_rate insns.
>
>     gcc.dg/pr45352.c, gcc.dg/pr45352-1.c, gcc.dg/pr45352-2.c: New tests.
>     gcc.target/i386/pr45352.c, gcc.target/i386/pr45352-1.c,
>     gcc.target/i386/pr45352-2.c: New tests.

Re: [PATCH] Fix PR45352

[H.J. Lu]

2010/11/3 Andrey Belevantsev <abel@ispras.ru>:
> Hello,
>
> Now that we have agreed about the scheduler not issuing more than issue_rate
> insns, the PR can actually be solved easier.  I only need the loop resetting
> sched cycles be in line with the rest of the scheduler and advance state
> when we have issued issue_rate insns.  The only bits from the old patch that
> are needed are not setting need_stall to 1 when no variable_issue hook
> exists and properly synchronizing rescheduling regions and resetting cycles
> regions.  These do not make us trying to issue more than issue_rate insns.
>
> The patch fixes all test cases and in the process of bootstrap/test on ia64,
> ok if it passes?
>
> Andrey
>
> 2010-10-22  Andrey Belevantsev  <abel@ispras.ru>
>
>    PR rtl-optimization/45352
>    * sel-sched.c (find_best_expr): Do not set pneed_stall when
>    the variable_issue hook is not implemented.
>    (fill_insns): Remove dead variable stall_iterations.
>    (init_seqno_1): Force EBB start for resetting sched cycles on any
>    successor blocks of the rescheduled region.
>    (sel_sched_region_1): Use bitmap_bit_p instead of bitmap_clear_bit.
>    (reset_sched_cycles_in_current_ebb): Add debug printing.
>    New variable issued_insns.  Advance state when we have issued
>    issue_rate insns.
>
>    gcc.dg/pr45352.c, gcc.dg/pr45352-1.c, gcc.dg/pr45352-2.c: New tests.
>    gcc.target/i386/pr45352.c, gcc.target/i386/pr45352-1.c,
>    gcc.target/i386/pr45352-2.c: New tests.
>

This caused:

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


-- 
H.J.

Re: [PATCH] Fix PR45352

[Andrey Belevantsev]

Hello,

On 08.11.2010 17:26, H.J. Lu wrote:
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46366
I've committed the following (r166798) as obvious to fix the test case. 
I've made sure that the test still fails without the patch for PR45352.
Sorry for not noticing the warnings earlier.

Andrey

Index: gcc/testsuite/gcc.target/i386/pr45352-2.c
===================================================================
*** gcc/testsuite/gcc.target/i386/pr45352-2.c   (revision 166797)
--- gcc/testsuite/gcc.target/i386/pr45352-2.c   (revision 166798)
*************** typedef struct
*** 40,48 ****
   {
   int xvmc_last_slice_code;}
   mpeg2dec_accel_t;
! static bitstream_init (picture_t * picture, void *start)
   {
     picture->bitstream_ptr = start;
   }
   static slice_xvmc_init (picture_t * picture, int code)
   {
--- 40,49 ----
   {
   int xvmc_last_slice_code;}
   mpeg2dec_accel_t;
! static int bitstream_init (picture_t * picture, void *start)
   {
     picture->bitstream_ptr = start;
+   return (int) (long) start;
   }
   static slice_xvmc_init (picture_t * picture, int code)
   {
*************** static slice_xvmc_init (picture_t * pict
*** 55,61 ****
       picture->f_motion.ref
         [0]
         [0]
!       = forward_reference_frame->base + (offset ? picture->pitches[0] : 0);
     picture->f_motion.ref[0][1] = (offset);
     if (picture->picture_structure)
         picture->pitches[0] <<= picture->pitches[1] <<= 1;
--- 56,62 ----
       picture->f_motion.ref
         [0]
         [0]
!       = (char) (long) (forward_reference_frame->base + (offset ? 
picture->pitches[0] : 0));
     picture->f_motion.ref[0][1] = (offset);
     if (picture->picture_structure)
         picture->pitches[0] <<= picture->pitches[1] <<= 1;
*************** void
*** 90,96 ****
   mpeg2_xvmc_slice
     (mpeg2dec_accel_t * accel, picture_t * picture, int code, uint8_t 
buffer,int mba_inc)
   {
!   xine_xvmc_t * xvmc = bitstream_init (picture, buffer);
     slice_xvmc_init (picture, code);
       while (1)
         {
--- 91,97 ----
   mpeg2_xvmc_slice
     (mpeg2dec_accel_t * accel, picture_t * picture, int code, uint8_t 
buffer,int mba_inc)
   {
!   xine_xvmc_t * xvmc = (xine_xvmc_t *) (long) bitstream_init (picture, 
(void *) (long) buffer);
     slice_xvmc_init (picture, code);
       while (1)
         {
Index: gcc/testsuite/ChangeLog
===================================================================
*** gcc/testsuite/ChangeLog     (revision 166797)
--- gcc/testsuite/ChangeLog     (revision 166798)
***************
*** 1,3 ****
--- 1,9 ----
+ 2010-11-16  Andrey Belevantsev  <abel@ispras.ru>
+
+       PR rtl-optimization/46366
+       * gcc.target/i386/pr45352-2.c: Silence warnings by using appropriate
+       casts.
+
   2010-11-16  Richard Guenther  <rguenther@suse.de>

         * gcc.dg/tree-ssa/ssa-fre-30.c: New testcase.

Re: [PATCH] Fix PR45352

[H.J. Lu]

On Mon, Nov 8, 2010 at 6:26 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 2010/11/3 Andrey Belevantsev <abel@ispras.ru>:
>> Hello,
>>
>> Now that we have agreed about the scheduler not issuing more than issue_rate
>> insns, the PR can actually be solved easier.  I only need the loop resetting
>> sched cycles be in line with the rest of the scheduler and advance state
>> when we have issued issue_rate insns.  The only bits from the old patch that
>> are needed are not setting need_stall to 1 when no variable_issue hook
>> exists and properly synchronizing rescheduling regions and resetting cycles
>> regions.  These do not make us trying to issue more than issue_rate insns.
>>
>> The patch fixes all test cases and in the process of bootstrap/test on ia64,
>> ok if it passes?
>>
>> Andrey
>>
>> 2010-10-22  Andrey Belevantsev  <abel@ispras.ru>
>>
>>    PR rtl-optimization/45352
>>    * sel-sched.c (find_best_expr): Do not set pneed_stall when
>>    the variable_issue hook is not implemented.
>>    (fill_insns): Remove dead variable stall_iterations.
>>    (init_seqno_1): Force EBB start for resetting sched cycles on any
>>    successor blocks of the rescheduled region.
>>    (sel_sched_region_1): Use bitmap_bit_p instead of bitmap_clear_bit.
>>    (reset_sched_cycles_in_current_ebb): Add debug printing.
>>    New variable issued_insns.  Advance state when we have issued
>>    issue_rate insns.
>>
>>    gcc.dg/pr45352.c, gcc.dg/pr45352-1.c, gcc.dg/pr45352-2.c: New tests.
>>    gcc.target/i386/pr45352.c, gcc.target/i386/pr45352-1.c,
>>    gcc.target/i386/pr45352-2.c: New tests.
>>
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46366
>

This also caused:

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


-- 
H.J.

[PATCH] Fix PR 46521 and 46522 (remnants of PR45352)

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]

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.


[-- Attachment #2: pr45352-2.diff --]
[-- Type: text/x-patch, Size: 3654 bytes --]

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);
+  }
+}

Re: [PATCH] Fix PR 46521 and 46522 (remnants of PR45352)

[Vladimir Makarov]

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.

Re: [PATCH] Fix PR 46521 and 46522 (remnants of PR45352)

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

> On 12/21/2010 10:58 AM, Andrey Belevantsev wrote:
>
>> 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.
Sigh, I have overlooked that we also need to recheck the state when we have 
reached the issue_rate limit (as we agreed on earlier this year), not only 
when we have a data dependency stall.  So the above amendment to the patch 
is needed, bootstrapped and tested on x86-64 with selective scheduler 
enabled.  Ok for trunk and branches?

Andrey

	2010-12-21 Andrey Belevantsev <abel@ispras.ru>
	
	PR rtl-optimization/45352
	* sel-sched.c (reset_sched_cycles_in_current_ebb): Also recheck the DFA state
	in the advancing loop when we have issued issue_rate insns.

gcc/testsuite:

	2010-12-21 Andrey Belevantsev <abel@ispras.ru>
	
	PR rtl-optimization/45352
	gcc.dg/pr45352-3.c: New.

[-- Attachment #2: pr45352-3.diff --]
[-- Type: text/x-patch, Size: 3107 bytes --]

Index: testsuite/gcc.dg/pr45352-3.c
===================================================================
*** testsuite/gcc.dg/pr45352-3.c	(revision 0)
--- testsuite/gcc.dg/pr45352-3.c	(revision 0)
***************
*** 0 ****
--- 1,16 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O -fprofile-generate -fgcse -fno-gcse-lm -fgcse-sm -fno-ivopts -fno-tree-loop-im -ftree-pre -funroll-loops -fno-web -fschedule-insns2 -fselective-scheduling2 -fsel-sched-pipelining" } */
+ 
+ extern volatile float f[];
+ 
+ void foo (void)
+ {
+   int i;
+   for (i = 0; i < 100; i++)
+     f[i] = 0;
+   for (i = 0; i < 100; i++)
+     f[i] = 0;
+   for (i = 0; i < 100; i++)
+     if (f[i])
+       __builtin_abort ();
+ }
Index: sel-sched.c
===================================================================
*** sel-sched.c	(revision 168224)
--- sel-sched.c	(working copy)
*************** reset_sched_cycles_in_current_ebb (void)
*** 6990,6996 ****
      {
        int cost, haifa_cost;
        int sort_p;
!       bool asm_p, real_insn, after_stall;
        int clock;
  
        if (!INSN_P (insn))
--- 6990,6996 ----
      {
        int cost, haifa_cost;
        int sort_p;
!       bool asm_p, real_insn, after_stall, all_issued;
        int clock;
  
        if (!INSN_P (insn))
*************** reset_sched_cycles_in_current_ebb (void)
*** 7026,7033 ****
            haifa_cost = cost;
            after_stall = 1;
          }
!       if (haifa_cost == 0
! 	  && issued_insns == issue_rate)
  	haifa_cost = 1;
        if (haifa_cost > 0)
  	{
--- 7026,7033 ----
            haifa_cost = cost;
            after_stall = 1;
          }
!       all_issued = issued_insns == issue_rate;
!       if (haifa_cost == 0 && all_issued)
  	haifa_cost = 1;
        if (haifa_cost > 0)
  	{
*************** reset_sched_cycles_in_current_ebb (void)
*** 7055,7065 ****
                  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);
--- 7055,7066 ----
                  break;
  
                /* When the data dependency stall is longer than the DFA stall,
!                  and when we have issued exactly issue_rate insns and stalled,
!                  it could be that after this 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 || all_issued)
                    && real_insn
                    && haifa_cost == 0)
                  haifa_cost = estimate_insn_cost (insn, curr_state);

Re: [PATCH] Fix PR 46521 and 46522 (remnants of PR45352)

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Hi,

I'm pinging this patch -- Zdenek has reported in the PR 45352 that it 
(finally) works fine with his testing.  The only change with the posted 
patch will be that I will update the copyright years with 2011 :)

Andrey

On 24.12.2010 12:45, Andrey Belevantsev wrote:
> Sigh, I have overlooked that we also need to recheck the state when we have
> reached the issue_rate limit (as we agreed on earlier this year), not only
> when we have a data dependency stall. So the above amendment to the patch
> is needed, bootstrapped and tested on x86-64 with selective scheduler
> enabled. Ok for trunk and branches?
>
> Andrey
>
> 2010-12-21 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/45352
> * sel-sched.c (reset_sched_cycles_in_current_ebb): Also recheck the DFA state
> in the advancing loop when we have issued issue_rate insns.
>
> gcc/testsuite:
>
> 2010-12-21 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/45352
> gcc.dg/pr45352-3.c: New.


[-- Attachment #2: pr45352-3.diff --]
[-- Type: text/x-patch, Size: 3107 bytes --]

Index: testsuite/gcc.dg/pr45352-3.c
===================================================================
*** testsuite/gcc.dg/pr45352-3.c	(revision 0)
--- testsuite/gcc.dg/pr45352-3.c	(revision 0)
***************
*** 0 ****
--- 1,16 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+ /* { dg-options "-O -fprofile-generate -fgcse -fno-gcse-lm -fgcse-sm -fno-ivopts -fno-tree-loop-im -ftree-pre -funroll-loops -fno-web -fschedule-insns2 -fselective-scheduling2 -fsel-sched-pipelining" } */
+ 
+ extern volatile float f[];
+ 
+ void foo (void)
+ {
+   int i;
+   for (i = 0; i < 100; i++)
+     f[i] = 0;
+   for (i = 0; i < 100; i++)
+     f[i] = 0;
+   for (i = 0; i < 100; i++)
+     if (f[i])
+       __builtin_abort ();
+ }
Index: sel-sched.c
===================================================================
*** sel-sched.c	(revision 168224)
--- sel-sched.c	(working copy)
*************** reset_sched_cycles_in_current_ebb (void)
*** 6990,6996 ****
      {
        int cost, haifa_cost;
        int sort_p;
!       bool asm_p, real_insn, after_stall;
        int clock;
  
        if (!INSN_P (insn))
--- 6990,6996 ----
      {
        int cost, haifa_cost;
        int sort_p;
!       bool asm_p, real_insn, after_stall, all_issued;
        int clock;
  
        if (!INSN_P (insn))
*************** reset_sched_cycles_in_current_ebb (void)
*** 7026,7033 ****
            haifa_cost = cost;
            after_stall = 1;
          }
!       if (haifa_cost == 0
! 	  && issued_insns == issue_rate)
  	haifa_cost = 1;
        if (haifa_cost > 0)
  	{
--- 7026,7033 ----
            haifa_cost = cost;
            after_stall = 1;
          }
!       all_issued = issued_insns == issue_rate;
!       if (haifa_cost == 0 && all_issued)
  	haifa_cost = 1;
        if (haifa_cost > 0)
  	{
*************** reset_sched_cycles_in_current_ebb (void)
*** 7055,7065 ****
                  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);
--- 7055,7066 ----
                  break;
  
                /* When the data dependency stall is longer than the DFA stall,
!                  and when we have issued exactly issue_rate insns and stalled,
!                  it could be that after this 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 || all_issued)
                    && real_insn
                    && haifa_cost == 0)
                  haifa_cost = estimate_insn_cost (insn, curr_state);

Re: [PATCH] Fix PR 46521 and 46522 (remnants of PR45352)

[Vladimir Makarov]

On 01/12/2011 03:41 AM, Andrey Belevantsev wrote:
> Hi,
>
> I'm pinging this patch -- Zdenek has reported in the PR 45352 that it 
> (finally) works fine with his testing.  The only change with the 
> posted patch will be that I will update the copyright years with 2011 :)
>
> Andrey
>
The patch is ok.  Thanks for it, Andrey.

> On 24.12.2010 12:45, Andrey Belevantsev wrote:
>> Sigh, I have overlooked that we also need to recheck the state when 
>> we have
>> reached the issue_rate limit (as we agreed on earlier this year), not 
>> only
>> when we have a data dependency stall. So the above amendment to the 
>> patch
>> is needed, bootstrapped and tested on x86-64 with selective scheduler
>> enabled. Ok for trunk and branches?
>>
>> Andrey
>>
>> 2010-12-21 Andrey Belevantsev <abel@ispras.ru>
>>
>> PR rtl-optimization/45352
>> * sel-sched.c (reset_sched_cycles_in_current_ebb): Also recheck the 
>> DFA state
>> in the advancing loop when we have issued issue_rate insns.
>>
>> gcc/testsuite:
>>
>> 2010-12-21 Andrey Belevantsev <abel@ispras.ru>
>>
>> PR rtl-optimization/45352
>> gcc.dg/pr45352-3.c: New.
>