Patchwork Initialize INSN_COND

login
register
mail settings
Submitter Alexander Monakov
Date June 7, 2011, 5:39 p.m.
Message ID <alpine.LNX.2.00.1106072117570.6250@monoid.intra.ispras.ru>
Download mbox | patch
Permalink /patch/99313/
State New
Headers show

Comments

Alexander Monakov - June 7, 2011, 5:39 p.m.
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?  I also verified that
HPUX failure mentioned by Steve is fixed on a cross-compiler.

> >> How does it use this machinery? If it's not doing a normal
> >> forward scan as in sched_analyze, the INSN_COND mechanism may break in
> >> other ways.

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.


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.
Gary Funck - June 7, 2011, 6:09 p.m.
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?
Alexander Monakov - June 7, 2011, 6:53 p.m.
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
Bernd Schmidt - June 7, 2011, 8:21 p.m.
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

Patch

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);