Patchwork [RFA,PR44618,PowerPC] Wrong code for -frename-registers

login
register
mail settings
Submitter edmar
Date May 23, 2011, 9:53 p.m.
Message ID <4DDAD75E.8030105@freescale.com>
Download mbox | patch
Permalink /patch/97094/
State New
Headers show

Comments

edmar - May 23, 2011, 9:53 p.m.
I completed re-testing everything.
It turns out I cannot reproduce the original error on gcc-4.4 (rev 173968)
So, I am submitting only the patch that I tested for gcc-4.5/4.6/4.7

Regression tested for e500mc target on:
4.5: Revision: 173928
4.6: Revision: 173936
trunk: Revision: 173966

The patch gcc.fix_rnreg4 applies directly to 4.6, 4.7 (1 line offset),
and 4.5 (-632 lines offset)

Thanks,
Edmar
2011-05-23  Edmar Wienskoski  edmar@freescale.com

	* gcc.target/powerpc/outofline_rnreg.c: New testcase.


2011-05-23  Edmar Wienskoski  edmar@freescale.com

	* rs6000.md (save_gpregs_<mode>): Replaced pattern with a set
	of similar patterns, where the MATCH_OPERAND for the function
	argument is replaced with individual references to hardware
	registers.
	* rs6000.md (save_fpregs_<mode>): Ditto
	* rs6000.md (restore_gpregs_<mode>): Ditto
	* rs6000.md (return_and_restore_gpregs_<mode>): Ditto
	* rs6000.md (return_and_restore_fpregs_<mode>): Ditto
	* rs6000.md (return_and_restore_fpregs_aix_<mode>): Ditto
David Edelsohn - May 25, 2011, 2:43 p.m.
On Mon, May 23, 2011 at 5:53 PM, edmar <edmar@freescale.com> wrote:
> I completed re-testing everything.
> It turns out I cannot reproduce the original error on gcc-4.4 (rev 173968)
> So, I am submitting only the patch that I tested for gcc-4.5/4.6/4.7
>
> Regression tested for e500mc target on:
> 4.5: Revision: 173928
> 4.6: Revision: 173936
> trunk: Revision: 173966
>
> The patch gcc.fix_rnreg4 applies directly to 4.6, 4.7 (1 line offset),
> and 4.5 (-632 lines offset)

Are you re-asking for approval?

The patch is okay.

Thanks, David

P.S. Please include the ChangeLog entry inline in the email message
and attach the patch to the email if it is large.  No tar files.
edmar - June 13, 2011, 3:29 p.m.
David,

I realized I did not "reply to all" when I first got your email.
You may have my email in your spam filter.

The issue is that I don't have WAA, and as Richard Guenther suggestion,
I want to ask if you could to do the commit for Freescale.

Thanks in advance for your patience.

In reference to: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44618

Regards,
Edmar


On 05/25/2011 09:43 AM, David Edelsohn wrote:
> On Mon, May 23, 2011 at 5:53 PM, edmar<edmar@freescale.com>  wrote:
>> I completed re-testing everything.
>> It turns out I cannot reproduce the original error on gcc-4.4 (rev 173968)
>> So, I am submitting only the patch that I tested for gcc-4.5/4.6/4.7
>>
>> Regression tested for e500mc target on:
>> 4.5: Revision: 173928
>> 4.6: Revision: 173936
>> trunk: Revision: 173966
>>
>> The patch gcc.fix_rnreg4 applies directly to 4.6, 4.7 (1 line offset),
>> and 4.5 (-632 lines offset)
> Are you re-asking for approval?
>
> The patch is okay.
>
> Thanks, David
>
> P.S. Please include the ChangeLog entry inline in the email message
> and attach the patch to the email if it is large.  No tar files.
>
> .
>

Patch

Index: gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c	(revision 0)
@@ -0,0 +1,15 @@ 
+/* Test that registers used by out of line restore functions does not get renamed.
+   AIX, and 64 bit targets uses r1, which rnreg stays away from.
+   Linux 32 bits targets uses r11, which is susceptible to be renamed */
+/* { dg-do compile } */
+/* { dg-require-effective-target ilp32 } */
+/* { dg-options "-Os -frename-registers -fdump-rtl-rnreg" } */
+/* "* renamed" or "* no available better choice" results are not acceptable */
+/* { dg-final { scan-rtl-dump-not "Register 11 in insn *" "rnreg" { target powerpc*-*-linux* } } } */
+/* { dg-final { cleanup-rtl-dump "rnreg" } } */
+int
+calc (int j)
+{
+  if (j<=1) return 1;
+  return calc(j-1)*(j+1);
+}
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 173936)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -15888,30 +15888,93 @@ 
   "{stm|stmw} %2,%1"
   [(set_attr "type" "store_ux")])
 
-(define_insn "*save_gpregs_<mode>"
+; The following comment applies to:
+;     save_gpregs_*
+;     save_fpregs_*
+;     restore_gpregs*
+;     return_and_restore_gpregs*
+;     return_and_restore_fpregs*
+;     return_and_restore_fpregs_aix*
+;
+; The out-of-line save / restore functions expects one input argument.
+; Since those are not standard call_insn's, we must avoid using
+; MATCH_OPERAND for that argument. That way the register rename
+; optimization will not try to rename this register.
+; Each pattern is repeated for each possible register number used in 
+; various ABIs (r11, r1, and for some functions r12)
+
+(define_insn "*save_gpregs_<mode>_r11"
   [(match_parallel 0 "any_parallel_operand"
 		   [(clobber (reg:P 65))
 		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
-                    (use (match_operand:P 2 "gpc_reg_operand" "r"))
-		    (set (match_operand:P 3 "memory_operand" "=m")
-			 (match_operand:P 4 "gpc_reg_operand" "r"))])]
+                    (use (reg:P 11))
+		    (set (match_operand:P 2 "memory_operand" "=m")
+			 (match_operand:P 3 "gpc_reg_operand" "r"))])]
   ""
   "bl %1"
   [(set_attr "type" "branch")
    (set_attr "length" "4")])
 
-(define_insn "*save_fpregs_<mode>"
+(define_insn "*save_gpregs_<mode>_r12"
   [(match_parallel 0 "any_parallel_operand"
 		   [(clobber (reg:P 65))
 		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
-                    (use (match_operand:P 2 "gpc_reg_operand" "r"))
-		    (set (match_operand:DF 3 "memory_operand" "=m")
-			 (match_operand:DF 4 "gpc_reg_operand" "d"))])]
+                    (use (reg:P 12))
+		    (set (match_operand:P 2 "memory_operand" "=m")
+			 (match_operand:P 3 "gpc_reg_operand" "r"))])]
   ""
   "bl %1"
   [(set_attr "type" "branch")
    (set_attr "length" "4")])
 
+(define_insn "*save_gpregs_<mode>_r1"
+  [(match_parallel 0 "any_parallel_operand"
+		   [(clobber (reg:P 65))
+		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
+                    (use (reg:P 1))
+		    (set (match_operand:P 2 "memory_operand" "=m")
+			 (match_operand:P 3 "gpc_reg_operand" "r"))])]
+  ""
+  "bl %1"
+  [(set_attr "type" "branch")
+   (set_attr "length" "4")])
+
+(define_insn "*save_fpregs_<mode>_r11"
+  [(match_parallel 0 "any_parallel_operand"
+		   [(clobber (reg:P 65))
+		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
+                    (use (reg:P 11))
+		    (set (match_operand:DF 2 "memory_operand" "=m")
+			 (match_operand:DF 3 "gpc_reg_operand" "d"))])]
+  ""
+  "bl %1"
+  [(set_attr "type" "branch")
+   (set_attr "length" "4")])
+
+(define_insn "*save_fpregs_<mode>_r12"
+  [(match_parallel 0 "any_parallel_operand"
+		   [(clobber (reg:P 65))
+		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
+                    (use (reg:P 12))
+		    (set (match_operand:DF 2 "memory_operand" "=m")
+			 (match_operand:DF 3 "gpc_reg_operand" "d"))])]
+  ""
+  "bl %1"
+  [(set_attr "type" "branch")
+   (set_attr "length" "4")])
+
+(define_insn "*save_fpregs_<mode>_r1"
+  [(match_parallel 0 "any_parallel_operand"
+		   [(clobber (reg:P 65))
+		    (use (match_operand:P 1 "symbol_ref_operand" "s"))
+                    (use (reg:P 1))
+		    (set (match_operand:DF 2 "memory_operand" "=m")
+			 (match_operand:DF 3 "gpc_reg_operand" "d"))])]
+  ""
+  "bl %1"
+  [(set_attr "type" "branch")
+   (set_attr "length" "4")])
+
 ; These are to explain that changes to the stack pointer should
 ; not be moved over stores to stack memory.
 (define_insn "stack_tie"
@@ -16004,57 +16067,161 @@ 
 ; FIXME: This would probably be somewhat simpler if the Cygnus sibcall
 ; stuff was in GCC.  Oh, and "any_parallel_operand" is a bit flexible...
 
-(define_insn "*restore_gpregs_<mode>"
+; The following comment applies to:
+;     save_gpregs_*
+;     save_fpregs_*
+;     restore_gpregs*
+;     return_and_restore_gpregs*
+;     return_and_restore_fpregs*
+;     return_and_restore_fpregs_aix*
+;
+; The out-of-line save / restore functions expects one input argument.
+; Since those are not standard call_insn's, we must avoid using
+; MATCH_OPERAND for that argument. That way the register rename
+; optimization will not try to rename this register.
+; Each pattern is repeated for each possible register number used in 
+; various ABIs (r11, r1, and for some functions r12)
+
+(define_insn "*restore_gpregs_<mode>_r11"
  [(match_parallel 0 "any_parallel_operand"
                   [(clobber (match_operand:P 1 "register_operand" "=l"))
                    (use (match_operand:P 2 "symbol_ref_operand" "s"))
-                   (use (match_operand:P 3 "gpc_reg_operand" "r"))
-		   (set (match_operand:P 4 "gpc_reg_operand" "=r")
-			(match_operand:P 5 "memory_operand" "m"))])]
+                   (use (reg:P 11))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
  ""
  "bl %2"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
-(define_insn "*return_and_restore_gpregs_<mode>"
+(define_insn "*restore_gpregs_<mode>_r12"
  [(match_parallel 0 "any_parallel_operand"
+                  [(clobber (match_operand:P 1 "register_operand" "=l"))
+                   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 12))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
+ ""
+ "bl %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*restore_gpregs_<mode>_r1"
+ [(match_parallel 0 "any_parallel_operand"
+                  [(clobber (match_operand:P 1 "register_operand" "=l"))
+                   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 1))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
+ ""
+ "bl %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*return_and_restore_gpregs_<mode>_r11"
+ [(match_parallel 0 "any_parallel_operand"
                   [(return)
 		   (clobber (match_operand:P 1 "register_operand" "=l"))
 		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-                   (use (match_operand:P 3 "gpc_reg_operand" "r"))
-		   (set (match_operand:P 4 "gpc_reg_operand" "=r")
-			(match_operand:P 5 "memory_operand" "m"))])]
+                   (use (reg:P 11))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
  ""
  "b %2"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
-(define_insn "*return_and_restore_fpregs_<mode>"
+(define_insn "*return_and_restore_gpregs_<mode>_r12"
  [(match_parallel 0 "any_parallel_operand"
                   [(return)
 		   (clobber (match_operand:P 1 "register_operand" "=l"))
 		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-                   (use (match_operand:P 3 "gpc_reg_operand" "r"))
-		   (set (match_operand:DF 4 "gpc_reg_operand" "=d")
-			(match_operand:DF 5 "memory_operand" "m"))])]
+                   (use (reg:P 12))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
  ""
  "b %2"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
-(define_insn "*return_and_restore_fpregs_aix_<mode>"
+(define_insn "*return_and_restore_gpregs_<mode>_r1"
  [(match_parallel 0 "any_parallel_operand"
+                  [(return)
+		   (clobber (match_operand:P 1 "register_operand" "=l"))
+		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 1))
+		   (set (match_operand:P 3 "gpc_reg_operand" "=r")
+			(match_operand:P 4 "memory_operand" "m"))])]
+ ""
+ "b %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*return_and_restore_fpregs_<mode>_r11"
+ [(match_parallel 0 "any_parallel_operand"
+                  [(return)
+		   (clobber (match_operand:P 1 "register_operand" "=l"))
+		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 11))
+		   (set (match_operand:DF 3 "gpc_reg_operand" "=d")
+			(match_operand:DF 4 "memory_operand" "m"))])]
+ ""
+ "b %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*return_and_restore_fpregs_<mode>_r12"
+ [(match_parallel 0 "any_parallel_operand"
+                  [(return)
+		   (clobber (match_operand:P 1 "register_operand" "=l"))
+		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 12))
+		   (set (match_operand:DF 3 "gpc_reg_operand" "=d")
+			(match_operand:DF 4 "memory_operand" "m"))])]
+ ""
+ "b %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*return_and_restore_fpregs_<mode>_r1"
+ [(match_parallel 0 "any_parallel_operand"
+                  [(return)
+		   (clobber (match_operand:P 1 "register_operand" "=l"))
+		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+                   (use (reg:P 1))
+		   (set (match_operand:DF 3 "gpc_reg_operand" "=d")
+			(match_operand:DF 4 "memory_operand" "m"))])]
+ ""
+ "b %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
+(define_insn "*return_and_restore_fpregs_aix_<mode>_r11"
+ [(match_parallel 0 "any_parallel_operand"
 		  [(return)
 		   (use (match_operand:P 1 "register_operand" "l"))
 		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-		   (use (match_operand:P 3 "gpc_reg_operand" "r"))
-		   (set (match_operand:DF 4 "gpc_reg_operand" "=d")
-			(match_operand:DF 5 "memory_operand" "m"))])]
+		   (use (reg:P 11))
+		   (set (match_operand:DF 3 "gpc_reg_operand" "=d")
+			(match_operand:DF 4 "memory_operand" "m"))])]
  ""
  "b %2"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
+(define_insn "*return_and_restore_fpregs_aix_<mode>_r1"
+ [(match_parallel 0 "any_parallel_operand"
+		  [(return)
+		   (use (match_operand:P 1 "register_operand" "l"))
+		   (use (match_operand:P 2 "symbol_ref_operand" "s"))
+		   (use (reg:P 1))
+		   (set (match_operand:DF 3 "gpc_reg_operand" "=d")
+			(match_operand:DF 4 "memory_operand" "m"))])]
+ ""
+ "b %2"
+ [(set_attr "type" "branch")
+  (set_attr "length" "4")])
+
 ; This is used in compiling the unwind routines.
 (define_expand "eh_return"
   [(use (match_operand 0 "general_operand" ""))]