diff mbox

, PR target/59054, powerpc code regression for power7/power8

Message ID 20131111214536.GA7985@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Nov. 11, 2013, 9:45 p.m. UTC 
This patch fixes PR 59054, which is a bug I introduced in early October, when I
added initial patches for allowing DFmode values in the traditional Altivec
registers on ISA 2.06, and SFmode values on ISA 2.07.  This patch eliminates
the constraints that allowed DImode values to go into the Altivec registers,
but had the result that for 0, the register allocator might choose to allocate
a floating point register instead of a GPR register, and then has to store the
value on the stack and reload (on ISA 2.06) or use direct move (on ISA 2.07) to
get the value in a register.

I have tested this with a bootstrap and make check on subversion id 204367 with
the fix from subversion id 204272 (fortran keyword problem).  At present, I
can't check it against the top of the trunk, due to PR 59009 (libsanitizer
breaking powerpc64-linux-gnu builds).

There were no regressions with the bootstrap or with make check.  This patch
also fixes the 64-bit failure in gcc.dg/stack-usage-1.c if you build the
compiler using the --with-cpu=power7 option.

In addition, I have included fixes for two other problems in my October
changes.  The first change only shows up when DFmode can go in the traditional
Altivec registers, in that I used %x on a CR register.  The second change is in
conversion of SFmode to DFmode, I used the wrong constraints.

Are these patches ok to install?  I would prefer to check in the patches right
now, but I can also wait until PR 59009 is fixed.

[gcc]
2013-11-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/59054
	* config/rs6000/rs6000.md (movdi_internal32): Eliminate
	constraints that would allow DImode into the traditional Altivec
	registers, but cause undesirable code generation when loading 0 as
	a constant.
	(movdi_internal32): Likewise.
	(cmp<mode>_fpr): Do not use %x for CR register output.
	(extendsfdf2_fpr): Fix constraints when -mallow-upper-df and
	-mallow-upper-sf debug switches are used.

[gcc/testsuite]
2013-11-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/59054
	* gcc.target/powerpc/pr59054.c: New test.

Comments

Michael Meissner Nov. 11, 2013, 10:01 p.m. UTC | #1 
On Mon, Nov 11, 2013 at 04:45:36PM -0500, Michael Meissner wrote:
> [gcc]
> 2013-11-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/59054
> 	* config/rs6000/rs6000.md (movdi_internal32): Eliminate
> 	constraints that would allow DImode into the traditional Altivec
> 	registers, but cause undesirable code generation when loading 0 as
> 	a constant.
> 	(movdi_internal32): Likewise.
> 	(cmp<mode>_fpr): Do not use %x for CR register output.
> 	(extendsfdf2_fpr): Fix constraints when -mallow-upper-df and
> 	-mallow-upper-sf debug switches are used.

Whoops, that second movdi_internal32 should be movdi_internal64.
David Edelsohn Nov. 12, 2013, 6:42 p.m. UTC | #2 
On Mon, Nov 11, 2013 at 4:45 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> This patch fixes PR 59054, which is a bug I introduced in early October, when I
> added initial patches for allowing DFmode values in the traditional Altivec
> registers on ISA 2.06, and SFmode values on ISA 2.07.  This patch eliminates
> the constraints that allowed DImode values to go into the Altivec registers,
> but had the result that for 0, the register allocator might choose to allocate
> a floating point register instead of a GPR register, and then has to store the
> value on the stack and reload (on ISA 2.06) or use direct move (on ISA 2.07) to
> get the value in a register.
>
> I have tested this with a bootstrap and make check on subversion id 204367 with
> the fix from subversion id 204272 (fortran keyword problem).  At present, I
> can't check it against the top of the trunk, due to PR 59009 (libsanitizer
> breaking powerpc64-linux-gnu builds).
>
> There were no regressions with the bootstrap or with make check.  This patch
> also fixes the 64-bit failure in gcc.dg/stack-usage-1.c if you build the
> compiler using the --with-cpu=power7 option.
>
> In addition, I have included fixes for two other problems in my October
> changes.  The first change only shows up when DFmode can go in the traditional
> Altivec registers, in that I used %x on a CR register.  The second change is in
> conversion of SFmode to DFmode, I used the wrong constraints.
>
> Are these patches ok to install?  I would prefer to check in the patches right
> now, but I can also wait until PR 59009 is fixed.
>
> [gcc]
> 2013-11-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/59054
>         * config/rs6000/rs6000.md (movdi_internal32): Eliminate
>         constraints that would allow DImode into the traditional Altivec
>         registers, but cause undesirable code generation when loading 0 as
>         a constant.
>         (movdi_internal32): Likewise.
>         (cmp<mode>_fpr): Do not use %x for CR register output.
>         (extendsfdf2_fpr): Fix constraints when -mallow-upper-df and
>         -mallow-upper-sf debug switches are used.
>
> [gcc/testsuite]
> 2013-11-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/59054
>         * gcc.target/powerpc/pr59054.c: New test.

Okay.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 204267)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -5226,7 +5226,7 @@  (define_insn "*cmp<mode>_fpr"
   "TARGET_<MODE>_FPR"
   "@
    fcmpu %0,%1,%2
-   xscmpudp %x0,%x1,%x2"
+   xscmpudp %0,%x1,%x2"
   [(set_attr "type" "fpcompare")])
 
 ;; Floating point conversions
@@ -5237,8 +5237,8 @@  (define_expand "extendsfdf2"
   "")
 
 (define_insn_and_split "*extendsfdf2_fpr"
-  [(set (match_operand:DF 0 "gpc_reg_operand" "=d,?d,d,wy,?wy,wv")
-	(float_extend:DF (match_operand:SF 1 "reg_or_mem_operand" "0,f,m,0,wz,Z")))]
+  [(set (match_operand:DF 0 "gpc_reg_operand" "=d,?d,d,ws,?ws,wv")
+	(float_extend:DF (match_operand:SF 1 "reg_or_mem_operand" "0,f,m,0,wy,Z")))]
   "TARGET_HARD_FLOAT && TARGET_FPRS && TARGET_DOUBLE_FLOAT"
   "@
    #
@@ -5360,7 +5360,7 @@  (define_insn "copysign<mode>3_fcpsgn"
   "TARGET_<MODE>_FPR && TARGET_CMPB"
   "@
    fcpsgn %0,%2,%1
-   xscpsgn<VSs> %x0,%x2,%x1"
+   xscpsgn<Fvsx> %x0,%x2,%x1"
   [(set_attr "type" "fp")])
 
 ;; For MIN, MAX, and conditional move, we use DEFINE_EXPAND's that involve a
@@ -10081,8 +10081,8 @@  (define_insn "p8_mfvsrd_4_disf"
 ;; Use of fprs is disparaged slightly otherwise reload prefers to reload
 ;; a gpr into a fpr instead of reloading an invalid 'Y' address
 (define_insn "*movdi_internal32"
-  [(set (match_operand:DI 0 "rs6000_nonimmediate_operand" "=Y,r,r,?m,?*d,?*d,r,?wa")
-	(match_operand:DI 1 "input_operand" "r,Y,r,d,m,d,IJKnGHF,O"))]
+  [(set (match_operand:DI 0 "rs6000_nonimmediate_operand" "=Y,r,r,?m,?*d,?*d,r")
+	(match_operand:DI 1 "input_operand" "r,Y,r,d,m,d,IJKnGHF"))]
   "! TARGET_POWERPC64
    && (gpc_reg_operand (operands[0], DImode)
        || gpc_reg_operand (operands[1], DImode))"
@@ -10093,8 +10093,7 @@  (define_insn "*movdi_internal32"
    stfd%U0%X0 %1,%0
    lfd%U1%X1 %0,%1
    fmr %0,%1
-   #
-   xxlxor %x0,%x0,%x0"
+   #"
   [(set_attr_alternative "type"
       [(const_string "store")
        (const_string "load")
@@ -10114,8 +10113,7 @@  (define_insn "*movdi_internal32"
 	   (const_string "fpload_u")
 	   (const_string "fpload")))
        (const_string "fp")
-       (const_string "*")
-       (const_string "vecsimple")])])
+       (const_string "*")])])
 
 (define_split
   [(set (match_operand:DI 0 "gpc_reg_operand" "")
@@ -10146,8 +10144,8 @@  (define_split
 { rs6000_split_multireg_move (operands[0], operands[1]); DONE; })
 
 (define_insn "*movdi_internal64"
-  [(set (match_operand:DI 0 "nonimmediate_operand" "=Y,r,r,r,r,r,?m,?*d,?*d,?Z,?wv,?wa,r,*h,*h,?wa,r,?*wg,r,?*wm")
-	(match_operand:DI 1 "input_operand" "r,Y,r,I,L,nF,d,m,d,wv,Z,wa,*h,r,0,O,*wg,r,*wm,r"))]
+  [(set (match_operand:DI 0 "nonimmediate_operand" "=Y,r,r,r,r,r,?m,?*d,?*d,r,*h,*h,r,?*wg,r,?*wm")
+	(match_operand:DI 1 "input_operand" "r,Y,r,I,L,nF,d,m,d,*h,r,0,*wg,r,*wm,r"))]
   "TARGET_POWERPC64
    && (gpc_reg_operand (operands[0], DImode)
        || gpc_reg_operand (operands[1], DImode))"
@@ -10161,13 +10159,9 @@  (define_insn "*movdi_internal64"
    stfd%U0%X0 %1,%0
    lfd%U1%X1 %0,%1
    fmr %0,%1
-   stxsd%U0x %x1,%y0
-   lxsd%U1x %x0,%y1
-   xxlor %x0,%x1,%x1
    mf%1 %0
    mt%0 %1
    nop
-   xxlxor %x0,%x0,%x0
    mftgpr %0,%1
    mffgpr %0,%1
    mfvsrd %0,%x1
@@ -10206,24 +10200,14 @@  (define_insn "*movdi_internal64"
 	   (const_string "fpload_u")
 	   (const_string "fpload")))
        (const_string "fp")
-       (if_then_else
-	 (match_test "update_indexed_address_mem (operands[0], VOIDmode)")
-	 (const_string "fpstore_ux")
-	 (const_string "fpstore"))
-       (if_then_else
-	 (match_test "update_indexed_address_mem (operands[1], VOIDmode)")
-	 (const_string "fpload_ux")
-	 (const_string "fpload"))
-       (const_string "vecsimple")
        (const_string "mfjmpr")
        (const_string "mtjmpr")
        (const_string "*")
-       (const_string "vecsimple")
        (const_string "mftgpr")
        (const_string "mffgpr")
        (const_string "mftgpr")
        (const_string "mffgpr")])
-   (set_attr "length" "4,4,4,4,4,20,4,4,4,4,4,4,4,4,4,4,4,4,4,4")])
+   (set_attr "length" "4,4,4,4,4,20,4,4,4,4,4,4,4,4,4,4")])
 
 ;; Generate all one-bits and clear left or right.
 ;; Use (and:DI (rotate:DI ...)) to avoid anddi3 unnecessary clobber.
Index: gcc/testsuite/gcc.target/powerpc/pr59054.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr59054.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr59054.c	(revision 0)
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-mcpu=power7 -O0 -m64" } */
+
+long foo (void) { return 0; }
+
+/* { dg-final { scan-assembler-not "xxlor" } } */
+/* { dg-final { scan-assembler-not "stfd" } } */