diff mbox series

[3/7] target/ppc: Use tcg_gen_gvec_dup_imm

Message ID 20200418150411.1831-4-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: Clean up tcg_gen_gvec_dupi interface | expand

Commit Message

Richard Henderson April 18, 2020, 3:04 p.m. UTC 
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(-)

-- 
2.20.1

Comments

David Gibson April 20, 2020, 3:41 a.m. UTC | #1 
On Sat, Apr 18, 2020 at 08:04:07AM -0700, Richard Henderson wrote:
> We can now unify the implementation of the 3 VSPLTI instructions.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Acked-by: David Gibson <david@gibson.dropbear.id.au>


> ---

>  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);

>  

>  #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)


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Alex Bennée April 20, 2020, 10:34 a.m. UTC | #2 
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
Richard Henderson April 21, 2020, 5:50 p.m. UTC | #3 
On 4/20/20 3:34 AM, Alex Bennée wrote:
>> +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.


Yes, but all of the other GEN_* macros retain them as well.
Without looking at the actual history, I'd say once upon a time they were
actually used, but we've now split opcode from implementation.

> With the removed extra unused macro params:

> 

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


I wouldn't do that without David's approval.  And possibly stripping out the
opcodes everywhere else.


r~
David Gibson April 22, 2020, 7:58 a.m. UTC | #4 
On Tue, Apr 21, 2020 at 10:50:25AM -0700, Richard Henderson wrote:
> On 4/20/20 3:34 AM, Alex Bennée wrote:

> >> +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.

> 

> Yes, but all of the other GEN_* macros retain them as well.

> Without looking at the actual history, I'd say once upon a time they were

> actually used, but we've now split opcode from implementation.

> 

> > With the removed extra unused macro params:

> > 

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

> 

> I wouldn't do that without David's approval.  And possibly stripping out the

> opcodes everywhere else.


Yeah.  Sounds like one of a nearly infinite number of global cleanups
that the target-ppc tcg code could do with.  Patches welcome.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
diff mbox series

Patch

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);
 
 #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)