diff mbox series

[rs6000] Fix PR80546: FAIL: gcc.target/powerpc/bool3-p[78].c scan-assembler-not

Message ID c9d4a3a0-402f-471b-5ca7-9596d1bf4e5b@vnet.ibm.com
State New
Headers show
Series [rs6000] Fix PR80546: FAIL: gcc.target/powerpc/bool3-p[78].c scan-assembler-not | expand

Commit Message

Peter Bergner March 30, 2018, 7:29 p.m. UTC 
Currently, the vwx_mov<mode>_* move patterns diparage use of GPR registers.
This tends to force all modes handled by the move patterns to prefer using
VSX registers, even in cases where it doesn't make sense (ie, TImode).
The bool3-p[78].c:ptr() test cases are such an example.  The following patch
is a minimal fix to get test cases PASSing again, by not disparaging GPR
usage if the modes are DImode or TImode.

Comparing before & after for the ptr4() test case, we now see:

-       mtvsrd 0,10
-       mtvsrd 1,11
-       xxpermdi 12,0,1,0
-       xxlnor 0,12,12
-       mfvsrd 10,0
-       xxpermdi 0,0,0,3
-       mfvsrd 11,0
+       not 10,10
+       not 11,11

The plan is to revisit these move patterns again in GCC9 to see if more
cleanup and enhancements can be made, but it's too late for that in GCC8.

This passed bootstrap and regtesting on both powerpc64le-linux and
powerpc64-linux with no regressions.  Mike also did a spec comparison with
the patch and only found two benchmarks (povray and xalancbmk) with any
code differences.  The -mcpu=power8 changes for both benchmarks we mostly
just address differences.  The -mcpu=power9 differences did show a few
cases where we generate better code than before (ie, we don't load values
into VSX registers just use a direct move them into a GPR register).

Is this ok for trunk?  This is marked as a GCC7 and GCC8 regression,
so ok for GCC 7 too after it has baked on trunk for a while?

Peter

	PR target/80546
	* config/rs6000/vsx.md (??r): New mode attribute.
	(*vsx_mov<mode>_64bit): Use it.
	(*vsx_mov<mode>_32bit): Likewise.

Comments

Segher Boessenkool March 30, 2018, 11:15 p.m. UTC | #1 
Hi!

On Fri, Mar 30, 2018 at 02:29:59PM -0500, Peter Bergner wrote:
> +;; A mode attribute to disparage use of GPR registers, except for scalar
> +;; interger modes.

Typo ("integer").

> +(define_mode_attr ??r	[(V16QI "??r")
> +			 (V8HI  "??r")
> +			 (V4SI  "??r")
> +			 (V4SF  "??r")
> +			 (V2DI  "??r")
> +			 (V2DF  "??r")
> +			 (DI    "r")
> +			 (DF    "??r")
> +			 (SF    "??r")
> +			 (V1TI  "??r")
> +			 (TI    "r")
> +			 (TF    "??r")
> +			 (KF    "??r")])

Uuuuugly :-)

DI cannot happen, just remove it (it is not part of VSX_M).  Neither
DF or SF.

> @@ -1200,7 +1216,7 @@ (define_insn_and_split "*xxspltib_<mode>
>  (define_insn "*vsx_mov<mode>_64bit"
>    [(set (match_operand:VSX_M 0 "nonimmediate_operand"
>                 "=ZwO,      <VSa>,     <VSa>,     r,         we,        ?wQ,
> -                ?&r,       ??r,       ??Y,       ??r,       wo,        v,
> +                ?&r,       ??r,       ??Y,       <??r>,     wo,        v,
>                  ?<VSa>,    *r,        v,         ??r,       wZ,        v")
>  
>  	(match_operand:VSX_M 1 "input_operand" 
> @@ -1229,7 +1245,7 @@ (define_insn "*vsx_mov<mode>_64bit"
>  ;;              LVX (VMX)  STVX (VMX)
>  (define_insn "*vsx_mov<mode>_32bit"
>    [(set (match_operand:VSX_M 0 "nonimmediate_operand"
> -               "=ZwO,      <VSa>,     <VSa>,     ??r,       ??Y,       ??r,
> +               "=ZwO,      <VSa>,     <VSa>,     ??r,       ??Y,       <??r>,
>                  wo,        v,         ?<VSa>,    *r,        v,         ??r,
>                  wZ,        v")

We should give the same treatment to (most or all of) the other ??r in
those patterns, or more likely, split TI off to a separate pattern.

But okay (with the fixes), and okay for 7.  Thanks!


Segher
Peter Bergner April 3, 2018, 12:01 a.m. UTC | #2 
On 3/30/18 6:15 PM, Segher Boessenkool wrote:
> But okay (with the fixes), and okay for 7.  Thanks!

Ok, committed now to trunk and GCC 7 with your suggestions.  Thanks!

Peter
diff mbox series

Patch

Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md	(revision 258802)
+++ gcc/config/rs6000/vsx.md	(working copy)
@@ -170,6 +170,22 @@  (define_mode_attr VSa	[(V16QI "wa")
 			 (TF	"wp")
 			 (KF	"wq")])
 
+;; A mode attribute to disparage use of GPR registers, except for scalar
+;; interger modes.
+(define_mode_attr ??r	[(V16QI "??r")
+			 (V8HI  "??r")
+			 (V4SI  "??r")
+			 (V4SF  "??r")
+			 (V2DI  "??r")
+			 (V2DF  "??r")
+			 (DI    "r")
+			 (DF    "??r")
+			 (SF    "??r")
+			 (V1TI  "??r")
+			 (TI    "r")
+			 (TF    "??r")
+			 (KF    "??r")])
+
 ;; Same size integer type for floating point data
 (define_mode_attr VSi [(V4SF  "v4si")
 		       (V2DF  "v2di")
@@ -1200,7 +1216,7 @@  (define_insn_and_split "*xxspltib_<mode>
 (define_insn "*vsx_mov<mode>_64bit"
   [(set (match_operand:VSX_M 0 "nonimmediate_operand"
                "=ZwO,      <VSa>,     <VSa>,     r,         we,        ?wQ,
-                ?&r,       ??r,       ??Y,       ??r,       wo,        v,
+                ?&r,       ??r,       ??Y,       <??r>,     wo,        v,
                 ?<VSa>,    *r,        v,         ??r,       wZ,        v")
 
 	(match_operand:VSX_M 1 "input_operand" 
@@ -1229,7 +1245,7 @@  (define_insn "*vsx_mov<mode>_64bit"
 ;;              LVX (VMX)  STVX (VMX)
 (define_insn "*vsx_mov<mode>_32bit"
   [(set (match_operand:VSX_M 0 "nonimmediate_operand"
-               "=ZwO,      <VSa>,     <VSa>,     ??r,       ??Y,       ??r,
+               "=ZwO,      <VSa>,     <VSa>,     ??r,       ??Y,       <??r>,
                 wo,        v,         ?<VSa>,    *r,        v,         ??r,
                 wZ,        v")