| Message ID | alpine.LNX.2.00.1106072117570.6250@monoid.intra.ispras.ru |
|---|---|
| State | New |
| Headers | show |
On 06/07/11 21:39:57, Alexander Monakov wrote: > 2011-06-07 Alexander Monakov <amonakov@ispras.ru> > > * sel-sched.c (move_op): Use correct type for 'res'. Verify that > code_motion_path_driver returned 0 or 1. > (sel_region_finish): Clear h_d_i_d. Alexander, will this patch fix the recently reported PR49303 and PR49304? If so, can a note be added to the ChangeLog entry? Also, would a DG test case make sense to help find future regressions, or is this failure mode simply too specific?
On Tue, Jun 7, 2011 at 10:09 PM, Gary Funck <gary@intrepid.com> wrote: > On 06/07/11 21:39:57, Alexander Monakov wrote: >> 2011-06-07 Alexander Monakov <amonakov@ispras.ru> >> >> * sel-sched.c (move_op): Use correct type for 'res'. Verify that >> code_motion_path_driver returned 0 or 1. >> (sel_region_finish): Clear h_d_i_d. > > Alexander, will this patch fix the recently reported PR49303 and PR49304? Yes. > If so, can a note be added to the ChangeLog entry? Also, would a > DG test case make sense to help find future regressions, or is this > failure mode simply too specific? I will amend the Changelog entry and add a smaller testcase that I have reduced from combine.c. Thanks for reporting this — I was in a hurry to submit this for approval before leaving. Alexander
On 06/07/2011 07:39 PM, Alexander Monakov wrote: > > > On Fri, 3 Jun 2011, Bernd Schmidt wrote: > >>>> Ok. Although I wonder how sel-sched can end up reusing an entry in >>>> h_d_i_d? > > Unlike Haifa scheduler, we recompute INSN_LUIDs for each region. However, we > call sched_deps_{init,finish} once per function (like Haifa) and that makes us > reuse entries in h_d_i_d. > > The proposed patch clears h_d_i_d when finishing a region in sel-sched. > Bootstrapped and regtested on ia64-linux, OK for trunk? You probably know best, so I'll approve it. Thanks for working on this. > Normal forward scan is only done to compute insn priorities; now, it will also > reset INSN_CONDs for instructions followed by an assignment to their > predicate (for exposed-pipeline targets). After that, sel-sched's dependency > analysis will treat such instructions as if they had no predicate, which is > very conservative. Unfortunately, correctness still may be broken after a > predicate assignment is moved into a different BB (or copied to one as a > bookkeeping copy). I'll try to think of a way to fix it when preparing the > predication patch. It's probably not urgent. We can't really use sel-sched on c6x, as I'd have to add the backtracking machinery to sel-sched for scheduling delay slots, and I don't understand it well enough to do so. Bernd
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c index 3f22a3c..8a39d80 100644 --- a/gcc/sel-sched.c +++ b/gcc/sel-sched.c @@ -6675,7 +6675,7 @@ move_op (insn_t insn, av_set_t orig_ops, expr_t expr_vliw, { struct moveop_static_params sparams; struct cmpd_local_params lparams; - bool res; + int res; /* Init params for code_motion_path_driver. */ sparams.dest = dest; @@ -6694,6 +6694,8 @@ move_op (insn_t insn, av_set_t orig_ops, expr_t expr_vliw, code_motion_path_driver_info = &move_op_hooks; res = code_motion_path_driver (insn, orig_ops, NULL, &lparams, &sparams); + gcc_assert (res != -1); + if (sparams.was_renamed) EXPR_WAS_RENAMED (expr_vliw) = true; @@ -7269,6 +7271,7 @@ sel_region_finish (bool reset_sched_cycles_p) finish_deps_global (); sched_finish_luids (); + VEC_free (haifa_deps_insn_data_def, heap, h_d_i_d); sel_finish_bbs (); BITMAP_FREE (blocks_to_reschedule);