| Message ID | 20190928095200.GA20382@SDF.ORG |
|---|---|
| State | New |
| Headers | show |
| Series | [ping,PR,target/85401] initialize the move cost table before using it | expand |
On 2019-09-28 5:52 a.m., coypu@sdf.org wrote: > re-posting, now CC'ing vmakarov who might be the right person to ping > about issues in this file. apologies for the noise if I'm wrong. > > -- > This seems to be the way the rest of ira-color.c does it. > I hope it's OK. It does fix the segfault. Yes, the patch is mostly ok. You can commit it into the trunk after applying changes mentioned below. If you do not have a write access, let me know I'll commit the patch by myself. > 2019-09-10 Maya Rashish <coypu@sdf.org> > > PR target/85401 > * ira-color.c: (allocno_copy_cost_saving) Call > ira_init_register_move_cost_if_necessary Wrong format for ChangeLog. Wrong place for the colon and missed period. It should be PR target/85401 * ira-color.c (allocno_copy_cost_saving): Call ira_init_register_move_cost_if_necessary. > > diff --git a/gcc/ira-color.c b/gcc/ira-color.c > index 99236994d64..5d721102e19 100644 > --- a/gcc/ira-color.c > +++ b/gcc/ira-color.c > @@ -2828,6 +2828,7 @@ allocno_copy_cost_saving (ira_allocno_t allocno, int hard_regno) > } > else > gcc_unreachable (); > + ira_init_register_move_cost_if_necessary(allocno_mode); Blank should be before the open parenthesis. > cost += cp->freq * ira_register_move_cost[allocno_mode][rclass][rclass]; > } > return cost; > > It would be nice to add a small test too. But it is not obligatory for this case as the patch is obvious and it might be hard to create a small test to reproduce the bug.
On Mon, Sep 30, 2019 at 11:46:24AM -0400, Vladimir Makarov wrote: > Yes, the patch is mostly ok. You can commit it into the trunk after > applying changes mentioned below. If you do not have a write access, let me > know I'll commit the patch by myself. I don't have commit access. It would be nice if you committed it :) > It would be nice to add a small test too. But it is not obligatory for this > case as the patch is obvious and it might be hard to create a small test to > reproduce the bug. I have the C code that produces this failure. I can creduce it, but I'm not sure there's a relationship between it and the bug. Doing unrelated changes (adding instruction scheduling) to vax also hid it. Is this kind of test still valuable? Thanks.
On 9/30/19 2:45 PM, coypu@sdf.org wrote: > On Mon, Sep 30, 2019 at 11:46:24AM -0400, Vladimir Makarov wrote: >> Yes, the patch is mostly ok. You can commit it into the trunk after >> applying changes mentioned below. If you do not have a write access, let me >> know I'll commit the patch by myself. > > I don't have commit access. It would be nice if you committed it :) I took care of the nits and committed the patch. > >> It would be nice to add a small test too. But it is not obligatory for this >> case as the patch is obvious and it might be hard to create a small test to >> reproduce the bug. > > I have the C code that produces this failure. I can creduce it, but I'm > not sure there's a relationship between it and the bug. > Doing unrelated changes (adding instruction scheduling) to vax also hid it. > > Is this kind of test still valuable? Often they are. jeff
diff --git a/gcc/ira-color.c b/gcc/ira-color.c index 99236994d64..5d721102e19 100644 --- a/gcc/ira-color.c +++ b/gcc/ira-color.c @@ -2828,6 +2828,7 @@ allocno_copy_cost_saving (ira_allocno_t allocno, int hard_regno) } else gcc_unreachable (); + ira_init_register_move_cost_if_necessary(allocno_mode); cost += cp->freq * ira_register_move_cost[allocno_mode][rclass][rclass]; } return cost;