This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, mips] delete bit-rotten ADJUST_REG_ALLOC_ORDER definition


On 05/13/14 14:11, Sandra Loosemore wrote:
This patch is a follow-up to this thread from a few years ago:

https://gcc.gnu.org/ml/gcc/2011-01/msg00093.html
https://gcc.gnu.org/ml/gcc/2011-01/msg00158.html

As noted there, the current definition of ADJUST_REG_ALLOC_ORDER is
obsolete:

(1) This hook is a holdover from the old pre-IRA register allocator and
it's not clear it does anything useful with IRA.

(2) It's inconsistent with the current REG_ALLOC_ORDER definition which
is not just {0, 1, 2, ...} any more.

I considered re-working the hook to jiggle the $24 ordering for MIPS16
relative to the current REG_ALLOC_ORDER definition, but it wasn't clear
to me either where in the ordering it should go, or that it would be
worthwhile to try to tune this.  Indeed, the CSiBE results for removing
it entirely are pretty much in the noise range, except that removing the
hack that is supposed to benefit MIPS16 resulted in more improvement for
that case than any of the others tested!  Here are some numbers
comparing geomean for old and new with -Os for various combinations:

default: 1062.3,1062.28
-EL -mips16 -msoft-float: 758.624,758.551
-EL -mips16: 763.398,763.321
-EL -msoft-float: 1115.11,1115.09
-EL: 1062.22,1062.2
-EL -mabi=64: 1181.8,1181.81
-mabi=64: 1181.93,1181.94
-mips16 -msoft-float: 758.337,758.236
-mips16: 763.11,763.011
-EL -mabi=64 -msoft-float: 1218.11,1218.11
-mabi=64 -msoft-float: 1218.24,1218.25
-msoft-float: 1115.73,1115.71
-EL -mmicromips -msoft-float: 792.314,792.32
-EL -mnan=2008: 1062.22,1062.2
-mnan=2008: 1062.3,1062.28

As for the invalid JALR patch I posted just now
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01015.html
I had trouble regression-testing this patch on trunk due to various
other breakage in the past week, and ended up doing it on a 4.9.0
checkout modified to support Mentor's extended set of mips-sde-elf
multilibs instead.  That's also the source of the CSiBE numbers above.
Also, those numbers do include the JALR patch in the baseline.

This patch did fix some of the dspr2-MULT.c and dspr2-MULTU.c test
failures that were reported in the original discussion, but I believe
the current XFAILs for those tests are still valid for the same reasons
listed in PR target/51729.

So, this patch should be considered more of a code cleanup thing, than
either an optimization improvement or a test regression fix.  OK to commit?

-Sandra


2014-05-13  Catherine Moore  <clm@codesourcery.com>
         Sandra Loosemore  <sandra@codesourcery.com>

     gcc/
     * config/mips/mips.c (mips_order_regs_for_local_alloc): Delete.
     * config/mips/mips.h (ADJUST_REG_ALLOC_ORDER): Delete.
     * config/mips/mips-protos.h (mips_order_regs_for_local_alloc): Delete.
OK for the trunk.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]