Re: [PATCH 3/7] target/ppc: Use tcg_gen_gvec_dup_imm

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> We can now unify the implementation of the 3 VSPLTI instructions.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/ppc/translate/vmx-impl.inc.c | 32 ++++++++++++++++-------------
>  target/ppc/translate/vsx-impl.inc.c |  2 +-
>  2 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c
> index 81d5a7a341..403ed3a01c 100644
> --- a/target/ppc/translate/vmx-impl.inc.c
> +++ b/target/ppc/translate/vmx-impl.inc.c
> @@ -1035,21 +1035,25 @@ GEN_VXRFORM_DUAL(vcmpbfp, PPC_ALTIVEC, PPC_NONE, \
>  GEN_VXRFORM_DUAL(vcmpgtfp, PPC_ALTIVEC, PPC_NONE, \
>                   vcmpgtud, PPC_NONE, PPC2_ALTIVEC_207)
>  
> -#define GEN_VXFORM_DUPI(name, tcg_op, opc2, opc3)                       \
> -static void glue(gen_, name)(DisasContext *ctx)                         \
> -    {                                                                   \
> -        int simm;                                                       \
> -        if (unlikely(!ctx->altivec_enabled)) {                          \
> -            gen_exception(ctx, POWERPC_EXCP_VPU);                       \
> -            return;                                                     \
> -        }                                                               \
> -        simm = SIMM5(ctx->opcode);                                      \
> -        tcg_op(avr_full_offset(rD(ctx->opcode)), 16, 16, simm);         \
> +static void gen_vsplti(DisasContext *ctx, int vece)
> +{
> +    int simm;
> +
> +    if (unlikely(!ctx->altivec_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_VPU);
> +        return;
>      }
>  
> -GEN_VXFORM_DUPI(vspltisb, tcg_gen_gvec_dup8i, 6, 12);
> -GEN_VXFORM_DUPI(vspltish, tcg_gen_gvec_dup16i, 6, 13);
> -GEN_VXFORM_DUPI(vspltisw, tcg_gen_gvec_dup32i, 6, 14);
> +    simm = SIMM5(ctx->opcode);
> +    tcg_gen_gvec_dup_imm(vece, avr_full_offset(rD(ctx->opcode)), 16, 16, simm);
> +}
> +
> +#define GEN_VXFORM_VSPLTI(name, vece, opc2, opc3) \
> +static void glue(gen_, name)(DisasContext *ctx) { gen_vsplti(ctx, vece); }
> +
> +GEN_VXFORM_VSPLTI(vspltisb, MO_8, 6, 12);
> +GEN_VXFORM_VSPLTI(vspltish, MO_16, 6, 13);
> +GEN_VXFORM_VSPLTI(vspltisw, MO_32, 6, 14);

There are unused parameters opc2/opc3 parameters here. Given that is it
really worth the glue obfuscation:

  static void gen_vspltisb(DisasContext *ctx)
  {
      gen_vsplti(ctx, MO_8);
  }

  static void gen_vspltish(DisasContext *ctx)
  {
      gen_vsplti(ctx, MO_8);
  }

  static void gen_vspltisw(DisasContext *ctx)
  {
      gen_vsplti(ctx, MO_8);
  }

Of course I tried grepping for their use and couldn't find them until I
realised the call to them was hidden inside another glue operation.

With the removed extra unused macro params:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>  
>  #define GEN_VXFORM_NOA(name, opc2, opc3)                                \
>  static void glue(gen_, name)(DisasContext *ctx)                         \
> @@ -1559,7 +1563,7 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE,
>  #undef GEN_VXRFORM_DUAL
>  #undef GEN_VXRFORM1
>  #undef GEN_VXRFORM
> -#undef GEN_VXFORM_DUPI
> +#undef GEN_VXFORM_VSPLTI
>  #undef GEN_VXFORM_NOA
>  #undef GEN_VXFORM_UIMM
>  #undef GEN_VAFORM_PAIRED
> diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c
> index 8287e272f5..b518de46db 100644
> --- a/target/ppc/translate/vsx-impl.inc.c
> +++ b/target/ppc/translate/vsx-impl.inc.c
> @@ -1579,7 +1579,7 @@ static void gen_xxspltib(DisasContext *ctx)
>              return;
>          }
>      }
> -    tcg_gen_gvec_dup8i(vsr_full_offset(rt), 16, 16, uim8);
> +    tcg_gen_gvec_dup_imm(MO_8, vsr_full_offset(rt), 16, 16, uim8);
>  }
>  
>  static void gen_xxsldwi(DisasContext *ctx)


-- 
Alex Bennée