| Message ID | alpine.LNX.2.11.1502242111100.27529@monopod.intra.ispras.ru |
|---|---|
| State | New |
| Headers | show |
Hi All, I prepared new patch which includes test-case. I can't agree with patch proposed by Alexander since other functions doing ready list reordering also use HID interface, so I put escape check in ix86_sched_reorder. Is it OK for trunk? 2015-02-25 Yuri Rumyantsev <ysrumyan@gmail.com> PR target/65161 * config/i386/i386.c (ix86_sched_reorder): Skip instruction reordering for selective scheduling. gcc/testsuite/ChangeLog * gcc.target/i386/pr65161.c: New test. 2015-02-25 2:54 GMT+03:00 Andrey Belevantsev <abel@ispras.ru>: > On 24.02.2015 21:16, Alexander Monakov wrote: >> >> >> >> On Tue, 24 Feb 2015, Yuri Rumyantsev wrote: >> >>> Hi All! >>> >>> Here is a simple patch to not perform instruction reordering for >>> selective scheduling since it uses interface of list scheduling >>> defined in "sched-int.h". >> >> >> As I see, the exact problem is that swap_top_of_ready_list accesses HID, >> so >> please use the more specialized patch below instead. > > > You have missed a space before call parentheses in the patch, otherwise it > looks fine. > > Andrey > > > >> >> Thanks. >> Alexander >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 7f5796a..6eccd54 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -26615,6 +26615,12 @@ swap_top_of_ready_list (rtx_insn **ready, int >> n_ready) >> dep_t dep; >> int clock1 = -1; >> int clock2 = -1; >> + >> + /* The following heuristic inspects h_i_d, but it is not extended for >> insns >> + created when doing selective scheduling. */ >> + if (sel_sched_p()) >> + return false; >> + >> #define INSN_TICK(INSN) (HID (INSN)->tick) >> >> if (!TARGET_SILVERMONT && !TARGET_INTEL) >> >
On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > Hi All, > > I prepared new patch which includes test-case. > > I can't agree with patch proposed by Alexander since other functions > doing ready list reordering also use HID interface, so I put escape > check in ix86_sched_reorder. I don't see how that is the case. Can you point me to specific lines of code in ix86_sched_reorder or its callees that access HID? In any case please use sel_sched_p () rather than flag_selective_scheduling2. Thanks. Alexander
Here is updated patch accordingly to Alexander comments. BTW another function using HID interface is do_reorder_for_imul and it is called from ix86_sched_reorder. Is it OK for trunk? 2015-02-25 13:26 GMT+03:00 Alexander Monakov <amonakov@ispras.ru>: > > > On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > >> Hi All, >> >> I prepared new patch which includes test-case. >> >> I can't agree with patch proposed by Alexander since other functions >> doing ready list reordering also use HID interface, so I put escape >> check in ix86_sched_reorder. > > I don't see how that is the case. Can you point me to specific lines of code > in ix86_sched_reorder or its callees that access HID? > > In any case please use sel_sched_p () rather than flag_selective_scheduling2. > > Thanks. > > Alexander
On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > Here is updated patch accordingly to Alexander comments. > > BTW another function using HID interface is do_reorder_for_imul and it > is called from ix86_sched_reorder. do_reorder_for_imul uses dependency list iteration macros, which use HDID, not HID, and HDID is populated during selective scheduling. How exactly does do_reorder_for_imul access HID? Alexander
I modified patch accordingly to Alexander comments. Is it OK for trunk? 2015-02-25 15:38 GMT+03:00 Alexander Monakov <amonakov@ispras.ru>: > > > On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > >> Here is updated patch accordingly to Alexander comments. >> >> BTW another function using HID interface is do_reorder_for_imul and it >> is called from ix86_sched_reorder. > > do_reorder_for_imul uses dependency list iteration macros, which use HDID, not > HID, and HDID is populated during selective scheduling. How exactly does > do_reorder_for_imul access HID? > > Alexander
On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > I modified patch accordingly to Alexander comments. > > Is it OK for trunk? If possible, please add a short comment explaining why a shortcut is necessary, for example "HID is not populated during selective scheduling". OK for trunk from selective scheduler maintainer's perspective. Thanks. Alexander
Hi All, Here is updated patch to fix ICE. Is it OK for trunk? 2015-02-25 Yuri Rumyantsev <ysrumyan@gmail.com> PR target/65161 * config/i386/i386.c (ix86_sched_reorder): Skip instruction reordering for selective scheduling. gcc/testsuite/ChangeLog * gcc.target/i386/pr65161.c: New test. 2015-02-25 17:04 GMT+03:00 Alexander Monakov <amonakov@ispras.ru>: > > > On Wed, 25 Feb 2015, Yuri Rumyantsev wrote: > >> I modified patch accordingly to Alexander comments. >> >> Is it OK for trunk? > > If possible, please add a short comment explaining why a shortcut is > necessary, for example "HID is not populated during selective scheduling". > > OK for trunk from selective scheduler maintainer's perspective. > > Thanks. > Alexander
On Wed, Feb 25, 2015 at 3:24 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: > Hi All, > > Here is updated patch to fix ICE. > > Is it OK for trunk? > > 2015-02-25 Yuri Rumyantsev <ysrumyan@gmail.com> > > PR target/65161 > * config/i386/i386.c (ix86_sched_reorder): Skip instruction reordering > for selective scheduling. > > gcc/testsuite/ChangeLog > * gcc.target/i386/pr65161.c: New test. OK. Thanks, Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 7f5796a..6eccd54 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -26615,6 +26615,12 @@ swap_top_of_ready_list (rtx_insn **ready, int n_ready) dep_t dep; int clock1 = -1; int clock2 = -1; + + /* The following heuristic inspects h_i_d, but it is not extended for insns + created when doing selective scheduling. */ + if (sel_sched_p()) + return false; + #define INSN_TICK(INSN) (HID (INSN)->tick) if (!TARGET_SILVERMONT && !TARGET_INTEL)