public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: edmar <edmar@freescale.com>
To: David Edelsohn <dje.gcc@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
Date: Tue, 24 May 2011 04:46:00 -0000	[thread overview]
Message-ID: <4DDAD75E.8030105@freescale.com> (raw)
In-Reply-To: <BANLkTim9R5g=VQ-ZPnThi7sAyQqn10=xcQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

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



[-- Attachment #2: gcc.fix_rnreg4 --]
[-- Type: text/plain, Size: 12085 bytes --]

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


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" ""))]

  parent reply	other threads:[~2011-05-23 21:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-20  8:31 David Edelsohn
2011-05-20  9:13 ` edmar
2011-05-24  4:46 ` edmar [this message]
2011-05-25 15:11   ` David Edelsohn
2011-06-13 17:27     ` edmar
  -- strict thread matches above, loose matches on Subject: below --
2011-05-19 18:10 edmar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DDAD75E.8030105@freescale.com \
    --to=edmar@freescale.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).