[PATCH], PR 77670, Fix PowerPC ISA 3.0 -mpower9-minmax code generation

[PATCH], PR 77670, Fix PowerPC ISA 3.0 -mpower9-minmax code generation

[Michael Meissner]

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

There are some instructions in PowerPC ISA 3.0 that are not currently enabled
by default when you use -mcpu=power9.  These instructions are currently added
with the -mpower9-minmax switch.

I recently built Spec 2006 with the current trunk compiler, and I discovered
that three of the benchmarks (gamess, povray, and soplex) do not build when you
use the -mpower9-minmiax option.

In particular, ISA 3.0 adds 3 new instructions: XSCMPEQDP, XSCMPGEDP, and
XSCMPGTDP that are similar to the vector instructions in that they set the
scalar part of the vector register to all 0's or all 1's so that the XXSEL
instruction can be used to do a floating point conditional move.

The code did not provide an inverted operation that moves the registers if the
condition is false instead of true.  I added the an insn splitter for the
inverted operation to fix the problem.

In testing it, I discovered that when I implemented the XXSEL operation for
scalar, I had the registers backwards.  I provided a fix for this as well.

I rebuilt Spec 2016, and it all builds now.  I also tested a small program on
the simulator and it generated the correct output.

Since this is in ISA 3.0 code only, I did not do the full bootstrap and make
check sequence.  I can do this if you prefer.

Is the patch ok to put into the trunk?

2016-09-20  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/77670
	* config/rs6000/predicates.md (invert_fpmask_comparison_operator):
	New predicate that matches the ISA 3.0 XSCMP{EQ,GT,GE}DP
	instructions when you want to invert the test.
	* config/rs6000/rs6000.md (fpmask<mode>): Use the arguments in the
	correct order for XXSEL.
	(mov<SFDF:mode><SFDF2:mode>cc_invert_p9): Define the inverted test
	for using XSCMP{EQ,GT,GE}DP.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: pr77670.patch01b --]
[-- Type: text/plain, Size: 2845 bytes --]

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 240283)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -1172,6 +1172,12 @@ (define_predicate "scc_rev_comparison_op
 (define_predicate "fpmask_comparison_operator"
   (match_code "eq,gt,ge"))
 
+;; Return 1 if OP is a comparison operator suitable for vector/scalar
+;; comparisons that generate a 0/-1 mask (i.e. the inverse of
+;; fpmask_comparison_operator).
+(define_predicate "invert_fpmask_comparison_operator"
+  (match_code "ne,unlt,unle"))
+
 ;; Return 1 if OP is a comparison operation that is valid for a branch
 ;; insn, which is true if the corresponding bit in the CC register is set.
 (define_predicate "branch_positive_comparison_operator"
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 240283)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -4882,6 +4882,43 @@ (define_insn_and_split "*mov<SFDF:mode><
  [(set_attr "length" "8")
   (set_attr "type" "vecperm")])
 
+;; Handle inverting the fpmask comparisons.
+(define_insn_and_split "*mov<SFDF:mode><SFDF2:mode>cc_invert_p9"
+  [(set (match_operand:SFDF 0 "vsx_register_operand" "=&<SFDF:Fv>,<SFDF:Fv>")
+	(if_then_else:SFDF
+	 (match_operator:CCFP 1 "invert_fpmask_comparison_operator"
+		[(match_operand:SFDF2 2 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")
+		 (match_operand:SFDF2 3 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")])
+	 (match_operand:SFDF 4 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")
+	 (match_operand:SFDF 5 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")))
+   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
+  "TARGET_P9_MINMAX"
+  "#"
+  ""
+  [(set (match_dup 6)
+	(if_then_else:V2DI (match_dup 9)
+			   (match_dup 7)
+			   (match_dup 8)))
+   (set (match_dup 0)
+	(if_then_else:SFDF (ne (match_dup 6)
+			       (match_dup 8))
+			   (match_dup 5)
+			   (match_dup 4)))]
+{
+  rtx op1 = operands[1];
+  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
+
+  if (GET_CODE (operands[6]) == SCRATCH)
+    operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+
+  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
 (define_insn "*fpmask<mode>"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa")
 	(if_then_else:V2DI
@@ -4901,7 +4938,7 @@ (define_insn "*xxsel<mode>"
 			   (match_operand:SFDF 3 "vsx_register_operand" "<Fv>")
 			   (match_operand:SFDF 4 "vsx_register_operand" "<Fv>")))]
   "TARGET_P9_MINMAX"
-  "xxsel %x0,%x1,%x3,%x4"
+  "xxsel %x0,%x4,%x3,%x1"
   [(set_attr "type" "vecmove")])
 
 \f

Re: [PATCH], PR 77670, Fix PowerPC ISA 3.0 -mpower9-minmax code generation

[Segher Boessenkool]

On Tue, Sep 20, 2016 at 07:38:43PM -0400, Michael Meissner wrote:
> Since this is in ISA 3.0 code only, I did not do the full bootstrap and make
> check sequence.  I can do this if you prefer.

Yes, please do, to make sure you didn't accidentally break something else.

> Is the patch ok to put into the trunk?


> +(define_insn_and_split "*mov<SFDF:mode><SFDF2:mode>cc_invert_p9"
> +  [(set (match_operand:SFDF 0 "vsx_register_operand" "=&<SFDF:Fv>,<SFDF:Fv>")
> +	(if_then_else:SFDF
> +	 (match_operator:CCFP 1 "invert_fpmask_comparison_operator"
> +		[(match_operand:SFDF2 2 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")
> +		 (match_operand:SFDF2 3 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")])
> +	 (match_operand:SFDF 4 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")
> +	 (match_operand:SFDF 5 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")))
> +   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
> +  "TARGET_P9_MINMAX"
> +  "#"
> +  ""

"&& 1" here?

Okay with that change.  Thanks,


Segher

Re: [PATCH], Backport PR 77670 fix to GCC 6.3

[Michael Meissner]

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

I tried building spec 2006 with the current gcc 6.x branch, and gamess, soplex,
and povray fail when I use -mcpu=power9 -mpower9-minmax.

The patch for the trunk applies to the GCC 6.x branch, and I was able to build
all of Spec 2006 with the patched compiler.  I did a bootstrap build and make
check with no regressions on a little endian Power8 system.  Can I apply this
patch to the gcc 6.x branch?

2016-09-29  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk

	2016-09-21  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/77670
	* config/rs6000/predicates.md (invert_fpmask_comparison_operator):
	New predicate that matches the ISA 3.0 XSCMP{EQ,GT,GE}DP
	instructions when you want to invert the test.
	* config/rs6000/rs6000.md (fpmask<mode>): Use the arguments in the
	correct order for XXSEL.
	(mov<SFDF:mode><SFDF2:mode>cc_invert_p9): Define the inverted test
	for using XSCMP{EQ,GT,GE}DP.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: pr77670.patch02b --]
[-- Type: text/plain, Size: 2849 bytes --]

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 240634)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -1163,6 +1163,12 @@ (define_predicate "scc_rev_comparison_op
 (define_predicate "fpmask_comparison_operator"
   (match_code "eq,gt,ge"))
 
+;; Return 1 if OP is a comparison operator suitable for vector/scalar
+;; comparisons that generate a 0/-1 mask (i.e. the inverse of
+;; fpmask_comparison_operator).
+(define_predicate "invert_fpmask_comparison_operator"
+  (match_code "ne,unlt,unle"))
+
 ;; Return 1 if OP is a comparison operation that is valid for a branch
 ;; insn, which is true if the corresponding bit in the CC register is set.
 (define_predicate "branch_positive_comparison_operator"
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 240634)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -4846,6 +4846,43 @@ (define_insn_and_split "*mov<SFDF:mode><
  [(set_attr "length" "8")
   (set_attr "type" "vecperm")])
 
+;; Handle inverting the fpmask comparisons.
+(define_insn_and_split "*mov<SFDF:mode><SFDF2:mode>cc_invert_p9"
+  [(set (match_operand:SFDF 0 "vsx_register_operand" "=&<SFDF:Fv>,<SFDF:Fv>")
+	(if_then_else:SFDF
+	 (match_operator:CCFP 1 "invert_fpmask_comparison_operator"
+		[(match_operand:SFDF2 2 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")
+		 (match_operand:SFDF2 3 "vsx_register_operand" "<SFDF2:Fv>,<SFDF2:Fv>")])
+	 (match_operand:SFDF 4 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")
+	 (match_operand:SFDF 5 "vsx_register_operand" "<SFDF:Fv>,<SFDF:Fv>")))
+   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
+  "TARGET_P9_MINMAX"
+  "#"
+  "&& 1"
+  [(set (match_dup 6)
+	(if_then_else:V2DI (match_dup 9)
+			   (match_dup 7)
+			   (match_dup 8)))
+   (set (match_dup 0)
+	(if_then_else:SFDF (ne (match_dup 6)
+			       (match_dup 8))
+			   (match_dup 5)
+			   (match_dup 4)))]
+{
+  rtx op1 = operands[1];
+  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
+
+  if (GET_CODE (operands[6]) == SCRATCH)
+    operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+
+  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
 (define_insn "*fpmask<mode>"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa")
 	(if_then_else:V2DI
@@ -4865,7 +4902,7 @@ (define_insn "*xxsel<mode>"
 			   (match_operand:SFDF 3 "vsx_register_operand" "<Fv>")
 			   (match_operand:SFDF 4 "vsx_register_operand" "<Fv>")))]
   "TARGET_P9_MINMAX"
-  "xxsel %x0,%x1,%x3,%x4"
+  "xxsel %x0,%x4,%x3,%x1"
   [(set_attr "type" "vecmove")])
 
 \f

Re: [PATCH], Backport PR 77670 fix to GCC 6.3

[Segher Boessenkool]

On Thu, Sep 29, 2016 at 06:04:47PM -0400, Michael Meissner wrote:
> I tried building spec 2006 with the current gcc 6.x branch, and gamess, soplex,
> and povray fail when I use -mcpu=power9 -mpower9-minmax.
> 
> The patch for the trunk applies to the GCC 6.x branch, and I was able to build
> all of Spec 2006 with the patched compiler.  I did a bootstrap build and make
> check with no regressions on a little endian Power8 system.  Can I apply this
> patch to the gcc 6.x branch?

Yes please.  Thanks,


Segher