Re: Fix PR 83530

2018-04-09 Thread Andrey Belevantsev
On 06.04.2018 18:55, Alexander Monakov wrote:
> 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.

Thank, I've committed the attached.

Andrey

> 
>> 2018-04-03  Andrey Belevantsev  
>>
>>  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);
>>  }

Index: gcc/ChangeLog
===
*** gcc/ChangeLog   (revision 259227)
--- gcc/ChangeLog   (revision 259228)
***
*** 1,3 
--- 1,13 
+ 2018-04-09  Andrey Belevantsev  
+ 
+   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.
+ 
  2018-04-08  Monk Chiang  
  
* config.gcc (nds32le-*-*, nds32be-*-*): Add nds32/nds32_intrinsic.h
Index: gcc/testsuite/gcc.dg/pr83530.c
===
*** gcc/testsuite/gcc.dg/pr83530.c  (revision 0)
--- gcc/testsuite/gcc.dg/pr83530.c  (revision 259228)
***
*** 0 
--- 1,15 
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O2 -fmodulo-sched -fselective-scheduling2" } */
+ int vm, z0;
+ short int mz;
+ 
+ int
+ ny (void)
+ {
+   int ch;
+ 
+   for (ch = 0; ch < 6; ++ch)
+ vm += ch / vm;
+ 
+   return z0 + mz;
+ }
Index: gcc/testsuite/ChangeLog
===
*** gcc/testsuite/ChangeLog (revision 259227)
--- gcc/testsuite/ChangeLog (revision 259228)
***
*** 1,3 
--- 1,8 
+ 2018-04-09  Andrey Belevantsev  
+ 
+   PR rtl-optimization/83530
+   * gcc.dg/pr83530.c: New test.
+ 
  2018-04-07  Thomas Koenig  
  
PR middle-end/82976
Index: gcc/sel-sched.c
===
*** gcc/sel-sched.c (revision 259227)
--- gcc/sel-sched.c (revision 259228)
*** remove_temp_moveop_nops (bool full_tidyi
*** 5004,5015 
 distinguishing between bookkeeping copies and original insns.  */
  static int max_uid_before_move_op = 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))
  /* Leave only the next insn in av_vliw.  */
  {
av_set_iterator av_it;
--- 5004,5019 
 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 alrea

Re: Fix PR 83530

2018-04-06 Thread Alexander Monakov
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  
> 
>   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);
>  }


Fix PR 83530

2018-04-03 Thread Andrey Belevantsev
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.

Best,
Andrey

2018-04-03  Andrey Belevantsev  

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.


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

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 76092f9587a..fca2b69c5ee 100644
--- 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.  */
-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);
 }
diff --git a/gcc/testsuite/gcc.dg/pr83530.c b/gcc/testsuite/gcc.dg/pr83530.c
new file mode 100644
index 000..f4d8927de92
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83530.c
@@ -0,0 +1,15 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fmodulo-sched -fselective-scheduling2" } */
+int vm, z0;
+short int mz;
+
+int
+ny (void)
+{
+  int ch;
+
+  for (ch = 0; ch < 6; ++ch)
+vm += ch / vm;
+
+  return z0 + mz;
+}