| Message ID | eb76f547-3840-76c6-fb17-4840d1bbbda2@ispras.ru |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR 83530 | expand |
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 <abel@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); > }
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 <abel@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); >> } Index: gcc/ChangeLog =================================================================== *** gcc/ChangeLog (revision 259227) --- gcc/ChangeLog (revision 259228) *************** *** 1,3 **** --- 1,13 ---- + 2018-04-09 Andrey Belevantsev <abel@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. + 2018-04-08 Monk Chiang <sh.chiang04@gmail.com> * 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 <abel@ispras.ru> + + PR rtl-optimization/83530 + * gcc.dg/pr83530.c: New test. + 2018-04-07 Thomas Koenig <tkoenig@gcc.gnu.org> 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 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) || force_next_insn) /* Leave only the next insn in av_vliw. */ { av_set_iterator av_it; *************** sel_sched_region (int rgn) *** 7641,7649 **** if (schedule_p) sel_sched_region_1 (); else ! /* Force initialization of INSN_SCHED_CYCLEs for correct bundling. */ ! reset_sched_cycles_p = true; ! sel_region_finish (reset_sched_cycles_p); } --- 7645,7659 ---- if (schedule_p) sel_sched_region_1 (); else ! { ! /* Schedule always selecting the next insn to make the correct data ! for bundling or other later passes. */ ! pipelining_p = false; ! force_next_insn = 1; ! sel_sched_region_1 (); ! force_next_insn = 0; ! } ! reset_sched_cycles_p = pipelining_p; sel_region_finish (reset_sched_cycles_p); }
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 00000000000..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; +}