From: edmar <edmar@freescale.com>
To: <gcc-patches@gcc.gnu.org>
Subject: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
Date: Thu, 19 May 2011 18:10:00 -0000 [thread overview]
Message-ID: <4DD53BAA.8070603@freescale.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 450 bytes --]
The original patch posted in bugzilla was fully tested for 4.4/4.5/4.6
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44618
I am reposting it here as a reference for review.
More recently, I re-tested it for 4.6, and it still pass with no regressions
on powerpc (32 bits)
If the general approach of the solution is acceptable, I would like to
have it re-tested with the latest in 4.4/4.5/4.6/4.7 and get it committed
in all branches.
Thanks
Edmar
[-- Attachment #2: sub_ppc_rnreg2-Changelog-gcc --]
[-- Type: text/plain, Size: 545 bytes --]
2010-06-21 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 (save_gpregs_<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
[-- Attachment #3: sub_ppc_rnreg2-Changelog-testsuite --]
[-- Type: text/plain, Size: 106 bytes --]
2010-06-21 Edmar Wienskoski edmar@freescale.com
* gcc.target/powerpc/outofline_rnreg.c: New testcase.
[-- Attachment #4: sub_ppc_rnreg2.diff --]
[-- Type: text/x-patch, Size: 11603 bytes --]
Index: gcc-20100617/gcc/config/rs6000/rs6000.md
===================================================================
--- gcc-20100617/gcc/config/rs6000/rs6000.md (revision 161285)
+++ gcc-20100617/gcc/config/rs6000/rs6000.md (working copy)
@@ -15448,30 +15448,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"
@@ -15564,57 +15627,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" ""))]
diff -uN gcc-20100615-orig/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c gcc-20100615/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c
--- gcc-20100615-orig/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c 1969-12-31 18:00:00.000000000 -0600
+++ gcc-20100615/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c 2010-06-15 11:50:13.463426108 -0500
@@ -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);
+}
[-- Attachment #5: sub_ppc_rnreg2-44.diff --]
[-- Type: text/x-patch, Size: 5120 bytes --]
Index: gcc44-20100616/gcc/config/rs6000/rs6000.md
===================================================================
--- gcc44-20100616/gcc/config/rs6000/rs6000.md (revision 160848)
+++ gcc44-20100616/gcc/config/rs6000/rs6000.md (working copy)
@@ -14701,13 +14701,26 @@
"{stm|stmw} %2,%1"
[(set_attr "type" "store_ux")])
+; 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 it.
+
(define_insn "*save_gpregs_<mode>"
[(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 %z1"
[(set_attr "type" "branch")
@@ -14717,9 +14730,9 @@
[(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" "f"))])]
+ (use (reg:P 11))
+ (set (match_operand:DF 2 "memory_operand" "=m")
+ (match_operand:DF 3 "gpc_reg_operand" "f"))])]
""
"bl %z1"
[(set_attr "type" "branch")
@@ -14817,13 +14830,25 @@
; FIXME: This would probably be somewhat simpler if the Cygnus sibcall
; stuff was in GCC. Oh, and "any_parallel_operand" is a bit flexible...
+; The following comment applies to:
+; save_gpregs_*
+; save_fpregs_*
+; restore_gpregs*
+; return_and_restore_gpregs*
+; return_and_restore_fpregs*
+;
+; 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 it.
+
(define_insn "*restore_gpregs_<mode>"
[(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 %z2"
[(set_attr "type" "branch")
@@ -14834,9 +14859,9 @@
[(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 %z2"
[(set_attr "type" "branch")
@@ -14847,9 +14872,9 @@
[(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" "=f")
- (match_operand:DF 5 "memory_operand" "m"))])]
+ (use (reg:P 11))
+ (set (match_operand:DF 3 "gpc_reg_operand" "=f")
+ (match_operand:DF 4 "memory_operand" "m"))])]
""
"b %z2"
[(set_attr "type" "branch")
diff -uN gcc-20100615-orig/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c gcc-20100615/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c
--- gcc-20100615-orig/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c 1969-12-31 18:00:00.000000000 -0600
+++ gcc-20100615/gcc/testsuite/gcc.target/powerpc/outofline_rnreg.c 2010-06-15 11:50:13.463426108 -0500
@@ -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);
+}
next reply other threads:[~2011-05-19 15:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-19 18:10 edmar [this message]
2011-05-20 8:31 David Edelsohn
2011-05-20 9:13 ` edmar
2011-05-24 4:46 ` edmar
2011-05-25 15:11 ` David Edelsohn
2011-06-13 17:27 ` 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=4DD53BAA.8070603@freescale.com \
--to=edmar@freescale.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).