> -----Original Message-----
> From: Simon Dardis [mailto:simon.dar...@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.lawre...@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
