* [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).