[Qemu-devel] [PATCH 0/3] target/arm: Vector expansion improvments.

[Qemu-devel] [PATCH 0/3] target/arm: Vector expansion improvments.

[Richard Henderson]

Based-on: <20190104223116.14037-1-richard.henderson@linaro.org>
which is queued in my tcg-next branch.

Patch 1 removes some conditionals that were added to the
generic expander.  The other two make use of the new min/max
vector expanders.


r~


Richard Henderson (3):
  target/arm: Rely on optimization within tcg_gen_gvec_or
  target/arm: Use vector minmax expanders for aarch64
  target/arm: Use vector minmax expanders for aarch32

 target/arm/translate-a64.c | 41 ++++++++++++++------------------------
 target/arm/translate-sve.c |  6 +-----
 target/arm/translate.c     | 37 ++++++++++++++++++++--------------
 3 files changed, 38 insertions(+), 46 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH 1/3] target/arm: Rely on optimization within tcg_gen_gvec_or

[Richard Henderson]

Since we're now handling a == b generically, we no longer need
to do it by hand within target/arm/.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c |  6 +-----
 target/arm/translate-sve.c |  6 +-----
 target/arm/translate.c     | 12 +++---------
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e1da1e4d6f..2d6f8c1b4f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -10152,11 +10152,7 @@ static void disas_simd_3same_logic(DisasContext *s, uint32_t insn)
         gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_andc, 0);
         return;
     case 2: /* ORR */
-        if (rn == rm) { /* MOV */
-            gen_gvec_fn2(s, is_q, rd, rn, tcg_gen_gvec_mov, 0);
-        } else {
-            gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_or, 0);
-        }
+        gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_or, 0);
         return;
     case 3: /* ORN */
         gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_orc, 0);
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index b15b615ceb..3a2eb51566 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -280,11 +280,7 @@ static bool trans_AND_zzz(DisasContext *s, arg_rrr_esz *a)
 
 static bool trans_ORR_zzz(DisasContext *s, arg_rrr_esz *a)
 {
-    if (a->rn == a->rm) { /* MOV */
-        return do_mov_z(s, a->rd, a->rn);
-    } else {
-        return do_vector3_z(s, tcg_gen_gvec_or, 0, a->rd, a->rn, a->rm);
-    }
+    return do_vector3_z(s, tcg_gen_gvec_or, 0, a->rd, a->rn, a->rm);
 }
 
 static bool trans_EOR_zzz(DisasContext *s, arg_rrr_esz *a)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 7c4675ffd8..33b1860148 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -6294,15 +6294,9 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
                 tcg_gen_gvec_andc(0, rd_ofs, rn_ofs, rm_ofs,
                                   vec_size, vec_size);
                 break;
-            case 2:
-                if (rn == rm) {
-                    /* VMOV */
-                    tcg_gen_gvec_mov(0, rd_ofs, rn_ofs, vec_size, vec_size);
-                } else {
-                    /* VORR */
-                    tcg_gen_gvec_or(0, rd_ofs, rn_ofs, rm_ofs,
-                                    vec_size, vec_size);
-                }
+            case 2: /* VORR */
+                tcg_gen_gvec_or(0, rd_ofs, rn_ofs, rm_ofs,
+                                vec_size, vec_size);
                 break;
             case 3: /* VORN */
                 tcg_gen_gvec_orc(0, rd_ofs, rn_ofs, rm_ofs,
-- 
2.17.2

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

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 2d6f8c1b4f..bef21ada71 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -10452,6 +10452,20 @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn)
     }
 
     switch (opcode) {
+    case 0x0c: /* SMAX, UMAX */
+        if (u) {
+            gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_umax, size);
+        } else {
+            gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_smax, size);
+        }
+        return;
+    case 0x0d: /* SMIN, UMIN */
+        if (u) {
+            gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_umin, size);
+        } else {
+            gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_smin, size);
+        }
+        return;
     case 0x10: /* ADD, SUB */
         if (u) {
             gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_sub, size);
@@ -10613,27 +10627,6 @@ static void disas_simd_3same_int(DisasContext *s, uint32_t insn)
                 genenvfn = fns[size][u];
                 break;
             }
-            case 0xc: /* SMAX, UMAX */
-            {
-                static NeonGenTwoOpFn * const fns[3][2] = {
-                    { gen_helper_neon_max_s8, gen_helper_neon_max_u8 },
-                    { gen_helper_neon_max_s16, gen_helper_neon_max_u16 },
-                    { tcg_gen_smax_i32, tcg_gen_umax_i32 },
-                };
-                genfn = fns[size][u];
-                break;
-            }
-
-            case 0xd: /* SMIN, UMIN */
-            {
-                static NeonGenTwoOpFn * const fns[3][2] = {
-                    { gen_helper_neon_min_s8, gen_helper_neon_min_u8 },
-                    { gen_helper_neon_min_s16, gen_helper_neon_min_u16 },
-                    { tcg_gen_smin_i32, tcg_gen_umin_i32 },
-                };
-                genfn = fns[size][u];
-                break;
-            }
             case 0xe: /* SABD, UABD */
             case 0xf: /* SABA, UABA */
             {
-- 
2.17.2

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

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 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;
-- 
2.17.2

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

[Peter Maydell]

On Sun, 6 Jan 2019 at 22:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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.

thanks
-- PMM

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

[Peter Maydell]

On Sun, 6 Jan 2019 at 22:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate-a64.c | 35 ++++++++++++++---------------------
>  1 file changed, 14 insertions(+), 21 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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 <peter.maydell@linaro.org> wrote:
>
> On Sun, 6 Jan 2019 at 22:50, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  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 <peter.maydell@linaro.org>
on this one.

thanks
-- PMM