| Message ID | 9bbafbac-047c-4e6c-cae1-15df6c0db6dc@ispras.ru |
|---|---|
| State | New |
| Headers | show |
On Mon, 14 Mar 2016, Andrey Belevantsev wrote: > In this case, we get an inconsistency between the sched-deps interface, saying > we can't move an insn writing the si register through a vector insn, and the > liveness analysis, saying we can. The latter doesn't take into account > implicit_reg_pending_clobbers set calculated in sched-deps before register > allocation. The solution is to reflect this set in our insn data > (sets/uses/clobbers). > > Ok for trunk? One nit; the prototype of the new function: extern void get_implicit_reg_pending_clobbers (rtx_insn *, HARD_REG_SET *); has source operand on the left, destination on the right; it's probably nicer to swap them around. OK as far as selective scheduler changes go, but this also needs a general scheduler maintainer ack for the sched-deps.c change. Vladimir, can you have a look? Thanks. Alexander
On 03/14/2016 05:23 PM, Alexander Monakov wrote: > On Mon, 14 Mar 2016, Andrey Belevantsev wrote: >> In this case, we get an inconsistency between the sched-deps interface, saying >> we can't move an insn writing the si register through a vector insn, and the >> liveness analysis, saying we can. The latter doesn't take into account >> implicit_reg_pending_clobbers set calculated in sched-deps before register >> allocation. The solution is to reflect this set in our insn data >> (sets/uses/clobbers). >> >> Ok for trunk? > > One nit; the prototype of the new function: > > extern void get_implicit_reg_pending_clobbers (rtx_insn *, HARD_REG_SET *); > > has source operand on the left, destination on the right; it's probably nicer > to swap them around. > > OK as far as selective scheduler changes go, but this also needs a general > scheduler maintainer ack for the sched-deps.c change. Vladimir, can you have > a look? Needs better documentation of the new function's arguments (as per general requirements for such things), but otherwise that part is ok (either arg order). The sel-sched parts should also have proper function comments however, and here: + { + SET_REGNO_REG_SET (IDATA_REG_SETS (id), regno); + } we don't use braces around single statements. Bernd
Hello, On 14.03.2016 19:45, Bernd Schmidt wrote: > On 03/14/2016 05:23 PM, Alexander Monakov wrote: >> On Mon, 14 Mar 2016, Andrey Belevantsev wrote: >>> In this case, we get an inconsistency between the sched-deps interface, >>> saying >>> we can't move an insn writing the si register through a vector insn, and >>> the >>> liveness analysis, saying we can. The latter doesn't take into account >>> implicit_reg_pending_clobbers set calculated in sched-deps before register >>> allocation. The solution is to reflect this set in our insn data >>> (sets/uses/clobbers). >>> >>> Ok for trunk? >> >> One nit; the prototype of the new function: >> >> extern void get_implicit_reg_pending_clobbers (rtx_insn *, HARD_REG_SET *); >> >> has source operand on the left, destination on the right; it's probably >> nicer >> to swap them around. >> >> OK as far as selective scheduler changes go, but this also needs a general >> scheduler maintainer ack for the sched-deps.c change. Vladimir, can you >> have >> a look? > > Needs better documentation of the new function's arguments (as per general > requirements for such things), but otherwise that part is ok (either arg > order). The sel-sched parts should also have proper function comments > however, and here: > > + { > + SET_REGNO_REG_SET (IDATA_REG_SETS (id), regno); > + } > > we don't use braces around single statements. I've incorporated both yours and Alexander's comments and committed the patch as rev. 234216. Andrey > > > Bernd
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c index 4961dfb..3d4a1d5 100644 --- a/gcc/sched-deps.c +++ b/gcc/sched-deps.c @@ -2860,6 +2860,17 @@ sched_macro_fuse_insns (rtx_insn *insn) } +/* Get the implicit reg pending clobbers for INSN. */ +void +get_implicit_reg_pending_clobbers (rtx_insn *insn, HARD_REG_SET *temp) +{ + extract_insn (insn); + preprocess_constraints (insn); + alternative_mask preferred = get_preferred_alternatives (insn); + ira_implicitly_set_insn_hard_regs (temp, preferred); + AND_COMPL_HARD_REG_SET (*temp, ira_no_alloc_regs); +} + /* Analyze an INSN with pattern X to find all dependencies. */ static void sched_analyze_insn (struct deps_desc *deps, rtx x, rtx_insn *insn) @@ -2872,12 +2883,7 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, rtx_insn *insn) if (! reload_completed) { HARD_REG_SET temp; - - extract_insn (insn); - preprocess_constraints (insn); - alternative_mask prefrred = get_preferred_alternatives (insn); - ira_implicitly_set_insn_hard_regs (&temp, prefrred); - AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs); + get_implicit_reg_pending_clobbers (insn, &temp); IOR_HARD_REG_SET (implicit_reg_pending_clobbers, temp); } diff --git a/gcc/sched-int.h b/gcc/sched-int.h index 378c3aa..d0e2c0e 100644 --- a/gcc/sched-int.h +++ b/gcc/sched-int.h @@ -1351,6 +1351,7 @@ extern void finish_deps_global (void); extern void deps_analyze_insn (struct deps_desc *, rtx_insn *); extern void remove_from_deps (struct deps_desc *, rtx_insn *); extern void init_insn_reg_pressure_info (rtx_insn *); +extern void get_implicit_reg_pending_clobbers (rtx_insn *, HARD_REG_SET *); extern dw_t get_dep_weak (ds_t, ds_t); extern ds_t set_dep_weak (ds_t, ds_t, dw_t); diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c index d6c86b8..e181cb9 100644 --- a/gcc/sel-sched-ir.c +++ b/gcc/sel-sched-ir.c @@ -2650,6 +2650,24 @@ maybe_downgrade_id_to_use (idata_t id, insn_t insn) IDATA_TYPE (id) = USE; } +/* Setup implicit register clobbers calculated by sched-deps before reload. */ +static void +setup_id_implicit_regs (idata_t id, insn_t insn) +{ + if (reload_completed) + return; + + HARD_REG_SET temp; + unsigned regno; + hard_reg_set_iterator hrsi; + + get_implicit_reg_pending_clobbers (insn, &temp); + EXECUTE_IF_SET_IN_HARD_REG_SET (temp, 0, regno, hrsi) + { + SET_REGNO_REG_SET (IDATA_REG_SETS (id), regno); + } +} + /* Setup register sets describing INSN in ID. */ static void setup_id_reg_sets (idata_t id, insn_t insn) @@ -2704,6 +2722,9 @@ setup_id_reg_sets (idata_t id, insn_t insn) } } + /* Also get implicit reg clobbers from sched-deps. */ + setup_id_implicit_regs (id, insn); + return_regset_to_pool (tmp); } @@ -2735,20 +2756,18 @@ deps_init_id (idata_t id, insn_t insn, bool force_unique_p) deps_init_id_data.force_use_p = false; init_deps (dc, false); - memcpy (&deps_init_id_sched_deps_info, &const_deps_init_id_sched_deps_info, sizeof (deps_init_id_sched_deps_info)); - if (spec_info != NULL) deps_init_id_sched_deps_info.generate_spec_deps = 1; - sched_deps_info = &deps_init_id_sched_deps_info; deps_analyze_insn (dc, insn); + /* Implicit reg clobbers received from sched-deps separately. */ + setup_id_implicit_regs (id, insn); free_deps (dc); - deps_init_id_data.id = NULL; } diff --git a/gcc/testsuite/gcc.target/i386/pr64411.C b/gcc/testsuite/gcc.target/i386/pr64411.C new file mode 100644 index 0000000..55858fb --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr64411.C @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -mcmodel=medium -fPIC -fschedule-insns -fselective-scheduling" } */ + +typedef __SIZE_TYPE__ size_t; + +extern "C" long strtol () + { return 0; } + +static struct { + void *sp[2]; +} info; + +union S813 +{ + void * c[5]; +} +s813; + +S813 a813[5]; +S813 check813 (S813, S813 *, S813); + +void checkx813 () +{ + __builtin_memset (&s813, '\0', sizeof (s813)); + __builtin_memset (&info, '\0', sizeof (info)); + check813 (s813, &a813[1], a813[2]); +}