On Tue, Dec 15, 2015 at 10:54:49AM +0000, Wilco Dijkstra wrote:
> ping
>
> > -----Original Message-----
> > From: Wilco Dijkstra [mailto:wilco.dijks...@arm.com]
> > Sent: 06 November 2015 20:06
> > To: 'gcc-patches@gcc.gnu.org'
> > Subject: [PATCH][AArch64] Add TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> >
> > This patch adds support for the TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> > hook. When the cost of GENERAL_REGS and FP_REGS is identical, the register
> > allocator always uses ALL_REGS even when it has a much higher cost. The
> > hook changes the class to either FP_REGS or GENERAL_REGS depending on the
> > mode of the register. This results in better register allocation overall,
> > fewer spills and reduced codesize - particularly in SPEC2006 gamess.
> >
> > GCC regression passes with several minor fixes.
> >
> > OK for commit?
> >
> > ChangeLog:
> > 2015-11-06 Wilco Dijkstra <wdijk...@arm.com>
> >
> > * gcc/config/aarch64/aarch64.c
> > (TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): New define.
> > (aarch64_ira_change_pseudo_allocno_class): New function.
> > * gcc/testsuite/gcc.target/aarch64/cvtf_1.c: Build with -O2.
> > * gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
> > (test_corners_sisd_di): Improve force to SIMD register.
> > (test_corners_sisd_si): Likewise.
> > * gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c: Build with -O2.
> > * gcc/testsuite/gcc.target/aarch64/vect-ld1r-compile-fp.c:
> > Remove scan-assembler check for ldr.
Drop the gcc/ from the ChangeLog.
> > --
> > gcc/config/aarch64/aarch64.c | 22
> > ++++++++++++++++++++++
> > gcc/testsuite/gcc.target/aarch64/cvtf_1.c | 2 +-
> > gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c | 4 ++--
> > gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c | 2 +-
> > .../gcc.target/aarch64/vect-ld1r-compile-fp.c | 1 -
These testsuite changes concern me a bit, and you don't mention them beyond
saying they are minor fixes...
> > diff --git a/gcc/testsuite/gcc.target/aarch64/cvtf_1.c
> > b/gcc/testsuite/gcc.target/aarch64/cvtf_1.c
> > index 5f2ff81..96501db 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/cvtf_1.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/cvtf_1.c
> > @@ -1,5 +1,5 @@
> > /* { dg-do run } */
> > -/* { dg-options "-save-temps -fno-inline -O1" } */
> > +/* { dg-options "-save-temps -fno-inline -O2" } */
This one says we have a code-gen regression at -O1 ?
> >
> > #define FCVTDEF(ftype,itype) \
> > void \
> > diff --git a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
> > b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
> > index 363f554..8465c89 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
> > @@ -186,9 +186,9 @@ test_corners_sisd_di (Int64x1 b)
> > {
> > force_simd_di (b);
> > b = b >> 63;
> > + force_simd_di (b);
> > b = b >> 0;
> > b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
> > - force_simd_di (b);
This one I don't understand, but seems to say that we've decided to move
b out of FP_REGS after getting it in there for b = b << 63; ? So this is
another register allocator regression?
> > diff --git a/gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c
> > b/gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c
> > index a49db3e..c5a9c52 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/vdup_lane_2.c
> > @@ -1,6 +1,6 @@
> > /* Test vdup_lane intrinsics work correctly. */
> > /* { dg-do run } */
> > -/* { dg-options "-O1 --save-temps" } */
> > +/* { dg-options "-O2 --save-temps" } */
Another -O1 regression ?
> >
> > #include <arm_neon.h>
> >
> > diff --git a/gcc/testsuite/gcc.target/aarch64/vect-ld1r-compile-fp.c
> > b/gcc/testsuite/gcc.target/aarch64/vect-ld1r-compile-fp.c
> > index 66e0168..4711c61 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/vect-ld1r-compile-fp.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/vect-ld1r-compile-fp.c
> > @@ -8,6 +8,5 @@ DEF (float)
> > DEF (double)
> >
> > /* { dg-final { scan-assembler "ld1r\\t\{v\[0-9\]+\.4s"} } */
> > -/* { dg-final { scan-assembler "ldr\\t\x\[0-9\]+"} } */
> > /* { dg-final { scan-assembler "ld1r\\t\{v\[0-9\]+\.2d"} } */
This one is fine, I don't really understand what it was hoping to catch
in the first place!
Could you go in to some detail about why your testsuite changes are correct?
Thanks,
James
