On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> This issue is when we cannot correctly make insn tick data for the
> unscheduled code (but processed by the modulo scheduler).  Fixed by closely
> following usual scheduling process as described in the PR.

This is ok with the following nit-picks fixed.

> 2018-04-03  Andrey Belevantsev  <a...@ispras.ru>
> 
>       PR rtl-optimization/83530
> 
>       * sel-sched.c (force_next_insn): New global variable.
>       (remove_insn_for_debug): When force_next_insn is true, also leave only
> next insn
>       in the ready list.
>       (sel_sched_region): When the region wasn't scheduled, make another pass
> over it
>       with force_next_insn set to 1.

Overlong lines.

>       * gcc.dg/pr8350.c: New test.

Typo in test name.
 
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -5004,12 +5004,16 @@ remove_temp_moveop_nops (bool full_tidying)
>     distinguishing between bookkeeping copies and original insns.  */
>  static int max_uid_before_move_op = 0;
> 
> +/* When true, we're always scheduling next insn on the already scheduled code
> +   to get the right insn data for the following bundling or other passes.  */
> +static int force_next_insn = 0;
> +
>  /* Remove from AV_VLIW_P all instructions but next when debug counter
>     tells us so.  Next instruction is fetched from BNDS.  */
>  static void
>  remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
>  {
> -  if (! dbg_cnt (sel_sched_insn_cnt))
> +  if (! dbg_cnt (sel_sched_insn_cnt) || force_next_insn)
>      /* Leave only the next insn in av_vliw.  */
>      {
>        av_set_iterator av_it;
> @@ -7642,7 +7646,13 @@ sel_sched_region (int rgn)
>      sel_sched_region_1 ();
>    else
>      /* Force initialization of INSN_SCHED_CYCLEs for correct bundling.  */

I believe this comment needs updating.

Please also consider moving both assignments of reset_sched_cycles_p to
after the if-else statement, just before sel_region_finish call.

> -    reset_sched_cycles_p = true;
> +    {
> +      reset_sched_cycles_p = false;
> +      pipelining_p = false;
> +      force_next_insn = 1;
> +      sel_sched_region_1 ();
> +      force_next_insn = 0;
> +    }
> 
>    sel_region_finish (reset_sched_cycles_p);
>  }

Reply via email to