public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
@ 2015-10-01 14:43 Simon Dardis
  2015-10-06 10:12 ` Alan Lawrence
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Dardis @ 2015-10-01 14:43 UTC (permalink / raw)
  To: Alan Lawrence, Matthew Fortune, Moore, Catherine; +Cc: gcc-patches

Hello,

This patch migrates the MIPS backend to the new vector reduction optabs. 


No new regressions, ok to apply?

Thanks,
Simon

gcc/ChangeLog:

	* config/mips/loongson.md	(vec_loongson_extract_lo_<mode>): New, extract low part to scalar.
	(reduc_uplus_<mode>): Remove.
	(reduc_plus_scal_<mode>): Rename from reduc_splus_<mode>, Use  vec loongson_extract_lo_<mode>.
	(reduc_smax_scal_<mode>, reduc_smin_scal_<mode>): Rename from reduc_smax_<mode>, 
	reduc_smax_<mode>, fix constraints, use vec loongson_extract_lo_<mode>.
	(reduc_umax_scal_<mode>, reduc_umin_scal_<mode>): Rename, change constraints.
	
Index: config/mips/loongson.md
===================================================================
--- config/mips/loongson.md	(revision 228282)
+++ config/mips/loongson.md	(working copy)
@@ -852,58 +852,66 @@
   "dsrl\t%0,%1,%2"
   [(set_attr "type" "fcvt")])
 
-(define_expand "reduc_uplus_<mode>"
-  [(match_operand:VWH 0 "register_operand" "")
-   (match_operand:VWH 1 "register_operand" "")]
+(define_insn "vec_loongson_extract_lo_<mode>"
+  [(set (match_operand:<V_inner> 0 "register_operand" "=r")
+        (vec_select:<V_inner>
+          (match_operand:VWHB 1 "register_operand" "f")
+          (parallel [(const_int 0)])))]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
-{
-  mips_expand_vec_reduc (operands[0], operands[1], gen_add<mode>3);
-  DONE;
-})
+  "mfc1\t%0,%1"
+  [(set_attr "type" "mfc")])
 
-; ??? Given that we're not describing a widening reduction, we should
-; not have separate optabs for signed and unsigned.
-(define_expand "reduc_splus_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
+(define_expand "reduc_plus_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VWHB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  emit_insn (gen_reduc_uplus_<mode>(operands[0], operands[1]));
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_add<mode>3);
+  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_smax_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
-   (match_operand:VWHB 1 "register_operand" "")]
+(define_expand "reduc_smax_scal_<mode>"
+  [(match_operand:HI 0 "register_operand" "")
+   (match_operand:VH 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_smax<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_smax<mode>3);
+  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_smin_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
-   (match_operand:VWHB 1 "register_operand" "")]
+(define_expand "reduc_smin_scal_<mode>"
+  [(match_operand:HI 0 "register_operand" "")
+   (match_operand:VH 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_smin<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_smin<mode>3);
+  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_umax_<mode>"
-  [(match_operand:VB 0 "register_operand" "")
+(define_expand "reduc_umax_scal_<mode>"
+  [(match_operand:QI 0 "register_operand" "")
    (match_operand:VB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_umax<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_umax<mode>3);
+  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_umin_<mode>"
-  [(match_operand:VB 0 "register_operand" "")
+(define_expand "reduc_umin_scal_<mode>"
+  [(match_operand:QI 0 "register_operand" "")
    (match_operand:VB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_umin<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_umin<mode>3);
+  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-10-01 14:43 [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs Simon Dardis
@ 2015-10-06 10:12 ` Alan Lawrence
  2015-10-07 10:50   ` Simon Dardis
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Lawrence @ 2015-10-06 10:12 UTC (permalink / raw)
  To: Simon Dardis, Matthew Fortune, Moore, Catherine; +Cc: gcc-patches

Thanks for working on this, Simon!

On 01/10/15 15:43, Simon Dardis wrote:
> -(define_expand "reduc_smax_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> -   (match_operand:VWHB 1 "register_operand" "")]
> +(define_expand "reduc_smax_scal_<mode>"
> +  [(match_operand:HI 0 "register_operand" "")
> +   (match_operand:VH 1 "register_operand" "")]


> -(define_expand "reduc_smin_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> -   (match_operand:VWHB 1 "register_operand" "")]
> +(define_expand "reduc_smin_scal_<mode>"
> +  [(match_operand:HI 0 "register_operand" "")
> +   (match_operand:VH 1 "register_operand" "")]

I note these two change from VWHB to VH; the latter is just V4HI, so this loses 
you smin/smax for V2SI and V8QI...is that intentional? (It looks like you define 
vec_loongson_extract_lo for all relevant modes so I would expect you to use 
<V_inner> as you do for reduc_plus_scal.)

(In contrast umax/umin only had VB = V8QI variants before.)

Also a minor stylistic point:

> +  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));

(Five instances) spurious space after (.


Cheers, Alan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-10-06 10:12 ` Alan Lawrence
@ 2015-10-07 10:50   ` Simon Dardis
  2015-10-07 11:40     ` Alan Lawrence
                       ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Simon Dardis @ 2015-10-07 10:50 UTC (permalink / raw)
  To: Alan Lawrence, Matthew Fortune, Moore, Catherine; +Cc: gcc-patches

On the change from smin/smax it was a deliberate change as I managed to confuse myself of the mode patterns, correct version follows. Reverted back to VWHB for smax/smin. Stylistic point addressed.

No new regression, ok for commit?

Thanks,
Simon

Index: config/mips/loongson.md
===================================================================
--- config/mips/loongson.md	(revision 228282)
+++ config/mips/loongson.md	(working copy)
@@ -852,58 +852,66 @@
   "dsrl\t%0,%1,%2"
   [(set_attr "type" "fcvt")])
 
-(define_expand "reduc_uplus_<mode>"
-  [(match_operand:VWH 0 "register_operand" "")
-   (match_operand:VWH 1 "register_operand" "")]
+(define_insn "vec_loongson_extract_lo_<mode>"
+  [(set (match_operand:<V_inner> 0 "register_operand" "=r")
+        (vec_select:<V_inner>
+          (match_operand:VWHB 1 "register_operand" "f")
+          (parallel [(const_int 0)])))]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
-{
-  mips_expand_vec_reduc (operands[0], operands[1], gen_add<mode>3);
-  DONE;
-})
+  "mfc1\t%0,%1"
+  [(set_attr "type" "mfc")])
 
-; ??? Given that we're not describing a widening reduction, we should
-; not have separate optabs for signed and unsigned.
-(define_expand "reduc_splus_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
+(define_expand "reduc_plus_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VWHB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  emit_insn (gen_reduc_uplus_<mode>(operands[0], operands[1]));
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_add<mode>3);
+  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_smax_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
+(define_expand "reduc_smax_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VWHB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_smax<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_smax<mode>3);
+  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_smin_<mode>"
-  [(match_operand:VWHB 0 "register_operand" "")
+(define_expand "reduc_smin_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VWHB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_smin<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_smin<mode>3);
+  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_umax_<mode>"
-  [(match_operand:VB 0 "register_operand" "")
+(define_expand "reduc_umax_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_umax<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_umax<mode>3);
+  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })
 
-(define_expand "reduc_umin_<mode>"
-  [(match_operand:VB 0 "register_operand" "")
+(define_expand "reduc_umin_scal_<mode>"
+  [(match_operand:<V_inner> 0 "register_operand" "")
    (match_operand:VB 1 "register_operand" "")]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
 {
-  mips_expand_vec_reduc (operands[0], operands[1], gen_umin<mode>3);
+  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
+  mips_expand_vec_reduc (tmp, operands[1], gen_umin<mode>3);
+  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
   DONE;
 })


-----Original Message-----
From: Alan Lawrence [mailto:alan.lawrence@arm.com] 
Sent: 06 October 2015 11:12
To: Simon Dardis; Matthew Fortune; Moore, Catherine
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.

Thanks for working on this, Simon!

On 01/10/15 15:43, Simon Dardis wrote:
> -(define_expand "reduc_smax_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> -   (match_operand:VWHB 1 "register_operand" "")]
> +(define_expand "reduc_smax_scal_<mode>"
> +  [(match_operand:HI 0 "register_operand" "")
> +   (match_operand:VH 1 "register_operand" "")]


> -(define_expand "reduc_smin_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> -   (match_operand:VWHB 1 "register_operand" "")]
> +(define_expand "reduc_smin_scal_<mode>"
> +  [(match_operand:HI 0 "register_operand" "")
> +   (match_operand:VH 1 "register_operand" "")]

I note these two change from VWHB to VH; the latter is just V4HI, so this loses you smin/smax for V2SI and V8QI...is that intentional? (It looks like you define vec_loongson_extract_lo for all relevant modes so I would expect you to use <V_inner> as you do for reduc_plus_scal.)

(In contrast umax/umin only had VB = V8QI variants before.)

Also a minor stylistic point:

> +  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));

(Five instances) spurious space after (.


Cheers, Alan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-10-07 10:50   ` Simon Dardis
@ 2015-10-07 11:40     ` Alan Lawrence
  2015-10-22 15:00     ` Alan Lawrence
  2015-11-03 14:08     ` Moore, Catherine
  2 siblings, 0 replies; 7+ messages in thread
From: Alan Lawrence @ 2015-10-07 11:40 UTC (permalink / raw)
  To: Simon Dardis, Matthew Fortune, Moore, Catherine; +Cc: gcc-patches

On 07/10/15 11:50, Simon Dardis wrote:
> On the change from smin/smax it was a deliberate change as I managed to confuse myself of the mode patterns, correct version follows. Reverted back to VWHB for smax/smin. Stylistic point addressed.
>
> No new regression, ok for commit?

Well, I'm not a MIPS maintainer, but it looks sensible to me now :)

Thanks again!
--Alan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-10-07 10:50   ` Simon Dardis
  2015-10-07 11:40     ` Alan Lawrence
@ 2015-10-22 15:00     ` Alan Lawrence
  2015-11-03 14:08     ` Moore, Catherine
  2 siblings, 0 replies; 7+ messages in thread
From: Alan Lawrence @ 2015-10-22 15:00 UTC (permalink / raw)
  To: Simon Dardis, Matthew Fortune, Moore, Catherine; +Cc: gcc-patches

On closer inspection I think you can also remove this guy (from loongson.md):

(define_insn "reduc_uplus_v8qi"
   [(set (match_operand:V8QI 0 "register_operand" "=f")
         (unspec:V8QI [(match_operand:V8QI 1 "register_operand" "f")]
                      UNSPEC_LOONGSON_BIADD))]
   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
   "biadd\t%0,%1"
   [(set_attr "type" "fabs")])

as reduc_plus_scal_<mode> handles both signed and unsigned cases.

Cheers, Alan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-10-07 10:50   ` Simon Dardis
  2015-10-07 11:40     ` Alan Lawrence
  2015-10-22 15:00     ` Alan Lawrence
@ 2015-11-03 14:08     ` Moore, Catherine
  2015-11-06 12:00       ` Simon Dardis
  2 siblings, 1 reply; 7+ messages in thread
From: Moore, Catherine @ 2015-11-03 14:08 UTC (permalink / raw)
  To: Simon Dardis, Alan Lawrence, Matthew Fortune; +Cc: gcc-patches



> -----Original Message-----
> From: Simon Dardis [mailto:Simon.Dardis@imgtec.com]
> Sent: Wednesday, October 07, 2015 6:51 AM
> To: Alan Lawrence; Matthew Fortune; Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> 
> On the change from smin/smax it was a deliberate change as I managed to
> confuse myself of the mode patterns, correct version follows. Reverted back
> to VWHB for smax/smin. Stylistic point addressed.
> 
> No new regression, ok for commit?
> 

Yes, OK to commit.  Sorry for the delay in review.
Catherine

> 
> Index: config/mips/loongson.md
> ==========================================================
> =========
> --- config/mips/loongson.md	(revision 228282)
> +++ config/mips/loongson.md	(working copy)
> @@ -852,58 +852,66 @@
>    "dsrl\t%0,%1,%2"
>    [(set_attr "type" "fcvt")])
> 
> -(define_expand "reduc_uplus_<mode>"
> -  [(match_operand:VWH 0 "register_operand" "")
> -   (match_operand:VWH 1 "register_operand" "")]
> +(define_insn "vec_loongson_extract_lo_<mode>"
> +  [(set (match_operand:<V_inner> 0 "register_operand" "=r")
> +        (vec_select:<V_inner>
> +          (match_operand:VWHB 1 "register_operand" "f")
> +          (parallel [(const_int 0)])))]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> -{
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_add<mode>3);
> -  DONE;
> -})
> +  "mfc1\t%0,%1"
> +  [(set_attr "type" "mfc")])
> 
> -; ??? Given that we're not describing a widening reduction, we should
> -; not have separate optabs for signed and unsigned.
> -(define_expand "reduc_splus_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_plus_scal_<mode>"
> +  [(match_operand:<V_inner> 0 "register_operand" "")
>     (match_operand:VWHB 1 "register_operand" "")]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  emit_insn (gen_reduc_uplus_<mode>(operands[0], operands[1]));
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_add<mode>3);
> +  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
>    DONE;
>  })
> 
> -(define_expand "reduc_smax_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_smax_scal_<mode>"
> +  [(match_operand:<V_inner> 0 "register_operand" "")
>     (match_operand:VWHB 1 "register_operand" "")]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_smax<mode>3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_smax<mode>3);
> +  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
>    DONE;
>  })
> 
> -(define_expand "reduc_smin_<mode>"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_smin_scal_<mode>"
> +  [(match_operand:<V_inner> 0 "register_operand" "")
>     (match_operand:VWHB 1 "register_operand" "")]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_smin<mode>3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_smin<mode>3);
> +  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
>    DONE;
>  })
> 
> -(define_expand "reduc_umax_<mode>"
> -  [(match_operand:VB 0 "register_operand" "")
> +(define_expand "reduc_umax_scal_<mode>"
> +  [(match_operand:<V_inner> 0 "register_operand" "")
>     (match_operand:VB 1 "register_operand" "")]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_umax<mode>3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_umax<mode>3);
> +  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
>    DONE;
>  })
> 
> -(define_expand "reduc_umin_<mode>"
> -  [(match_operand:VB 0 "register_operand" "")
> +(define_expand "reduc_umin_scal_<mode>"
> +  [(match_operand:<V_inner> 0 "register_operand" "")
>     (match_operand:VB 1 "register_operand" "")]
>    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_umin<mode>3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_umin<mode>3);
> +  emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
>    DONE;
>  })
> 
> 
> -----Original Message-----
> From: Alan Lawrence [mailto:alan.lawrence@arm.com]
> Sent: 06 October 2015 11:12
> To: Simon Dardis; Matthew Fortune; Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> 
> Thanks for working on this, Simon!
> 
> On 01/10/15 15:43, Simon Dardis wrote:
> > -(define_expand "reduc_smax_<mode>"
> > -  [(match_operand:VWHB 0 "register_operand" "")
> > -   (match_operand:VWHB 1 "register_operand" "")]
> > +(define_expand "reduc_smax_scal_<mode>"
> > +  [(match_operand:HI 0 "register_operand" "")
> > +   (match_operand:VH 1 "register_operand" "")]
> 
> 
> > -(define_expand "reduc_smin_<mode>"
> > -  [(match_operand:VWHB 0 "register_operand" "")
> > -   (match_operand:VWHB 1 "register_operand" "")]
> > +(define_expand "reduc_smin_scal_<mode>"
> > +  [(match_operand:HI 0 "register_operand" "")
> > +   (match_operand:VH 1 "register_operand" "")]
> 
> I note these two change from VWHB to VH; the latter is just V4HI, so this
> loses you smin/smax for V2SI and V8QI...is that intentional? (It looks like you
> define vec_loongson_extract_lo for all relevant modes so I would expect you
> to use <V_inner> as you do for reduc_plus_scal.)
> 
> (In contrast umax/umin only had VB = V8QI variants before.)
> 
> Also a minor stylistic point:
> 
> > +  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0],
> tmp));
> 
> (Five instances) spurious space after (.
> 
> 
> Cheers, Alan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
  2015-11-03 14:08     ` Moore, Catherine
@ 2015-11-06 12:00       ` Simon Dardis
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Dardis @ 2015-11-06 12:00 UTC (permalink / raw)
  To: Moore, Catherine, Alan Lawrence, Matthew Fortune; +Cc: gcc-patches

Committed r229844.

Thanks,
Simon

> -----Original Message-----
> From: Moore, Catherine [mailto:Catherine_Moore@mentor.com]
> Sent: 03 November 2015 14:09
> To: Simon Dardis; Alan Lawrence; Matthew Fortune
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> 
> 
> 
> > -----Original Message-----
> > From: Simon Dardis [mailto:Simon.Dardis@imgtec.com]
> > Sent: Wednesday, October 07, 2015 6:51 AM
> > To: Alan Lawrence; Matthew Fortune; Moore, Catherine
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> >
> > On the change from smin/smax it was a deliberate change as I managed
> > to confuse myself of the mode patterns, correct version follows.
> > Reverted back to VWHB for smax/smin. Stylistic point addressed.
> >
> > No new regression, ok for commit?
> >
> 
> Yes, OK to commit.  Sorry for the delay in review.
> Catherine
> 
> >
> > Index: config/mips/loongson.md
> >
> ==========================================================
> > =========
> > --- config/mips/loongson.md	(revision 228282)
> > +++ config/mips/loongson.md	(working copy)
> > @@ -852,58 +852,66 @@
> >    "dsrl\t%0,%1,%2"
> >    [(set_attr "type" "fcvt")])
> >
> > -(define_expand "reduc_uplus_<mode>"
> > -  [(match_operand:VWH 0 "register_operand" "")
> > -   (match_operand:VWH 1 "register_operand" "")]
> > +(define_insn "vec_loongson_extract_lo_<mode>"
> > +  [(set (match_operand:<V_inner> 0 "register_operand" "=r")
> > +        (vec_select:<V_inner>
> > +          (match_operand:VWHB 1 "register_operand" "f")
> > +          (parallel [(const_int 0)])))]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> > -{
> > -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_add<mode>3);
> > -  DONE;
> > -})
> > +  "mfc1\t%0,%1"
> > +  [(set_attr "type" "mfc")])
> >
> > -; ??? Given that we're not describing a widening reduction, we should
> > -; not have separate optabs for signed and unsigned.
> > -(define_expand "reduc_splus_<mode>"
> > -  [(match_operand:VWHB 0 "register_operand" "")
> > +(define_expand "reduc_plus_scal_<mode>"
> > +  [(match_operand:<V_inner> 0 "register_operand" "")
> >     (match_operand:VWHB 1 "register_operand" "")]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> >  {
> > -  emit_insn (gen_reduc_uplus_<mode>(operands[0], operands[1]));
> > +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> > + mips_expand_vec_reduc (tmp, operands[1], gen_add<mode>3);
> emit_insn
> > + (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
> >    DONE;
> >  })
> >
> > -(define_expand "reduc_smax_<mode>"
> > -  [(match_operand:VWHB 0 "register_operand" "")
> > +(define_expand "reduc_smax_scal_<mode>"
> > +  [(match_operand:<V_inner> 0 "register_operand" "")
> >     (match_operand:VWHB 1 "register_operand" "")]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> >  {
> > -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_smax<mode>3);
> > +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> > + mips_expand_vec_reduc (tmp, operands[1], gen_smax<mode>3);
> > + emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
> >    DONE;
> >  })
> >
> > -(define_expand "reduc_smin_<mode>"
> > -  [(match_operand:VWHB 0 "register_operand" "")
> > +(define_expand "reduc_smin_scal_<mode>"
> > +  [(match_operand:<V_inner> 0 "register_operand" "")
> >     (match_operand:VWHB 1 "register_operand" "")]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> >  {
> > -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_smin<mode>3);
> > +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> > + mips_expand_vec_reduc (tmp, operands[1], gen_smin<mode>3);
> > + emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
> >    DONE;
> >  })
> >
> > -(define_expand "reduc_umax_<mode>"
> > -  [(match_operand:VB 0 "register_operand" "")
> > +(define_expand "reduc_umax_scal_<mode>"
> > +  [(match_operand:<V_inner> 0 "register_operand" "")
> >     (match_operand:VB 1 "register_operand" "")]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> >  {
> > -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_umax<mode>3);
> > +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> > + mips_expand_vec_reduc (tmp, operands[1], gen_umax<mode>3);
> > + emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
> >    DONE;
> >  })
> >
> > -(define_expand "reduc_umin_<mode>"
> > -  [(match_operand:VB 0 "register_operand" "")
> > +(define_expand "reduc_umin_scal_<mode>"
> > +  [(match_operand:<V_inner> 0 "register_operand" "")
> >     (match_operand:VB 1 "register_operand" "")]
> >    "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> >  {
> > -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_umin<mode>3);
> > +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> > + mips_expand_vec_reduc (tmp, operands[1], gen_umin<mode>3);
> > + emit_insn (gen_vec_loongson_extract_lo_<mode> (operands[0], tmp));
> >    DONE;
> >  })
> >
> >
> > -----Original Message-----
> > From: Alan Lawrence [mailto:alan.lawrence@arm.com]
> > Sent: 06 October 2015 11:12
> > To: Simon Dardis; Matthew Fortune; Moore, Catherine
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> >
> > Thanks for working on this, Simon!
> >
> > On 01/10/15 15:43, Simon Dardis wrote:
> > > -(define_expand "reduc_smax_<mode>"
> > > -  [(match_operand:VWHB 0 "register_operand" "")
> > > -   (match_operand:VWHB 1 "register_operand" "")]
> > > +(define_expand "reduc_smax_scal_<mode>"
> > > +  [(match_operand:HI 0 "register_operand" "")
> > > +   (match_operand:VH 1 "register_operand" "")]
> >
> >
> > > -(define_expand "reduc_smin_<mode>"
> > > -  [(match_operand:VWHB 0 "register_operand" "")
> > > -   (match_operand:VWHB 1 "register_operand" "")]
> > > +(define_expand "reduc_smin_scal_<mode>"
> > > +  [(match_operand:HI 0 "register_operand" "")
> > > +   (match_operand:VH 1 "register_operand" "")]
> >
> > I note these two change from VWHB to VH; the latter is just V4HI, so
> > this loses you smin/smax for V2SI and V8QI...is that intentional? (It
> > looks like you define vec_loongson_extract_lo for all relevant modes
> > so I would expect you to use <V_inner> as you do for reduc_plus_scal.)
> >
> > (In contrast umax/umin only had VB = V8QI variants before.)
> >
> > Also a minor stylistic point:
> >
> > > +  emit_insn ( gen_vec_loongson_extract_lo_<mode> (operands[0],
> > tmp));
> >
> > (Five instances) spurious space after (.
> >
> >
> > Cheers, Alan


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-11-06 12:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-01 14:43 [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs Simon Dardis
2015-10-06 10:12 ` Alan Lawrence
2015-10-07 10:50   ` Simon Dardis
2015-10-07 11:40     ` Alan Lawrence
2015-10-22 15:00     ` Alan Lawrence
2015-11-03 14:08     ` Moore, Catherine
2015-11-06 12:00       ` Simon Dardis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).