RFA: patch for test PR70669 test

Message ID 68d7c42c-3ebe-532b-6403-63f8975d97d2@redhat.com
State New
Headers show
Series
  • RFA: patch for test PR70669 test
Related show

Commit Message

Vladimir Makarov Nov. 22, 2018, 7:47 p.m.
Today I committed a patch

https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01945.html

   But it makes gcc.target/powerpc/pr70669.c to fail.  Here is the patch 
to fix the failure.  The expected code assumes that variable r should 
get a general reg.  I suspect the expectation is wrong. There are three 
possible choices for r: a general reg, a vsx reg, and memory.

   The cost of move of vsx to/from mem is 4, the cost of move of general 
reg of this mode to/from mem is 8, and the cost of move of general reg 
to/from vsx is 12.

   So approximately if r is assigned memory we have cost 8 * 2 (asm) + 4 
* 2 (plus) = 24 (when we use memory at *q), if r is assigned to vsx the 
cost is 12*2(asm) + 4(r=*q) = 28, and if r is assigned a general reg the 
cost is 12 * 2 (plus) + 8 (r = *q) = 32.

   ira-costs should choose mem although it chose a general reg before my 
patch (it is a big topic why it chose a general reg. In brief, it is 
because some inherent drawbacks of the current cost calculation 
algorithm). The patch I recently submitted solves some drawbacks and 
memory is chosen for r.

  To generate the expected code with r in a general reg, I just decrease 
the number of getting r value into vsx reg by changing r + r onto -r.

Is the following patch ok for trunk?

Comments

Segher Boessenkool Nov. 23, 2018, 12:30 a.m. | #1
Hi!

On Thu, Nov 22, 2018 at 02:47:10PM -0500, Vladimir Makarov wrote:
>   Today I committed a patch

> 

> https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01945.html

> 

>   But it makes gcc.target/powerpc/pr70669.c to fail.  Here is the patch 

> to fix the failure.  The expected code assumes that variable r should 

> get a general reg.  I suspect the expectation is wrong. There are three 

> possible choices for r: a general reg, a vsx reg, and memory.

> 

>   The cost of move of vsx to/from mem is 4, the cost of move of general 

> reg of this mode to/from mem is 8, and the cost of move of general reg 

> to/from vsx is 12.

> 

>   So approximately if r is assigned memory we have cost 8 * 2 (asm) + 4 

> * 2 (plus) = 24 (when we use memory at *q), if r is assigned to vsx the 

> cost is 12*2(asm) + 4(r=*q) = 28, and if r is assigned a general reg the 

> cost is 12 * 2 (plus) + 8 (r = *q) = 32.

> 

>   ira-costs should choose mem although it chose a general reg before my 

> patch (it is a big topic why it chose a general reg. In brief, it is 

> because some inherent drawbacks of the current cost calculation 

> algorithm). The patch I recently submitted solves some drawbacks and 

> memory is chosen for r.


That sounds all correct.  Thanks for fixing this!

>  To generate the expected code with r in a general reg, I just decrease 

> the number of getting r value into vsx reg by changing r + r onto -r.

> 

> Is the following patch ok for trunk?


Yes, the test now again tests what it is meant to test (that you get
m[tf]vsrd from reloads).  It's fine for trunk, thanks!


Segher


2018-11-22  Vladimir Makarov  <vmakarov@redhat.com>

	* gcc.target/powerpc/pr70669.c: Use unary minus instead of
	addition.
Vladimir Makarov Nov. 23, 2018, 9:44 p.m. | #2
On 11/22/2018 07:30 PM, Segher Boessenkool wrote:
> Hi!

>

> On Thu, Nov 22, 2018 at 02:47:10PM -0500, Vladimir Makarov wrote:

>

>>   To generate the expected code with r in a general reg, I just decrease

>> the number of getting r value into vsx reg by changing r + r onto -r.

>>

>> Is the following patch ok for trunk?

> Yes, the test now again tests what it is meant to test (that you get

> m[tf]vsrd from reloads).  It's fine for trunk, thanks!

>

>

Thank you, Segher.  Committed as rev. 266421.

Patch

Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 266318)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2018-11-22  Vladimir Makarov  <vmakarov@redhat.com>
+
+	* gcc.target/powerpc/pr70669.c: Use unary minus instead of
+	addition.
+
 2018-11-20  Jan Hubicka  <hubicka@ucw.cz>
 
 	PR ipa/87706
Index: testsuite/gcc.target/powerpc/pr70669.c
===================================================================
--- testsuite/gcc.target/powerpc/pr70669.c	(revision 266318)
+++ testsuite/gcc.target/powerpc/pr70669.c	(working copy)
@@ -13,7 +13,7 @@  void foo (TYPE *p, TYPE *q)
 #ifndef NO_ASM
   __asm__ (" # %0" : "+r" (r));
 #endif
-  *p = r + r;
+  *p = -r;
 }
 
 /* { dg-final { scan-assembler       "mfvsrd"    } } */