Re: [Qemu-devel] [PATCH v5 2/2] target/mips: Optimize ILVEV.<B|H|W|D> MSA instructions

[Mateja Marjanovic]

On 3.4.19. 01:25, Aleksandar Markovic wrote:
> On Apr 2, 2019 5:20 PM, "Mateja Marjanovic" 
> <address@hidden <mailto:address@hidden>> wrote:
> >
> > From: Mateja Marjanovic <address@hidden 
> <mailto:address@hidden>>
> >
> > Optimize set of MSA instructions ILVEV, using directly
>
> Use full instruction names, with the only exception of possible 
> Bachus-Naur forms... again.
I will, I didn't change it from some of the previous versions.
> > tcg registers and performing logic on them instead of
> > using helpers.
> >
> > In the following table, the first column is the performance
> > before this patch. The second represents the performance,
> > after converting from helpers to tcg, but without using
> > tcg_gen_deposit function. The third one is the solution
> > which is implemented in this patch.
> >
> >  instr    ||   before    || no-deposit ||  with-deposit
> > ========================================================
> >  ilvev.b  ||  126.92 ms  ||  24.52 ms  ||  24.43 ms
> >  ilvev.h  ||   93.67 ms  ||  23.92 ms  ||  23.86 ms
> >  ilvev.w  ||  117.86 ms  ||  23.83 ms  ||  22.17 ms
> >  ilvev.d  ||   45.49 ms  ||  19.74 ms  ||  19.71 ms
> >
> > The solution with deposit is suggested by Richard Henderson.
> >
> > Signed-off-by: Mateja Marjanovic <address@hidden 
> <mailto:address@hidden>>
> > ---
>
> The byte and halfword cases of this patch most likely produce highly 
> unoptimized code for cases:
>
> wd == wt == ws
> wd == wt != ws
> wd != ws == wt
> wd == ws != wt
>
> Please take these cases into account.
>
> The same for patch 1/2.
Maybe, but if I put if statements asking are the registers the same,
it would affect the performance significantly in all cases. If some
registers were equal, it would be faster, but if not, just those if 
statements
would slow things down.
> Thanks,
> Aleksandar
Thanks,
Mateja

> >  target/mips/helper.h     |   1 -
> >  target/mips/msa_helper.c |   9 ----
> >  target/mips/translate.c  | 105 
> ++++++++++++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 104 insertions(+), 11 deletions(-)
> >
> > diff --git a/target/mips/helper.h b/target/mips/helper.h
> > index 02e16c7..82f6a40 100644
> > --- a/target/mips/helper.h
> > +++ b/target/mips/helper.h
> > @@ -864,7 +864,6 @@ DEF_HELPER_5(msa_pckev_df, void, env, i32, i32, 
> i32, i32)
> >  DEF_HELPER_5(msa_pckod_df, void, env, i32, i32, i32, i32)
> >  DEF_HELPER_5(msa_ilvl_df, void, env, i32, i32, i32, i32)
> >  DEF_HELPER_5(msa_ilvr_df, void, env, i32, i32, i32, i32)
> > -DEF_HELPER_5(msa_ilvev_df, void, env, i32, i32, i32, i32)
> >  DEF_HELPER_5(msa_vshf_df, void, env, i32, i32, i32, i32)
> >  DEF_HELPER_5(msa_srar_df, void, env, i32, i32, i32, i32)
> >  DEF_HELPER_5(msa_srlr_df, void, env, i32, i32, i32, i32)
> > diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
> > index a7ea6aa..d5c3842 100644
> > --- a/target/mips/msa_helper.c
> > +++ b/target/mips/msa_helper.c
> > @@ -1197,15 +1197,6 @@ MSA_FN_DF(ilvl_df)
> >      } while (0)
> >  MSA_FN_DF(ilvr_df)
> >  #undef MSA_DO
> > -
> > -#define MSA_DO(DF)                      \
> > -    do {                                \
> > -        pwx->DF[2*i]   = pwt->DF[2*i];  \
> > -        pwx->DF[2*i+1] = pws->DF[2*i];  \
> > -    } while (0)
> > -MSA_FN_DF(ilvev_df)
> > -#undef MSA_DO
> > -
> >  #undef MSA_LOOP_COND
> >
> >  #define MSA_LOOP_COND(DF) \
> > diff --git a/target/mips/translate.c b/target/mips/translate.c
> > index 04406d6..e26c6a6 100644
> > --- a/target/mips/translate.c
> > +++ b/target/mips/translate.c
> > @@ -28974,6 +28974,94 @@ static inline void gen_ilvod_d(CPUMIPSState 
> *env, uint32_t wd,
> >      tcg_gen_mov_i64(msa_wr_d[wd * 2 + 1], msa_wr_d[ws * 2 + 1]);
> >  }
> >
> > +/*
> > + * [MSA] ILVEV.B wd, ws, wt
> > + *
> > + *   Vector Interleave Even (byte data elements)
> > + *
> > + */
> > +static inline void gen_ilvev_b(CPUMIPSState *env, uint32_t wd,
> > +                               uint32_t ws, uint32_t wt)
> > +{
> > +    TCGv_i64 t1 = tcg_temp_new_i64();
> > +    TCGv_i64 t2 = tcg_temp_new_i64();
> > +    const uint64_t mask = 0x00ff00ff00ff00ffULL;
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2], mask);
> > +    tcg_gen_andi_i64(t2, msa_wr_d[ws * 2], mask);
> > +    tcg_gen_shli_i64(t2, t2, 8);
> > +    tcg_gen_or_i64(msa_wr_d[wd * 2], t1, t2);
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2 + 1], mask);
> > +    tcg_gen_andi_i64(t2, msa_wr_d[ws * 2 + 1], mask);
> > +    tcg_gen_shli_i64(t2, t2, 8);
> > +    tcg_gen_or_i64(msa_wr_d[wd * 2 + 1], t1, t2);
> > +
> > +    tcg_temp_free_i64(t1);
> > +    tcg_temp_free_i64(t2);
> > +}
> > +
> > +/*
> > + * [MSA] ILVEV.H wd, ws, wt
> > + *
> > + *   Vector Interleave Even (halfword data elements)
> > + *
> > + */
> > +static inline void gen_ilvev_h(CPUMIPSState *env, uint32_t wd,
> > +                               uint32_t ws, uint32_t wt)
> > +{
> > +    TCGv_i64 t1 = tcg_temp_new_i64();
> > +    TCGv_i64 t2 = tcg_temp_new_i64();
> > +    const uint64_t mask = 0x0000ffff0000ffffULL;
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2], mask);
> > +    tcg_gen_andi_i64(t2, msa_wr_d[ws * 2], mask);
> > +    tcg_gen_shli_i64(t2, t2, 16);
> > +    tcg_gen_or_i64(msa_wr_d[wd * 2], t1, t2);
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2 + 1], mask);
> > +    tcg_gen_andi_i64(t2, msa_wr_d[ws * 2 + 1], mask);
> > +    tcg_gen_shli_i64(t2, t2, 16);
> > +    tcg_gen_or_i64(msa_wr_d[wd * 2 + 1], t1, t2);
> > +
> > +    tcg_temp_free_i64(t1);
> > +    tcg_temp_free_i64(t2);
> > +}
> > +
> > +/*
> > + * [MSA] ILVEV.W wd, ws, wt
> > + *
> > + *   Vector Interleave Even (word data elements)
> > + *
> > + */
> > +static inline void gen_ilvev_w(CPUMIPSState *env, uint32_t wd,
> > +                               uint32_t ws, uint32_t wt)
> > +{
> > +    TCGv_i64 t1 = tcg_temp_new_i64();
> > +    const uint64_t mask = 0x00000000ffffffffULL;
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2], mask);
> > +    tcg_gen_deposit_i64(msa_wr_d[wd * 2], t1, msa_wr_d[ws * 2], 32, 
> 32);
> > +
> > +    tcg_gen_andi_i64(t1, msa_wr_d[wt * 2 + 1], mask);
> > +    tcg_gen_deposit_i64(msa_wr_d[wd * 2 + 1], t1, msa_wr_d[ws * 2 + 
> 1], 32, 32);
> > +
> > +    tcg_temp_free_i64(t1);
> > +}
> > +
> > +/*
> > + * [MSA] ILVEV.D wd, ws, wt
> > + *
> > + *   Vector Interleave Even (Double data elements)
> > + *
> > + */
> > +static inline void gen_ilvev_d(CPUMIPSState *env, uint32_t wd,
> > +                               uint32_t ws, uint32_t wt)
> > +{
> > +    tcg_gen_mov_i64(msa_wr_d[wd * 2 + 1], msa_wr_d[ws * 2]);
> > +    tcg_gen_mov_i64(msa_wr_d[wd * 2], msa_wr_d[wt * 2]);
> > +}
> > +
> >  static void gen_msa_3r(CPUMIPSState *env, DisasContext *ctx)
> >  {
> >  #define MASK_MSA_3R(op)    (MASK_MSA_MINOR(op) | (op & (0x7 << 23)))
> > @@ -29130,7 +29218,22 @@ static void gen_msa_3r(CPUMIPSState *env, 
> DisasContext *ctx)
> >          gen_helper_msa_mod_s_df(cpu_env, tdf, twd, tws, twt);
> >          break;
> >      case OPC_ILVEV_df:
> > -        gen_helper_msa_ilvev_df(cpu_env, tdf, twd, tws, twt);
> > +        switch (df) {
> > +        case DF_BYTE:
> > +            gen_ilvev_b(env, wd, ws, wt);
> > +            break;
> > +        case DF_HALF:
> > +            gen_ilvev_h(env, wd, ws, wt);
> > +            break;
> > +        case DF_WORD:
> > +            gen_ilvev_w(env, wd, ws, wt);
> > +            break;
> > +        case DF_DOUBLE:
> > +            gen_ilvev_d(env, wd, ws, wt);
> > +            break;
> > +        default:
> > +            assert(0);
> > +        }
> >          break;
> >      case OPC_BINSR_df:
> >          gen_helper_msa_binsr_df(cpu_env, tdf, twd, tws, twt);
> > --
> > 2.7.4
> >
> >