RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.

[Simon Dardis <Simon dot Dardis at imgtec dot com>]

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