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: FW: [PATCH] [MIPS] microMIPS gcc support



> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Tuesday, February 19, 2013 1:25 PM
> To: Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej
> Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support
> 
> Thanks, this is looking much better now.
> 
> 
> > Index: gcc.target/mips/umips-constraints-1.c
> >
> ==========================================================
> =========
> > --- gcc.target/mips/umips-constraints-1.c	(revision 0)
> > +++ gcc.target/mips/umips-constraints-1.c	(revision 0)
> > @@ -0,0 +1,14 @@
> > +/* { dg-options "(-mmicromips)" } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > +
> > +MICROMIPS void
> > +foo (int *x)
> > +{
> > +  asm volatile ("insn1\t%a0" :: "ZD" (&x[0]));
> > +  asm volatile ("insn2\t%a0" :: "ZD" (&x[511]));
> > +  asm volatile ("insn3\t%a0" :: "ZD" (&x[512])); }
> > +
> > +/* { dg-final { scan-assembler "\tinsn1\t0\\(" } } */
> > +/* { dg-final { scan-assembler "\tinsn2\t2044\\(" } } */
> > +/* { dg-final { scan-assembler "\tinsn3\t2048\\(" } } */
> 
> But the insn3 is wrong, isn't it?  I suggested scan-assembler-not for that one
> because I thought it had to be a 12-bit signed offset on microMIPS.  The
> patch has:
> 
Yes, it was wrong.  Now fixed.

> 
> Sorry, only just realised you were skipping -O1.  Please add a comment saying
> why it fails at -O1 or (preferably) add whichever other option is needed for
> the test to pass at -O1.  I assume it's -fpeephole2 in this case.
> 
> Same for all tests with this skip.

-fpeephole2 did the trick.

> 
> > Index: gcc.target/mips/umips-lwp-swp-2.c
> >
> ==========================================================
> =========
> > --- gcc.target/mips/umips-lwp-swp-2.c	(revision 0)
> > +++ gcc.target/mips/umips-lwp-swp-2.c	(revision 0)
> > @@ -0,0 +1,19 @@
> > +/* { dg-options "-mgp32 (-mmicromips)" } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" "-O1" "-Os" } {
> > +"" } } */ int a[2];
> > +
> > +MICROMIPS f (b)
> > +{
> > +  unsigned int i;
> > +  int *p;
> > +  for (p = &a[b], i = b; --i < ~0; )
> > +    *--p = i * 3 + (int)a;
> > +
> > +}
> > +
> > +MICROMIPS main ()
> > +{
> > +  a[0] = a[1] = 0;
> > +  f (2);
> > +}
> > +/* { dg-final { scan-assembler "\tswp\t\\\$3,0\\(\\\$3\\)" } }*/
> 
> Is this a test of the swap_p case?  Thanks if so, but could you add a comment
> saying where the SWP gets generated, and why the test needs to be skipped
> at -O1 and -Os?
> 
Yes,  this was the swap_p case.  But it turns out that umips-lpw-swp-1.c was also testing the swap_p case.  I've now dropped this test in favor of a different test that exercises the swap_p = false case.
You had mentioned testing for explicit registers.  This is turning out to be problematic.  The Linux and ELF tool chains don't always use the same register set.  In addition, -O3 seems to expose more opportunities for generating these instructions so the code generated isn't consistent among optimization options.  I've changed it back to just testing for swp, but I'm open to other suggestions.  

> 
> Looks good otherwise, but please post an updated patch.  It's probably
> stating the obvious, but the patch is too invasive for this late stage of 4.8, so
> it'll need to wait until 4.9.
> 

The updated patch is attached.  How's it looking this time?

Thanks,
Catherine

Attachment: gcc.cl
Description: gcc.cl

Attachment: gcc.patch
Description: gcc.patch

Attachment: libgcc.cl
Description: libgcc.cl

Attachment: libgcc.patch
Description: libgcc.patch

Attachment: gcc-testsuite.cl
Description: gcc-testsuite.cl

Attachment: gcc-testsuite.patch
Description: gcc-testsuite.patch


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