* [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
@ 2011-05-19 18:10 edmar
0 siblings, 0 replies; 6+ messages in thread
From: edmar @ 2011-05-19 18:10 UTC (permalink / raw)
To: gcc-patches
[-- 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);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
@ 2011-05-20 8:31 David Edelsohn
2011-05-20 9:13 ` edmar
2011-05-24 4:46 ` edmar
0 siblings, 2 replies; 6+ messages in thread
From: David Edelsohn @ 2011-05-20 8:31 UTC (permalink / raw)
To: Edmar Wienskoski; +Cc: GCC Patches
010-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
Okay.
Did you intend to include the gcc44 diff?
Also, it's better if you include the ChangeLog inline and the patch as
an attachment, instead of vice versa.
Thanks, David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
2011-05-20 8:31 David Edelsohn
@ 2011-05-20 9:13 ` edmar
2011-05-24 4:46 ` edmar
1 sibling, 0 replies; 6+ messages in thread
From: edmar @ 2011-05-20 9:13 UTC (permalink / raw)
To: David Edelsohn; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]
This time I put them in a tar file, to make sure the mail client don't
mess with them.
(ChangeLog is already inside the patch as well)
After I complete the regression tests, I will post a new version against
latest
subversion revision. Will post it here and on bugzilla.
Thanks,
Edmar
On 05/19/2011 04:36 PM, David Edelsohn wrote:
> 010-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
>
> Okay.
>
> Did you intend to include the gcc44 diff?
>
> Also, it's better if you include the ChangeLog inline and the patch as
> an attachment, instead of vice versa.
>
> Thanks, David
>
> .
>
[-- Attachment #2: sub_ppc_rnreg3.tar --]
[-- Type: application/x-tar, Size: 20480 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
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
1 sibling, 1 reply; 6+ messages in thread
From: edmar @ 2011-05-24 4:46 UTC (permalink / raw)
To: David Edelsohn; +Cc: GCC Patches
[-- 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" ""))]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
2011-05-24 4:46 ` edmar
@ 2011-05-25 15:11 ` David Edelsohn
2011-06-13 17:27 ` edmar
0 siblings, 1 reply; 6+ messages in thread
From: David Edelsohn @ 2011-05-25 15:11 UTC (permalink / raw)
To: edmar; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers
2011-05-25 15:11 ` David Edelsohn
@ 2011-06-13 17:27 ` edmar
0 siblings, 0 replies; 6+ messages in thread
From: edmar @ 2011-06-13 17:27 UTC (permalink / raw)
To: David Edelsohn; +Cc: GCC Patches
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.
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-13 15:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-19 18:10 [RFA] [PR44618] [PowerPC] Wrong code for -frename-registers edmar
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
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).