Re: [Qemu-devel] [PATCH 10/20] target-mips: add MSA I8 format instructions

[James Hogan]

On Mon, Jul 14, 2014 at 10:55:53AM +0100, Yongbok Kim wrote:
> add MSA I8 format instructions
> 
> Signed-off-by: Yongbok Kim <address@hidden>
> ---
>  target-mips/helper.h     |   11 ++++
>  target-mips/msa_helper.c |  140 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  target-mips/translate.c  |   94 ++++++++++++++++++++++++++++++-
>  3 files changed, 243 insertions(+), 2 deletions(-)
> 
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index 74ef094..174bc62 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -689,3 +689,14 @@ DEF_HELPER_FLAGS_3(dmthlip, 0, void, tl, tl, env)
>  #endif
>  DEF_HELPER_FLAGS_3(wrdsp, 0, void, tl, tl, env)
>  DEF_HELPER_FLAGS_2(rddsp, 0, tl, tl, env)
> +
> +/* MIPS SIMD Architecture */
> +
> +DEF_HELPER_4(msa_andi_b, void, env, i32, i32, i32)
> +DEF_HELPER_4(msa_bmnzi_b, void, env, i32, i32, i32)
> +DEF_HELPER_4(msa_bmzi_b, void, env, i32, i32, i32)
> +DEF_HELPER_4(msa_bseli_b, void, env, i32, i32, i32)
> +DEF_HELPER_4(msa_nori_b, void, env, i32, i32, i32)
> +DEF_HELPER_4(msa_ori_b, void, env, i32, i32, i32)
> +DEF_HELPER_5(msa_shf_df, void, env, i32, i32, i32, i32)
> +DEF_HELPER_4(msa_xori_b, void, env, i32, i32, i32)
> diff --git a/target-mips/msa_helper.c b/target-mips/msa_helper.c
> index 5afc9ae..2355809 100644
> --- a/target-mips/msa_helper.c
> +++ b/target-mips/msa_helper.c
> @@ -194,3 +194,143 @@ static inline void msa_store_wr_elem(CPUMIPSState *env, 
> uint64_t val,
>          assert(0);
>      }
>  }
> +
> +void helper_msa_andi_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        B(pwd, i) = B(pws, i) & i8;
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +void helper_msa_ori_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        B(pwd, i) = B(pws, i) | i8;
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +void helper_msa_nori_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        B(pwd, i) = ~(B(pws, i) | i8);
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +void helper_msa_xori_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        B(pwd, i) = B(pws, i) ^ i8;
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +#define BIT_MOVE_IF_NOT_ZERO(dest, arg1, arg2, df) \
> +            dest = UNSIGNED(((dest & (~arg2)) | (arg1 & arg2)), df)
> +
> +void helper_msa_bmnzi_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        BIT_MOVE_IF_NOT_ZERO(B(pwd, i), B(pws, i), i8, DF_BYTE);
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +#define BIT_MOVE_IF_ZERO(dest, arg1, arg2, df) \
> +            dest = UNSIGNED((dest & arg2) | (arg1 & (~arg2)), df)
> +
> +void helper_msa_bmzi_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        BIT_MOVE_IF_ZERO(B(pwd, i), B(pws, i), i8, DF_BYTE);
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}
> +
> +#define BIT_SELECT(dest, arg1, arg2, df) \
> +            dest = UNSIGNED((arg1 & (~dest)) | (arg2 & dest), df)
> +
> +void helper_msa_bseli_b(CPUMIPSState *env, uint32_t wd, uint32_t ws,
> +        uint32_t i8)
> +{
> +    void *pwd = &(env->active_fpu.fpr[wd]);
> +    void *pws = &(env->active_fpu.fpr[ws]);
> +    ALL_B_ELEMENTS(i, MSA_WRLEN) {
> +        BIT_SELECT(B(pwd, i), B(pws, i), i8, DF_BYTE);
> +    } DONE_ALL_ELEMENTS;
> +    if (env->active_msa.msair & MSAIR_WRP_BIT) {
> +        env->active_msa.msamodify |= (1 << wd);
> +    }
> +}

I reckon the functions above could all be done easily enough in TCG by
repeating i8 up to 64-bits (at translation time) and doing the
operations on 64-bit quantities. Out of interest, was there a particular
motivation to do it with helpers?

In any case, that can always be an experiment for a later patch, and it
all looks technically correct.

Reviewed-by: James Hogan <address@hidden>

Cheers
James