diff mbox

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

Message ID 20160920233843.GA31065@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Sept. 20, 2016, 11:38 p.m. UTC
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.

Comments

Segher Boessenkool Sept. 21, 2016, 6:36 p.m. UTC | #1
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
diff mbox

Patch

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")])