Patchwork [V6,15/18] target-ppc: Move To/From VSR Instructions

login
register
mail settings
Submitter Tom Musta
Date Jan. 10, 2014, 7:07 p.m.
Message ID <1389380882-5597-16-git-send-email-tommusta@gmail.com>
Download mbox | patch
Permalink /patch/309382/
State New
Headers show

Comments

Tom Musta - Jan. 10, 2014, 7:07 p.m.
This patch adds the Move To VSR instructions (mfvsrd, mfvsrwz)
and Move From VSR instructions (mtvsrd, mtvsrwa, mtvsrwz).  These
instructions are unusual in that they are considered a floating
point instruction if the indexed VSR is in the first half of the
array (0-31) but they are considered vector instructions if the
indexed VSR is in the second half of the array (32-63).

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
V6: New.

 target-ppc/translate.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)
Richard Henderson - Jan. 10, 2014, 9:29 p.m.
On 01/10/2014 11:07 AM, Tom Musta wrote:
> +#define MV_VSR(name, tcgop1, tcgop2, target, source)            \
> +static void gen_##name(DisasContext *ctx)                       \
> +{                                                               \
> +    if (xS(ctx->opcode) < 32) {                                 \
> +        if (unlikely(!ctx->fpu_enabled)) {                      \
> +            gen_exception(ctx, POWERPC_EXCP_FPU);               \
> +            return;                                             \
> +        }                                                       \
> +    } else {                                                    \
> +        if (unlikely(!ctx->altivec_enabled)) {                  \
> +            gen_exception(ctx, POWERPC_EXCP_VPU);               \
> +            return;                                             \
> +        }                                                       \
> +    }                                                           \
> +    TCGv_i64 tmp = tcg_temp_new_i64();                          \
> +    tcg_gen_##tcgop1(tmp, source);                              \
> +    tcg_gen_##tcgop2(target, tmp);                              \
> +    tcg_temp_free_i64(tmp);                                     \
> +}
> +
> +
> +MV_VSR(mfvsrwz, ext32u_i64, trunc_i64_tl, cpu_gpr[rA(ctx->opcode)], \
> +       cpu_vsrh(xS(ctx->opcode)))
> +MV_VSR(mtvsrwa, extu_tl_i64, ext32s_i64, cpu_vsrh(xT(ctx->opcode)), \
> +       cpu_gpr[rA(ctx->opcode)])
> +MV_VSR(mtvsrwz, extu_tl_i64, ext32u_i64, cpu_vsrh(xT(ctx->opcode)), \
> +       cpu_gpr[rA(ctx->opcode)])
> +#if defined(TARGET_PPC64)
> +MV_VSR(mfvsrd, mov_i64, mov_i64, cpu_gpr[rA(ctx->opcode)], \
> +       cpu_vsrh(xS(ctx->opcode)))
> +MV_VSR(mtvsrd, mov_i64, mov_i64, cpu_vsrh(xT(ctx->opcode)), \
> +       cpu_gpr[rA(ctx->opcode)])
> +#endif

Better to do this in one step:

mfcsrwz:	tcg_gen_ext32u_tl
mtvsrwa:	tcg_gen_ext_tl_i64
mtvsrwz:	tcg_gen_extu_tl_i64
m[tf]vsrd:	tcg_gen_mov_i64


r~
Tom Musta - Jan. 14, 2014, 2:14 p.m.
On 1/10/2014 3:29 PM, Richard Henderson wrote:
> On 01/10/2014 11:07 AM, Tom Musta wrote:
>> +#define MV_VSR(name, tcgop1, tcgop2, target, source)            \
>> +static void gen_##name(DisasContext *ctx)                       \
>> +{                                                               \
>> +    if (xS(ctx->opcode) < 32) {                                 \
>> +        if (unlikely(!ctx->fpu_enabled)) {                      \
>> +            gen_exception(ctx, POWERPC_EXCP_FPU);               \
>> +            return;                                             \
>> +        }                                                       \
>> +    } else {                                                    \
>> +        if (unlikely(!ctx->altivec_enabled)) {                  \
>> +            gen_exception(ctx, POWERPC_EXCP_VPU);               \
>> +            return;                                             \
>> +        }                                                       \
>> +    }                                                           \
>> +    TCGv_i64 tmp = tcg_temp_new_i64();                          \
>> +    tcg_gen_##tcgop1(tmp, source);                              \
>> +    tcg_gen_##tcgop2(target, tmp);                              \
>> +    tcg_temp_free_i64(tmp);                                     \
>> +}
>> +
>> +
>> +MV_VSR(mfvsrwz, ext32u_i64, trunc_i64_tl, cpu_gpr[rA(ctx->opcode)], \
>> +       cpu_vsrh(xS(ctx->opcode)))
>> +MV_VSR(mtvsrwa, extu_tl_i64, ext32s_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +MV_VSR(mtvsrwz, extu_tl_i64, ext32u_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +#if defined(TARGET_PPC64)
>> +MV_VSR(mfvsrd, mov_i64, mov_i64, cpu_gpr[rA(ctx->opcode)], \
>> +       cpu_vsrh(xS(ctx->opcode)))
>> +MV_VSR(mtvsrd, mov_i64, mov_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +#endif
> 
> Better to do this in one step:
> 
> mfcsrwz:	tcg_gen_ext32u_tl
> mtvsrwa:	tcg_gen_ext_tl_i64
> mtvsrwz:	tcg_gen_extu_tl_i64
> m[tf]vsrd:	tcg_gen_mov_i64
> 
> 
> r~
> 

Richard:

As always, thanks for reviewing my patches.

I agree on m[tf]vsrd because these are 64-bit instructions and therefore both
source and target are always i64s.

However, the word versions are a bit more tricky.  The VSR operand is always
an i64.  However, the GPR operand is either an i32 (on 32-bit implementations)
or a part of an i64.  I could not find single TCG operations to handle
these cases.  Specifically, here is what I think doesn't work with your
suggestions:

(1) Using tcg_gen_ext32u_tl for mfvsrwz does not compile on 32-bit PPC -- the
ext32u_tl operation is equivalent to mov_i32 but the source operand (VSRH) is
an i64.

(2) Using tcg_gen_ext_tl_i64 for mtvsrwa compiles but is incorrect on 64-bit PPC
-- the ext_tl_i64 translates to mov_i64.  The instruction semantic is to sign
extend the lower 32 bits of the 64-bit source GPR.

(3) Similarly, using tcg_gen_extu_tl_i64 for mtvsrwz is incorrect for 64-bit
PPC -- this is a mov_i64 and hence does not zero out the upper 32 bits of the
target VSRH.

I can recode the "d" versions to eliminate the extraneous tcg op.
Richard Henderson - Jan. 15, 2014, 9:07 p.m.
On 01/14/2014 06:14 AM, Tom Musta wrote:
> However, the word versions are a bit more tricky.  The VSR operand is always
> an i64.  However, the GPR operand is either an i32 (on 32-bit implementations)
> or a part of an i64.  I could not find single TCG operations to handle
> these cases.  Specifically, here is what I think doesn't work with your
> suggestions:
> 
> (1) Using tcg_gen_ext32u_tl for mfvsrwz does not compile on 32-bit PPC -- the
> ext32u_tl operation is equivalent to mov_i32 but the source operand (VSRH) is
> an i64.
> 
> (2) Using tcg_gen_ext_tl_i64 for mtvsrwa compiles but is incorrect on 64-bit PPC
> -- the ext_tl_i64 translates to mov_i64.  The instruction semantic is to sign
> extend the lower 32 bits of the 64-bit source GPR.
> 
> (3) Similarly, using tcg_gen_extu_tl_i64 for mtvsrwz is incorrect for 64-bit
> PPC -- this is a mov_i64 and hence does not zero out the upper 32 bits of the
> target VSRH.

Hmm, you're right that there's not one symbol that does the job for each case.
However, there's an opcode that does the job for each case.  So perhaps we need

#ifdef TARGET_PPC64
#define ppc_gen_mfcsrwz    tcg_gen_ext32u_i64
#define ppc_gen_mtvsrwa    tcg_gen_ext32s_i64
#define ppc_gen_mtvsrwz    tcg_gen_ext32u_i64
#else
#define ppc_gen_mfcsrwz    tcg_gen_trunc_i64_i32
#define ppc_gen_mtvsrwa    tcg_gen_ext_i32_i64
#define ppc_gen_mtvsrwz    tcg_gen_extu_i32_i64
#endif

and then use those within your other macros.


r~

Patch

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index e2dd272..ec945a2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7175,6 +7175,40 @@  static void gen_stxvw4x(DisasContext *ctx)
     tcg_temp_free_i64(tmp);
 }
 
+#define MV_VSR(name, tcgop1, tcgop2, target, source)            \
+static void gen_##name(DisasContext *ctx)                       \
+{                                                               \
+    if (xS(ctx->opcode) < 32) {                                 \
+        if (unlikely(!ctx->fpu_enabled)) {                      \
+            gen_exception(ctx, POWERPC_EXCP_FPU);               \
+            return;                                             \
+        }                                                       \
+    } else {                                                    \
+        if (unlikely(!ctx->altivec_enabled)) {                  \
+            gen_exception(ctx, POWERPC_EXCP_VPU);               \
+            return;                                             \
+        }                                                       \
+    }                                                           \
+    TCGv_i64 tmp = tcg_temp_new_i64();                          \
+    tcg_gen_##tcgop1(tmp, source);                              \
+    tcg_gen_##tcgop2(target, tmp);                              \
+    tcg_temp_free_i64(tmp);                                     \
+}
+
+
+MV_VSR(mfvsrwz, ext32u_i64, trunc_i64_tl, cpu_gpr[rA(ctx->opcode)], \
+       cpu_vsrh(xS(ctx->opcode)))
+MV_VSR(mtvsrwa, extu_tl_i64, ext32s_i64, cpu_vsrh(xT(ctx->opcode)), \
+       cpu_gpr[rA(ctx->opcode)])
+MV_VSR(mtvsrwz, extu_tl_i64, ext32u_i64, cpu_vsrh(xT(ctx->opcode)), \
+       cpu_gpr[rA(ctx->opcode)])
+#if defined(TARGET_PPC64)
+MV_VSR(mfvsrd, mov_i64, mov_i64, cpu_gpr[rA(ctx->opcode)], \
+       cpu_vsrh(xS(ctx->opcode)))
+MV_VSR(mtvsrd, mov_i64, mov_i64, cpu_vsrh(xT(ctx->opcode)), \
+       cpu_gpr[rA(ctx->opcode)])
+#endif
+
 static void gen_xxpermdi(DisasContext *ctx)
 {
     if (unlikely(!ctx->vsx_enabled)) {
@@ -10094,6 +10128,14 @@  GEN_HANDLER_E(stxsspx, 0x1F, 0xC, 0x14, 0, PPC_NONE, PPC2_VSX207),
 GEN_HANDLER_E(stxvd2x, 0x1F, 0xC, 0x1E, 0, PPC_NONE, PPC2_VSX),
 GEN_HANDLER_E(stxvw4x, 0x1F, 0xC, 0x1C, 0, PPC_NONE, PPC2_VSX),
 
+GEN_HANDLER_E(mfvsrwz, 0x1F, 0x13, 0x03, 0x0000F800, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(mtvsrwa, 0x1F, 0x13, 0x06, 0x0000F800, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(mtvsrwz, 0x1F, 0x13, 0x07, 0x0000F800, PPC_NONE, PPC2_VSX207),
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E(mfvsrd, 0x1F, 0x13, 0x01, 0x0000F800, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(mtvsrd, 0x1F, 0x13, 0x05, 0x0000F800, PPC_NONE, PPC2_VSX207),
+#endif
+
 #undef GEN_XX2FORM
 #define GEN_XX2FORM(name, opc2, opc3, fl2)                           \
 GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 0, PPC_NONE, fl2), \