| Message ID | 001501d11bbc$085031d0$18f09570$@arm.com |
|---|---|
| State | New |
| Headers | show |
On 11/10/2015 08:30 AM, Wilco Dijkstra wrote: > Ping of https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00829.html: > > > > This fixes a bug in register preferencing. When live range splitting creates > a new register from > another, it copies most fields except for the register preferences. The > preference GENERAL_REGS is > used as reg_pref[i].prefclass is initialized with GENERAL_REGS in > allocate_reg_info () and > resize_reg_info (). > > This initialization value is not innocuous like the comment suggests - if a > new register has a > non-integer mode, it is forced to prefer GENERAL_REGS. This changes the > register costs in pass 2 so > that they are incorrect. As a result the liverange is either spilled or > allocated to an integer > register: > > void g(double); > void f(double x) > { > if (x == 0) > return; > g (x); > g (x); > } > > f: > fcmp d0, #0.0 > bne .L6 > ret > .L6: > stp x19, x30, [sp, -16]! > fmov x19, d0 > bl g > fmov d0, x19 > ldp x19, x30, [sp], 16 > b g > > With the fix it uses a floating point register as expected. Given a similar > issue in > https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02253.html, would it not be > better to change the > initialization values of reg_pref to illegal register classes so this kind > of issue can be trivially > found with an assert? Also would it not be a good idea to have a single > register copy function that > ensures all data is copied? Having a function and the assert would be wonderful. If you have a patch for this, I'll be glad to review it. If you don't have a patch or have no time or willing to work on it, you can commit given here patch into the trunk. Thanks. > > ChangeLog: 2014-12-09 Wilco Dijkstra wdijkstr@arm.com > > * gcc/ira-emit.c (ira_create_new_reg): Copy preference classes. > > --- > gcc/ira-emit.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gcc/ira-emit.c b/gcc/ira-emit.c > index d246b7f..d736836 100644 > --- a/gcc/ira-emit.c > +++ b/gcc/ira-emit.c > @@ -348,6 +348,7 @@ rtx > ira_create_new_reg (rtx original_reg) > { > rtx new_reg; > + int original_regno = REGNO (original_reg); > > new_reg = gen_reg_rtx (GET_MODE (original_reg)); > ORIGINAL_REGNO (new_reg) = ORIGINAL_REGNO (original_reg); > @@ -356,8 +357,16 @@ ira_create_new_reg (rtx original_reg) > REG_ATTRS (new_reg) = REG_ATTRS (original_reg); > if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL) > fprintf (ira_dump_file, " Creating newreg=%i from oldreg=%i\n", > - REGNO (new_reg), REGNO (original_reg)); > + REGNO (new_reg), original_regno); > ira_expand_reg_equiv (); > + > + /* Copy the preference classes to new_reg. */ > + resize_reg_info (); > + setup_reg_classes (REGNO (new_reg), > + reg_preferred_class (original_regno), > + reg_alternate_class (original_regno), > + reg_allocno_class (original_regno)); > + > return new_reg; > } >
diff --git a/gcc/ira-emit.c b/gcc/ira-emit.c index d246b7f..d736836 100644 --- a/gcc/ira-emit.c +++ b/gcc/ira-emit.c @@ -348,6 +348,7 @@ rtx ira_create_new_reg (rtx original_reg) { rtx new_reg; + int original_regno = REGNO (original_reg); new_reg = gen_reg_rtx (GET_MODE (original_reg)); ORIGINAL_REGNO (new_reg) = ORIGINAL_REGNO (original_reg); @@ -356,8 +357,16 @@ ira_create_new_reg (rtx original_reg) REG_ATTRS (new_reg) = REG_ATTRS (original_reg); if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL) fprintf (ira_dump_file, " Creating newreg=%i from oldreg=%i\n", - REGNO (new_reg), REGNO (original_reg)); + REGNO (new_reg), original_regno); ira_expand_reg_equiv (); + + /* Copy the preference classes to new_reg. */ + resize_reg_info (); + setup_reg_classes (REGNO (new_reg), + reg_preferred_class (original_regno), + reg_alternate_class (original_regno), + reg_allocno_class (original_regno)); + return new_reg; }
Ping of https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00829.html: This fixes a bug in register preferencing. When live range splitting creates a new register from another, it copies most fields except for the register preferences. The preference GENERAL_REGS is used as reg_pref[i].prefclass is initialized with GENERAL_REGS in allocate_reg_info () and resize_reg_info (). This initialization value is not innocuous like the comment suggests - if a new register has a non-integer mode, it is forced to prefer GENERAL_REGS. This changes the register costs in pass 2 so that they are incorrect. As a result the liverange is either spilled or allocated to an integer register: void g(double); void f(double x) { if (x == 0) return; g (x); g (x); } f: fcmp d0, #0.0 bne .L6 ret .L6: stp x19, x30, [sp, -16]! fmov x19, d0 bl g fmov d0, x19 ldp x19, x30, [sp], 16 b g With the fix it uses a floating point register as expected. Given a similar issue in https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02253.html, would it not be better to change the initialization values of reg_pref to illegal register classes so this kind of issue can be trivially found with an assert? Also would it not be a good idea to have a single register copy function that ensures all data is copied? ChangeLog: 2014-12-09 Wilco Dijkstra wdijkstr@arm.com * gcc/ira-emit.c (ira_create_new_reg): Copy preference classes. --- gcc/ira-emit.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)