| Message ID | 6D39441BF12EF246A7ABCE6654B0235320EED6ED@LEMAIL01.le.imgtec.org |
|---|---|
| State | New |
| Headers | show |
Ping! > -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] > On Behalf Of Matthew Fortune > Sent: 18 August 2014 14:25 > To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org) > Cc: Richard Sandiford; Eric Christopher (echristo@gmail.com) > Subject: [PATCH][MIPS] Do not reload unallocated FP_REGS pseudos via GR_REGS > > The secondary_reload_class hook gets called for pseudos which have > not been allocated a hard register. For pseudos with the FP_REGS > class this results in the hook stating that the pseudo must be > reloaded via GR_REGS as it is neither in an FP_REG nor in memory. > This is obviously wasteful as LRA will go on to allocate an FP_REG > and a GR_REG along with a move and load/store as required. > > An example code sequence which is seen prior to the patch is: > > lw $2, xxx > lw $3, xxx > mtc1 $2, $f0 > mtc1 $3, $f1 > > and is replaced by: > > ldc1 $f0, xxx > > Similarly for stores and 4-byte values. Assembly output from > compare-all-tests shows that this is pretty much the only change > to generated code. There were 3 times as many lw/sw/mtc1/mfc1 > instructions removed as compared to [ls][wd]c1 instructions > added. There was also some noise from different register > allocation but overall instruction counts remain very similar: > > Added fp load store: 13631 > Removed integer load/store/move: 41504 > Added other: 14431 > Removed other: 13244 > > This patch split out from: > https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01815.html > > OK to commit? > > thanks, > Matthew > > gcc/ > * config/mips/mips.c (mips_secondary_reload_class): Handle > regno < 0 case. > --- > gcc/config/mips/mips.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c > index d8654c4..9bee4e6 100644 > --- a/gcc/config/mips/mips.c > +++ b/gcc/config/mips/mips.c > @@ -12134,8 +12134,9 @@ mips_secondary_reload_class (enum reg_class rclass, > > if (reg_class_subset_p (rclass, FP_REGS)) > { > - if (MEM_P (x) > - && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8)) > + if (regno < 0 > + || (MEM_P (x) > + && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8))) > /* In this case we can use lwc1, swc1, ldc1 or sdc1. We'll use > pairs of lwc1s and swc1s if ldc1 and sdc1 are not supported. */ > return NO_REGS; > -- > 1.9.4
Matthew Fortune <Matthew.Fortune@imgtec.com> writes: > Ping! For the record, I agree this is the right fix. Thanks, Richard >> -----Original Message----- >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] >> On Behalf Of Matthew Fortune >> Sent: 18 August 2014 14:25 >> To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org) >> Cc: Richard Sandiford; Eric Christopher (echristo@gmail.com) >> Subject: [PATCH][MIPS] Do not reload unallocated FP_REGS pseudos via GR_REGS >> >> The secondary_reload_class hook gets called for pseudos which have >> not been allocated a hard register. For pseudos with the FP_REGS >> class this results in the hook stating that the pseudo must be >> reloaded via GR_REGS as it is neither in an FP_REG nor in memory. >> This is obviously wasteful as LRA will go on to allocate an FP_REG >> and a GR_REG along with a move and load/store as required. >> >> An example code sequence which is seen prior to the patch is: >> >> lw $2, xxx >> lw $3, xxx >> mtc1 $2, $f0 >> mtc1 $3, $f1 >> >> and is replaced by: >> >> ldc1 $f0, xxx >> >> Similarly for stores and 4-byte values. Assembly output from >> compare-all-tests shows that this is pretty much the only change >> to generated code. There were 3 times as many lw/sw/mtc1/mfc1 >> instructions removed as compared to [ls][wd]c1 instructions >> added. There was also some noise from different register >> allocation but overall instruction counts remain very similar: >> >> Added fp load store: 13631 >> Removed integer load/store/move: 41504 >> Added other: 14431 >> Removed other: 13244 >> >> This patch split out from: >> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01815.html >> >> OK to commit? >> >> thanks, >> Matthew >> >> gcc/ >> * config/mips/mips.c (mips_secondary_reload_class): Handle >> regno < 0 case. >> --- >> gcc/config/mips/mips.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c >> index d8654c4..9bee4e6 100644 >> --- a/gcc/config/mips/mips.c >> +++ b/gcc/config/mips/mips.c >> @@ -12134,8 +12134,9 @@ mips_secondary_reload_class (enum reg_class rclass, >> >> if (reg_class_subset_p (rclass, FP_REGS)) >> { >> - if (MEM_P (x) >> - && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8)) >> + if (regno < 0 >> + || (MEM_P (x) >> + && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8))) >> /* In this case we can use lwc1, swc1, ldc1 or sdc1. We'll use >> pairs of lwc1s and swc1s if ldc1 and sdc1 are not supported. */ >> return NO_REGS; >> -- >> 1.9.4
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index d8654c4..9bee4e6 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -12134,8 +12134,9 @@ mips_secondary_reload_class (enum reg_class rclass, if (reg_class_subset_p (rclass, FP_REGS)) { - if (MEM_P (x) - && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8)) + if (regno < 0 + || (MEM_P (x) + && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8))) /* In this case we can use lwc1, swc1, ldc1 or sdc1. We'll use pairs of lwc1s and swc1s if ldc1 and sdc1 are not supported. */ return NO_REGS;