diff mbox

[MIPS] MSA machine description fixes

Message ID 72E73D4BBFC6704695754C43662FC584C4F54EBE@PUMAIL01.pu.imgtec.org
State New
Headers show

Commit Message

Prachi Godbole Feb. 7, 2017, 12:02 p.m. UTC 
Hi,

The patch fixes some bugs as mentioned below.

1. mips_gen_const_int_vector(): Change type for argument VAL from int to HOST_WIDE_INT to allow const vector of type doubleword. It in turn enables generation of BCLRI.d instead of AND.d for immediate const vector operand with only one bit clear.

2. MSA dot product family instruction for .d format: Fix wrong MODE for vec_select in the second mult operand. It enables some optimizations like CSE fwprop etc.

3. signed min/max immediate: Fix print operand code so as to print signed immediate instead of an unsigned one.

4. MSA max/min absolute instruction family: Introduce mode iterator in if_then_else construct. It enables some optimizations like CSE fwprop etc.

Tests for all of them are also included in the patch.

Ok for trunk?

Regards,
Prachi

Changelog:

2017-02-07  Prachi Godbole  <prachi.godbole@imgtec.com>

gcc/
	* config/mips/mips-msa.md (msa_dotp_<su>_d, msa_dpadd_<su>_d,
	msa_dpsub_<su>_d): Fix MODE for vec_select.
	(msa_fmax_a_<msafmt>, msa_fmin_a_<msafmt>, msa_max_a_<msafmt>,
	msa_min_a_<msafmt>): Introduce mode interator for if_then_else.
	(smin<mode>3, smax<mode>3): Change operand print code from 'B' to 'E'.
	* config/mips/mips.c (mips_gen_const_int_vector): Change type of last
	argument.
	* config/mips/mips-protos.h (mips_gen_const_int_vector): Likewise.

gcc/testsuite/
	* gcc.target/mips/msa-1.c: New tests.

Comments

Moore, Catherine Feb. 21, 2017, 11:35 p.m. UTC | #1 
> The patch fixes some bugs as mentioned below.
> 
> 1. mips_gen_const_int_vector(): Change type for argument VAL from
> int to HOST_WIDE_INT to allow const vector of type doubleword. It in
> turn enables generation of BCLRI.d instead of AND.d for immediate
> const vector operand with only one bit clear.
> 
> 2. MSA dot product family instruction for .d format: Fix wrong MODE
> for vec_select in the second mult operand. It enables some
> optimizations like CSE fwprop etc.
> 
> 3. signed min/max immediate: Fix print operand code so as to print
> signed immediate instead of an unsigned one.
> 
> 4. MSA max/min absolute instruction family: Introduce mode iterator
> in if_then_else construct. It enables some optimizations like CSE
> fwprop etc.
> 
> Tests for all of them are also included in the patch.
> 
> Ok for trunk?
> 
> Changelog:
> 
> 2017-02-07  Prachi Godbole  <prachi.godbole@imgtec.com>
> 
> gcc/
> 	* config/mips/mips-msa.md (msa_dotp_<su>_d,
> msa_dpadd_<su>_d,
> 	msa_dpsub_<su>_d): Fix MODE for vec_select.
> 	(msa_fmax_a_<msafmt>, msa_fmin_a_<msafmt>,
> msa_max_a_<msafmt>,
> 	msa_min_a_<msafmt>): Introduce mode interator for
> if_then_else.
> 	(smin<mode>3, smax<mode>3): Change operand print code
> from 'B' to 'E'.
> 	* config/mips/mips.c (mips_gen_const_int_vector): Change
> type of last
> 	argument.
> 	* config/mips/mips-protos.h (mips_gen_const_int_vector):
> Likewise.
> 
Hi Prachi,

This part of the patch is fine, although I would like you to split it into three patches giving a separate commit for each bug fix.

> gcc/testsuite/
> 	* gcc.target/mips/msa-1.c: New tests.

The testsuite patch should be split be split as well.  I have a couple of comments on the test itself.
> 
> 
> Index: gcc/testsuite/gcc.target/mips/msa-1.c
> ==========================================================
> =========
> --- gcc/testsuite/gcc.target/mips/msa-1.c       (revision 0)
> +++ gcc/testsuite/gcc.target/mips/msa-1.c       (revision 0)
> @@ -0,0 +1,69 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mno-mips16 -mfp64 -mhard-float -mmsa" } */
> +
> +typedef int v4i32 __attribute__ ((vector_size(16)));
> +typedef long long v2i64 __attribute__ ((vector_size(16)));
> +typedef float v4f32 __attribute__ ((vector_size(16)));
> +
> +/* Test MSA AND.d optimization: generate BCLRI.d instead, for
> immediate const
> +   vector operand with only one bit clear.  */
> +
> +void
> +and_d_msa (v2i64 *vx, v2i64 *vy)
> +{
> +  v2i64 and_vec = {0x7FFFFFFFFFFFFFFF, 0x7FFFFFFFFFFFFFFF};
> +  *vy = (*vx) & and_vec;
> +}

I know that compiler crashed for this test, but let's test for the generation of the BCLRI as well.

> +
> +/* Test MSA dot product family for CSE optimization.  */
> +
> +static v4i32 g = {0, 92, 93, 94};
> +static v4i32 h = {12, 24, 36, 48};
> +static v2i64 l = {84, 98};
> +
> +void
> +dotp_d_msa (v2i64 *c)
> +{
> +  l = __builtin_msa_dotp_s_d (g, h);
> +}
> +
> +void
> +dpadd_d_msa (v2i64 *c)
> +{
> +  *c = __builtin_msa_dpadd_s_d (l, g, h);
> +}
> +
> +void
> +dpsub_d_msa (v2i64 *c)
> +{
> +  *c = __builtin_msa_dpsub_s_d (l, g, h);
> +}

Likewise.  Can you test the assembly or one of the dumps for the optimization that you're looking for?

> +
> +/* Test MSA signed min/max immediate for correct assembly output.
> */
> +
> +void
> +min_s_msa (v4i32 *vx, v4i32 *vy)
> +{
> +  *vy = __builtin_msa_mini_s_w (*vx, -15);
> +}
> +/* { dg-final { scan-assembler "-15" } }  */
> +
> +void
> +max_s_msa (v4i32 *vx, v4i32 *vy)
> +{
> +  *vy = __builtin_msa_maxi_s_w (*vx, -15);
> +}
> +/* { dg-final { scan-assembler "-15" } }  */
> +
> +/* Test MSA min_a/max_a instructions for forward propagation
> optimization.  */
> +
> +#define FUNC(NAME, TYPE, RETTYPE) RETTYPE NAME##_a_msa (TYPE
> *vx, TYPE *vy) \
> +{ \
> +  TYPE dest = __builtin_msa_##NAME##_a_w (*vx, *vy); \
> +  return dest[0]; \
> +}
> +
> +FUNC(fmin, v4f32, float)
> +FUNC(fmax, v4f32, float)
> +FUNC(min, v4i32, int)
> +FUNC(max, v4i32, int)

This is OK.

Please repost the patches with the modifications.
Thanks,
Catherine
diff mbox

Patch

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 245205)
+++ gcc/config/mips/mips.c	(working copy)
@@ -21608,7 +21608,7 @@ 
 /* Return a const_int vector of VAL with mode MODE.  */
 
 rtx
-mips_gen_const_int_vector (machine_mode mode, int val)
+mips_gen_const_int_vector (machine_mode mode, HOST_WIDE_INT val)
 {
   int nunits = GET_MODE_NUNITS (mode);
   rtvec v = rtvec_alloc (nunits);
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h	(revision 245205)
+++ gcc/config/mips/mips-protos.h	(working copy)
@@ -294,7 +294,7 @@ 
 extern bool mips_const_vector_bitimm_set_p (rtx, machine_mode);
 extern bool mips_const_vector_bitimm_clr_p (rtx, machine_mode);
 extern rtx mips_msa_vec_parallel_const_half (machine_mode, bool);
-extern rtx mips_gen_const_int_vector (machine_mode, int);
+extern rtx mips_gen_const_int_vector (machine_mode, HOST_WIDE_INT);
 extern bool mips_secondary_memory_needed (enum reg_class, enum reg_class,
 					  machine_mode);
 extern bool mips_cannot_change_mode_class (machine_mode,
Index: gcc/config/mips/mips-msa.md
===================================================================
--- gcc/config/mips/mips-msa.md	(revision 245205)
+++ gcc/config/mips/mips-msa.md	(working copy)
@@ -1230,10 +1230,10 @@ 
 		(parallel [(const_int 0) (const_int 2)]))))
 	  (mult:V2DI
 	    (any_extend:V2DI
-	      (vec_select:V4SI (match_dup 1)
+	      (vec_select:V2SI (match_dup 1)
 		(parallel [(const_int 1) (const_int 3)])))
 	    (any_extend:V2DI
-	      (vec_select:V4SI (match_dup 2)
+	      (vec_select:V2SI (match_dup 2)
 		(parallel [(const_int 1) (const_int 3)]))))))]
   "ISA_HAS_MSA"
   "dotp_<su>.d\t%w0,%w1,%w2"
@@ -1319,10 +1319,10 @@ 
 		  (parallel [(const_int 0) (const_int 2)]))))
 	    (mult:V2DI
 	      (any_extend:V2DI
-		(vec_select:V4SI (match_dup 2)
+		(vec_select:V2SI (match_dup 2)
 		  (parallel [(const_int 1) (const_int 3)])))
 	      (any_extend:V2DI
-		(vec_select:V4SI (match_dup 3)
+		(vec_select:V2SI (match_dup 3)
 		  (parallel [(const_int 1) (const_int 3)])))))
 	  (match_operand:V2DI 1 "register_operand" "0")))]
   "ISA_HAS_MSA"
@@ -1414,10 +1414,10 @@ 
 		  (parallel [(const_int 0) (const_int 2)]))))
 	    (mult:V2DI
 	      (any_extend:V2DI
-		(vec_select:V4SI (match_dup 2)
+		(vec_select:V2SI (match_dup 2)
 		  (parallel [(const_int 1) (const_int 3)])))
 	      (any_extend:V2DI
-		(vec_select:V4SI (match_dup 3)
+		(vec_select:V2SI (match_dup 3)
 		  (parallel [(const_int 1) (const_int 3)])))))))]
   "ISA_HAS_MSA"
   "dpsub_<su>.d\t%w0,%w2,%w3"
@@ -1688,7 +1688,7 @@ 
 
 (define_insn "msa_fmax_a_<msafmt>"
   [(set (match_operand:FMSA 0 "register_operand" "=f")
-	(if_then_else
+	(if_then_else:FMSA
 	   (gt (abs:FMSA (match_operand:FMSA 1 "register_operand" "f"))
 	       (abs:FMSA (match_operand:FMSA 2 "register_operand" "f")))
 	   (match_dup 1)
@@ -1709,7 +1709,7 @@ 
 
 (define_insn "msa_fmin_a_<msafmt>"
   [(set (match_operand:FMSA 0 "register_operand" "=f")
-	(if_then_else
+	(if_then_else:FMSA
 	   (lt (abs:FMSA (match_operand:FMSA 1 "register_operand" "f"))
 	       (abs:FMSA (match_operand:FMSA 2 "register_operand" "f")))
 	   (match_dup 1)
@@ -2174,7 +2174,7 @@ 
 
 (define_insn "msa_max_a_<msafmt>"
   [(set (match_operand:IMSA 0 "register_operand" "=f")
-	(if_then_else
+	(if_then_else:IMSA
 	   (gt (abs:IMSA (match_operand:IMSA 1 "register_operand" "f"))
 	       (abs:IMSA (match_operand:IMSA 2 "register_operand" "f")))
 	   (match_dup 1)
@@ -2191,7 +2191,7 @@ 
   "ISA_HAS_MSA"
   "@
    max_s.<msafmt>\t%w0,%w1,%w2
-   maxi_s.<msafmt>\t%w0,%w1,%B2"
+   maxi_s.<msafmt>\t%w0,%w1,%E2"
   [(set_attr "type" "simd_int_arith")
    (set_attr "mode" "<MODE>")])
 
@@ -2208,7 +2208,7 @@ 
 
 (define_insn "msa_min_a_<msafmt>"
   [(set (match_operand:IMSA 0 "register_operand" "=f")
-	(if_then_else
+	(if_then_else:IMSA
 	   (lt (abs:IMSA (match_operand:IMSA 1 "register_operand" "f"))
 	       (abs:IMSA (match_operand:IMSA 2 "register_operand" "f")))
 	   (match_dup 1)
@@ -2225,7 +2225,7 @@ 
   "ISA_HAS_MSA"
   "@
    min_s.<msafmt>\t%w0,%w1,%w2
-   mini_s.<msafmt>\t%w0,%w1,%B2"
+   mini_s.<msafmt>\t%w0,%w1,%E2"
   [(set_attr "type" "simd_int_arith")
    (set_attr "mode" "<MODE>")])
Index: gcc/testsuite/gcc.target/mips/msa-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/msa-1.c       (revision 0)
+++ gcc/testsuite/gcc.target/mips/msa-1.c       (revision 0)
@@ -0,0 +1,69 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mno-mips16 -mfp64 -mhard-float -mmsa" } */
+
+typedef int v4i32 __attribute__ ((vector_size(16)));
+typedef long long v2i64 __attribute__ ((vector_size(16)));
+typedef float v4f32 __attribute__ ((vector_size(16)));
+
+/* Test MSA AND.d optimization: generate BCLRI.d instead, for immediate const
+   vector operand with only one bit clear.  */
+
+void
+and_d_msa (v2i64 *vx, v2i64 *vy)
+{
+  v2i64 and_vec = {0x7FFFFFFFFFFFFFFF, 0x7FFFFFFFFFFFFFFF};
+  *vy = (*vx) & and_vec;
+}
+
+/* Test MSA dot product family for CSE optimization.  */
+
+static v4i32 g = {0, 92, 93, 94};
+static v4i32 h = {12, 24, 36, 48};
+static v2i64 l = {84, 98};
+
+void
+dotp_d_msa (v2i64 *c)
+{
+  l = __builtin_msa_dotp_s_d (g, h);
+}
+
+void
+dpadd_d_msa (v2i64 *c)
+{
+  *c = __builtin_msa_dpadd_s_d (l, g, h);
+}
+
+void
+dpsub_d_msa (v2i64 *c)
+{
+  *c = __builtin_msa_dpsub_s_d (l, g, h);
+}
+
+/* Test MSA signed min/max immediate for correct assembly output.  */
+
+void
+min_s_msa (v4i32 *vx, v4i32 *vy)
+{
+  *vy = __builtin_msa_mini_s_w (*vx, -15);
+}
+/* { dg-final { scan-assembler "-15" } }  */
+
+void
+max_s_msa (v4i32 *vx, v4i32 *vy)
+{
+  *vy = __builtin_msa_maxi_s_w (*vx, -15);
+}
+/* { dg-final { scan-assembler "-15" } }  */
+
+/* Test MSA min_a/max_a instructions for forward propagation optimization.  */
+
+#define FUNC(NAME, TYPE, RETTYPE) RETTYPE NAME##_a_msa (TYPE *vx, TYPE *vy) \
+{ \
+  TYPE dest = __builtin_msa_##NAME##_a_w (*vx, *vy); \
+  return dest[0]; \
+}
+
+FUNC(fmin, v4f32, float)
+FUNC(fmax, v4f32, float)
+FUNC(min, v4i32, int)
+FUNC(max, v4i32, int)