Re: [Qemu-devel] [PATCH 3/3] target/arm: Use vector minmax expanders for aarch32

[Peter Maydell]

On Tue, 8 Jan 2019 at 18:14, Peter Maydell <address@hidden> wrote:
>
> On Sun, 6 Jan 2019 at 22:50, Richard Henderson
> <address@hidden> wrote:
> >
> > Signed-off-by: Richard Henderson <address@hidden>
> > ---
> >  target/arm/translate.c | 25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/arm/translate.c b/target/arm/translate.c
> > index 33b1860148..f3f172f384 100644
> > --- a/target/arm/translate.c
> > +++ b/target/arm/translate.c
> > @@ -6368,6 +6368,25 @@ static int disas_neon_data_insn(DisasContext *s, 
> > uint32_t insn)
> >              tcg_gen_gvec_cmp(u ? TCG_COND_GEU : TCG_COND_GE, size,
> >                               rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size);
> >              return 0;
> > +
> > +        case NEON_3R_VMAX:
> > +            if (u) {
> > +                tcg_gen_gvec_umax(size, rd_ofs, rn_ofs, rm_ofs,
> > +                                  vec_size, vec_size);
> > +            } else {
> > +                tcg_gen_gvec_smax(size, rd_ofs, rn_ofs, rm_ofs,
> > +                                  vec_size, vec_size);
> > +            }
> > +            return 0;
> > +        case NEON_3R_VMIN:
> > +            if (u) {
> > +                tcg_gen_gvec_umin(size, rd_ofs, rn_ofs, rm_ofs,
> > +                                  vec_size, vec_size);
> > +            } else {
> > +                tcg_gen_gvec_smin(size, rd_ofs, rn_ofs, rm_ofs,
> > +                                  vec_size, vec_size);
> > +            }
> > +            return 0;
> >          }
> >
> >          if (size == 3) {
> > @@ -6533,12 +6552,6 @@ static int disas_neon_data_insn(DisasContext *s, 
> > uint32_t insn)
> >          case NEON_3R_VQRSHL:
> >              GEN_NEON_INTEGER_OP_ENV(qrshl);
> >              break;
> > -        case NEON_3R_VMAX:
> > -            GEN_NEON_INTEGER_OP(max);
> > -            break;
> > -        case NEON_3R_VMIN:
> > -            GEN_NEON_INTEGER_OP(min);
> > -            break;
> >          case NEON_3R_VABD:
> >              GEN_NEON_INTEGER_OP(abd);
> >              break;
> > --
>
> This leaves the helpers neon_max_[su]{8,16} unused and deletable,
> I think? neon_max_[su]32 is used only via the #defines of
> neon_pmax_[su]32 so could be renamed to pmax. Similarly min/pmin.

...but if you wanted to do the cleanup in a followon patch
you can have
Reviewed-by: Peter Maydell <address@hidden>
on this one.

thanks
-- PMM