public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, testsuite, mips] Fix two failing MIPS tests.
@ 2014-05-02 17:39 Steve Ellcey 
  2014-05-03  6:52 ` Richard Sandiford
  0 siblings, 1 reply; 5+ messages in thread
From: Steve Ellcey  @ 2014-05-02 17:39 UTC (permalink / raw)
  To: gcc-patches; +Cc: rdsandiford


Two MIPS specific tests started failing a few days ago, this patch fixes
them.  The tests are trying to check that a performance optimization is
done and that one constant is derived from another and not simply loaded
as a completely new constant.

Post 4.9 changes have affected which constant gets loaded and which one
gets derived (and how it is derived) from it.  The new code looks as
good as the old code:

Old:

 	li	$5,305397760			# 0x12340000
 	addiu	$4,$5,1
 	addiu	$5,$5,-1

New:

	li	$5,305332224			# 0x12330000
 	ori	$5,$5,0xffff
 	addiu	$4,$5,2

My proposed fix is to change the asm scan to verify that there is only 1 'li'
instruction and to not worry about what value is being loaded or what
instructions are used to generate the second constant.

Tested on MIPS32 and MIPS64 (n32 and 64 ABIs).

OK to checkin?

Steve Ellcey
sellcey@mips.com



2014-05-02  Steve Ellcey  <sellcey@mips.com>

	* gcc.target/mips/const-anchor-1.c: Modify asm scan.
	* gcc.target/mips/const-anchor-2.c: Ditto.


diff --git a/gcc/testsuite/gcc.target/mips/const-anchor-1.c b/gcc/testsuite/gcc.target/mips/const-anchor-1.c
index a5f01e4..debab12 100644
--- a/gcc/testsuite/gcc.target/mips/const-anchor-1.c
+++ b/gcc/testsuite/gcc.target/mips/const-anchor-1.c
@@ -1,8 +1,8 @@
-/* Derive a constant (0x1233ffff) from an intermediate value
-   (0x1234000) used to build another constant.  */
+/* Derive one constant from an intermediate value used to build another
+   constant.  Verify that there is only one load immediate instruction
+   generated.  */
 /* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
-/* { dg-final { scan-assembler-not "0x12330000|305332224" } } */
-/* { dg-final { scan-assembler "\td?addiu\t\\\$5,\\\$\[0-9\]*,-1" } } */
+/* { dg-final { scan-assembler-times "\tli\t" 1 } } */
 
 NOMIPS16 void f ()
 {
diff --git a/gcc/testsuite/gcc.target/mips/const-anchor-2.c b/gcc/testsuite/gcc.target/mips/const-anchor-2.c
index 8dad5a7..2edd1ef 100644
--- a/gcc/testsuite/gcc.target/mips/const-anchor-2.c
+++ b/gcc/testsuite/gcc.target/mips/const-anchor-2.c
@@ -1,7 +1,8 @@
-/* Derive a constant (0x30001) from another constant.  */
+/* Derive one constant from an intermediate value used to build another
+   constant.  Verify that there is only one load immediate instruction
+   generated.  */
 /* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
-/* { dg-final { scan-assembler-not "0x300000|196608" } } */
-/* { dg-final { scan-assembler "\td?addiu\t\\\$5,\\\$\[0-9\]*,32763" } } */
+/* { dg-final { scan-assembler-times "\tli\t" 1 } } */
 
 NOMIPS16 void f ()
 {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch, testsuite, mips] Fix two failing MIPS tests.
  2014-05-02 17:39 [Patch, testsuite, mips] Fix two failing MIPS tests Steve Ellcey 
@ 2014-05-03  6:52 ` Richard Sandiford
  2014-05-05 16:17   ` Steve Ellcey
  2014-05-05 19:42   ` Steve Ellcey
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Sandiford @ 2014-05-03  6:52 UTC (permalink / raw)
  To: Steve Ellcey ; +Cc: gcc-patches

"Steve Ellcey " <sellcey@mips.com> writes:
> Two MIPS specific tests started failing a few days ago, this patch fixes
> them.  The tests are trying to check that a performance optimization is
> done and that one constant is derived from another and not simply loaded
> as a completely new constant.
>
> Post 4.9 changes have affected which constant gets loaded and which one
> gets derived (and how it is derived) from it.  The new code looks as
> good as the old code:
>
> Old:
>
>  	li	$5,305397760			# 0x12340000
>  	addiu	$4,$5,1
>  	addiu	$5,$5,-1
>
> New:
>
> 	li	$5,305332224			# 0x12330000
>  	ori	$5,$5,0xffff
>  	addiu	$4,$5,2

This isn't as good though -- $4 now depends on two previous
instructions.  Do you know which specific change was responsible?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch, testsuite, mips] Fix two failing MIPS tests.
  2014-05-03  6:52 ` Richard Sandiford
@ 2014-05-05 16:17   ` Steve Ellcey
  2014-05-05 19:42   ` Steve Ellcey
  1 sibling, 0 replies; 5+ messages in thread
From: Steve Ellcey @ 2014-05-05 16:17 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

On Sat, 2014-05-03 at 07:52 +0100, Richard Sandiford wrote:

> > Old:
> >
> >  	li	$5,305397760			# 0x12340000
> >  	addiu	$4,$5,1
> >  	addiu	$5,$5,-1
> >
> > New:
> >
> > 	li	$5,305332224			# 0x12330000
> >  	ori	$5,$5,0xffff
> >  	addiu	$4,$5,2
> 
> This isn't as good though -- $4 now depends on two previous
> instructions.  Do you know which specific change was responsible?
> 
> Thanks,
> Richard

It was this one:

2014-04-29  James Greenhalgh  <james.greenhalgh@arm.com>

        * calls.c (initialize_argument_information): Always treat
        PUSH_ARGS_REVERSED as 1, simplify code accordingly.
        (expand_call): Likewise.
        (emit_library_call_calue_1): Likewise.
        * expr.c (PUSH_ARGS_REVERSED): Do not define.
        (emit_push_insn): Always treat PUSH_ARGS_REVERSED as 1, simplify
        code accordingly.

Steve Ellcey
sellcey@mips.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch, testsuite, mips] Fix two failing MIPS tests.
  2014-05-03  6:52 ` Richard Sandiford
  2014-05-05 16:17   ` Steve Ellcey
@ 2014-05-05 19:42   ` Steve Ellcey
  2014-05-05 20:44     ` Richard Sandiford
  1 sibling, 1 reply; 5+ messages in thread
From: Steve Ellcey @ 2014-05-05 19:42 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

On Sat, 2014-05-03 at 07:52 +0100, Richard Sandiford wrote:

> > Old:
> >
> >  	li	$5,305397760			# 0x12340000
> >  	addiu	$4,$5,1
> >  	addiu	$5,$5,-1
> >
> > New:
> >
> > 	li	$5,305332224			# 0x12330000
> >  	ori	$5,$5,0xffff
> >  	addiu	$4,$5,2
> 
> This isn't as good though -- $4 now depends on two previous
> instructions.  Do you know which specific change was responsible?
> 
> Thanks,
> Richard

A quick follow up.  The code in question is:

    void f () { g (0x12340001, 0x1233ffff); }

And if I swap the arguments I get something more like the old code.
But if I run an older GCC on that new swapped code then I get something
like the new code.  So while the old code may have been better for this
example, the new code is better if I swap the arguments around.

Steve Ellcey
sellcey@mips.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch, testsuite, mips] Fix two failing MIPS tests.
  2014-05-05 19:42   ` Steve Ellcey
@ 2014-05-05 20:44     ` Richard Sandiford
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Sandiford @ 2014-05-05 20:44 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches

Steve Ellcey <sellcey@mips.com> writes:
> On Sat, 2014-05-03 at 07:52 +0100, Richard Sandiford wrote:
>
>> > Old:
>> >
>> >  	li	$5,305397760			# 0x12340000
>> >  	addiu	$4,$5,1
>> >  	addiu	$5,$5,-1
>> >
>> > New:
>> >
>> > 	li	$5,305332224			# 0x12330000
>> >  	ori	$5,$5,0xffff
>> >  	addiu	$4,$5,2
>> 
>> This isn't as good though -- $4 now depends on two previous
>> instructions.  Do you know which specific change was responsible?
>> 
>> Thanks,
>> Richard
>
> A quick follow up.  The code in question is:
>
>     void f () { g (0x12340001, 0x1233ffff); }
>
> And if I swap the arguments I get something more like the old code.
> But if I run an older GCC on that new swapped code then I get something
> like the new code.  So while the old code may have been better for this
> example, the new code is better if I swap the arguments around.

OK, in that case it sounds like the optimisation wasn't working as well
as it was supposed to.  But I think we should fix that rather than change
the expected output.  AFAICT the new output means the const-anchor
optimisation isn't kicking in.

Thanks,
Richard



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-05-05 20:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-02 17:39 [Patch, testsuite, mips] Fix two failing MIPS tests Steve Ellcey 
2014-05-03  6:52 ` Richard Sandiford
2014-05-05 16:17   ` Steve Ellcey
2014-05-05 19:42   ` Steve Ellcey
2014-05-05 20:44     ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).