* [PATCH v2 0/4] Move VSX storage access and compare insns to
@ 2024-06-13 9:33 Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 1/4] target/ppc: Moving VSX scalar storage access insns to decodetree Chinmay Rath
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-13 9:33 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, richard.henderson, harshpb
Moving all remaining VSX storage access instructions and all VSX compare
instructions of XX3 form with RC field, to decodetree specification.
Change log :
v2:
- Addressed comments by Richard in v1
- Patch 2/4 : Handled proper ea calculation in narrow mode.
Also created a new function for ea calculation instead of inlining,
for later use by (p){lx,stx}vp insns.
- Patch 4/4 : Unified helper calls.
- Retained Richard's "Reviewed-by" in patches 1, 3 and 4.
v1: https://lore.kernel.org/qemu-devel/20240607144921.726730-1-rathc@linux.ibm.com/
Chinmay Rath (4):
target/ppc: Moving VSX scalar storage access insns to decodetree.
target/ppc: Move VSX vector with length storage access insns to
decodetree.
target/ppc: Move VSX vector storage access insns to decodetree.
target/ppc: Move VSX fp compare insns to decodetree.
target/ppc/helper.h | 24 +-
target/ppc/insn32.decode | 41 +++
target/ppc/fpu_helper.c | 16 +-
target/ppc/mem_helper.c | 8 +-
target/ppc/translate.c | 18 ++
target/ppc/translate/vsx-impl.c.inc | 416 ++++++++++++++--------------
target/ppc/translate/vsx-ops.c.inc | 49 ----
7 files changed, 290 insertions(+), 282 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] target/ppc: Moving VSX scalar storage access insns to decodetree.
2024-06-13 9:33 [PATCH v2 0/4] Move VSX storage access and compare insns to Chinmay Rath
@ 2024-06-13 9:33 ` Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 2/4] target/ppc: Move VSX vector with length " Chinmay Rath
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-13 9:33 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, richard.henderson, harshpb
Moving the following instructions to decodetree specification :
lxs{d, iwa, ibz, ihz, iwz, sp}x : X-form
stxs{d, ib, ih, iw, sp}x : X-form
The changes were verified by validating that the tcg-ops generated by those
instructions remain the same, which were captured using the '-d in_asm,op' flag.
Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 13 +++++
target/ppc/translate/vsx-impl.c.inc | 79 +++++++++++++----------------
target/ppc/translate/vsx-ops.c.inc | 11 ----
3 files changed, 49 insertions(+), 54 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 30d6f9f750..88753c75e1 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -792,6 +792,19 @@ STXVRHX 011111 ..... ..... ..... 0010101101 . @X_TSX
STXVRWX 011111 ..... ..... ..... 0011001101 . @X_TSX
STXVRDX 011111 ..... ..... ..... 0011101101 . @X_TSX
+LXSDX 011111 ..... ..... ..... 1001001100 . @X_TSX
+LXSIWAX 011111 ..... ..... ..... 0001001100 . @X_TSX
+LXSIBZX 011111 ..... ..... ..... 1100001101 . @X_TSX
+LXSIHZX 011111 ..... ..... ..... 1100101101 . @X_TSX
+LXSIWZX 011111 ..... ..... ..... 0000001100 . @X_TSX
+LXSSPX 011111 ..... ..... ..... 1000001100 . @X_TSX
+
+STXSDX 011111 ..... ..... ..... 1011001100 . @X_TSX
+STXSIBX 011111 ..... ..... ..... 1110001101 . @X_TSX
+STXSIHX 011111 ..... ..... ..... 1110101101 . @X_TSX
+STXSIWX 011111 ..... ..... ..... 0010001100 . @X_TSX
+STXSSPX 011111 ..... ..... ..... 1010001100 . @X_TSX
+
## VSX Vector Binary Floating-Point Sign Manipulation Instructions
XVABSDP 111100 ..... 00000 ..... 111011001 .. @XX2
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index a769f199ce..de2a26a213 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -24,30 +24,27 @@ static inline TCGv_ptr gen_acc_ptr(int reg)
return r;
}
-#define VSX_LOAD_SCALAR(name, operation) \
-static void gen_##name(DisasContext *ctx) \
-{ \
- TCGv EA; \
- TCGv_i64 t0; \
- if (unlikely(!ctx->vsx_enabled)) { \
- gen_exception(ctx, POWERPC_EXCP_VSXU); \
- return; \
- } \
- t0 = tcg_temp_new_i64(); \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_reg_index(ctx, EA); \
- gen_qemu_##operation(ctx, t0, EA); \
- set_cpu_vsr(xT(ctx->opcode), t0, true); \
- /* NOTE: cpu_vsrl is undefined */ \
+static bool do_lxs(DisasContext *ctx, arg_X *a,
+ void (*op)(DisasContext *, TCGv_i64, TCGv))
+{
+ TCGv EA;
+ TCGv_i64 t0;
+ REQUIRE_VSX(ctx);
+ t0 = tcg_temp_new_i64();
+ gen_set_access_type(ctx, ACCESS_INT);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
+ op(ctx, t0, EA);
+ set_cpu_vsr(a->rt, t0, true);
+ /* NOTE: cpu_vsrl is undefined */
+ return true;
}
-VSX_LOAD_SCALAR(lxsdx, ld64_i64)
-VSX_LOAD_SCALAR(lxsiwax, ld32s_i64)
-VSX_LOAD_SCALAR(lxsibzx, ld8u_i64)
-VSX_LOAD_SCALAR(lxsihzx, ld16u_i64)
-VSX_LOAD_SCALAR(lxsiwzx, ld32u_i64)
-VSX_LOAD_SCALAR(lxsspx, ld32fs)
+TRANS_FLAGS2(VSX, LXSDX, do_lxs, gen_qemu_ld64_i64);
+TRANS_FLAGS2(VSX207, LXSIWAX, do_lxs, gen_qemu_ld32s_i64);
+TRANS_FLAGS2(ISA300, LXSIBZX, do_lxs, gen_qemu_ld8u_i64);
+TRANS_FLAGS2(ISA300, LXSIHZX, do_lxs, gen_qemu_ld16u_i64);
+TRANS_FLAGS2(VSX207, LXSIWZX, do_lxs, gen_qemu_ld32u_i64);
+TRANS_FLAGS2(VSX207, LXSSPX, do_lxs, gen_qemu_ld32fs);
static void gen_lxvd2x(DisasContext *ctx)
{
@@ -266,29 +263,25 @@ VSX_VECTOR_LOAD_STORE_LENGTH(stxvl)
VSX_VECTOR_LOAD_STORE_LENGTH(stxvll)
#endif
-#define VSX_STORE_SCALAR(name, operation) \
-static void gen_##name(DisasContext *ctx) \
-{ \
- TCGv EA; \
- TCGv_i64 t0; \
- if (unlikely(!ctx->vsx_enabled)) { \
- gen_exception(ctx, POWERPC_EXCP_VSXU); \
- return; \
- } \
- t0 = tcg_temp_new_i64(); \
- gen_set_access_type(ctx, ACCESS_INT); \
- EA = tcg_temp_new(); \
- gen_addr_reg_index(ctx, EA); \
- get_cpu_vsr(t0, xS(ctx->opcode), true); \
- gen_qemu_##operation(ctx, t0, EA); \
+static bool do_stxs(DisasContext *ctx, arg_X *a,
+ void (*op)(DisasContext *, TCGv_i64, TCGv))
+{
+ TCGv EA;
+ TCGv_i64 t0;
+ REQUIRE_VSX(ctx);
+ t0 = tcg_temp_new_i64();
+ gen_set_access_type(ctx, ACCESS_INT);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
+ get_cpu_vsr(t0, a->rt, true);
+ op(ctx, t0, EA);
+ return true;
}
-VSX_STORE_SCALAR(stxsdx, st64_i64)
-
-VSX_STORE_SCALAR(stxsibx, st8_i64)
-VSX_STORE_SCALAR(stxsihx, st16_i64)
-VSX_STORE_SCALAR(stxsiwx, st32_i64)
-VSX_STORE_SCALAR(stxsspx, st32fs)
+TRANS_FLAGS2(VSX, STXSDX, do_stxs, gen_qemu_st64_i64);
+TRANS_FLAGS2(ISA300, STXSIBX, do_stxs, gen_qemu_st8_i64);
+TRANS_FLAGS2(ISA300, STXSIHX, do_stxs, gen_qemu_st16_i64);
+TRANS_FLAGS2(VSX207, STXSIWX, do_stxs, gen_qemu_st32_i64);
+TRANS_FLAGS2(VSX207, STXSSPX, do_stxs, gen_qemu_st32fs);
static void gen_stxvd2x(DisasContext *ctx)
{
diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc
index 3c0a70cb7c..d44cb55836 100644
--- a/target/ppc/translate/vsx-ops.c.inc
+++ b/target/ppc/translate/vsx-ops.c.inc
@@ -1,9 +1,3 @@
-GEN_HANDLER_E(lxsdx, 0x1F, 0x0C, 0x12, 0, PPC_NONE, PPC2_VSX),
-GEN_HANDLER_E(lxsiwax, 0x1F, 0x0C, 0x02, 0, PPC_NONE, PPC2_VSX207),
-GEN_HANDLER_E(lxsiwzx, 0x1F, 0x0C, 0x00, 0, PPC_NONE, PPC2_VSX207),
-GEN_HANDLER_E(lxsibzx, 0x1F, 0x0D, 0x18, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(lxsihzx, 0x1F, 0x0D, 0x19, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(lxsspx, 0x1F, 0x0C, 0x10, 0, PPC_NONE, PPC2_VSX207),
GEN_HANDLER_E(lxvd2x, 0x1F, 0x0C, 0x1A, 0, PPC_NONE, PPC2_VSX),
GEN_HANDLER_E(lxvwsx, 0x1F, 0x0C, 0x0B, 0, PPC_NONE, PPC2_ISA300),
GEN_HANDLER_E(lxvdsx, 0x1F, 0x0C, 0x0A, 0, PPC_NONE, PPC2_VSX),
@@ -15,11 +9,6 @@ GEN_HANDLER_E(lxvl, 0x1F, 0x0D, 0x08, 0, PPC_NONE, PPC2_ISA300),
GEN_HANDLER_E(lxvll, 0x1F, 0x0D, 0x09, 0, PPC_NONE, PPC2_ISA300),
#endif
-GEN_HANDLER_E(stxsdx, 0x1F, 0xC, 0x16, 0, PPC_NONE, PPC2_VSX),
-GEN_HANDLER_E(stxsibx, 0x1F, 0xD, 0x1C, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(stxsihx, 0x1F, 0xD, 0x1D, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(stxsiwx, 0x1F, 0xC, 0x04, 0, PPC_NONE, PPC2_VSX207),
-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(stxvh8x, 0x1F, 0x0C, 0x1D, 0, PPC_NONE, PPC2_ISA300),
--
2.39.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-13 9:33 [PATCH v2 0/4] Move VSX storage access and compare insns to Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 1/4] target/ppc: Moving VSX scalar storage access insns to decodetree Chinmay Rath
@ 2024-06-13 9:33 ` Chinmay Rath
2024-06-16 19:13 ` Richard Henderson
2024-06-13 9:33 ` [PATCH v2 3/4] target/ppc: Move VSX vector " Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 4/4] target/ppc: Move VSX fp compare " Chinmay Rath
3 siblings, 1 reply; 12+ messages in thread
From: Chinmay Rath @ 2024-06-13 9:33 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, richard.henderson, harshpb
Moving the following instructions to decodetree specification :
{l, st}xvl(l) : X-form
The changes were verified by validating that the tcg-ops generated by those
instructions remain the same, which were captured using the '-d in_asm,op' flag.
Also added a new function to calculate the effective address :
EA <- (RA == 0) ? 0 : GPR[RA], which is now used by the above-said insns,
and shall be used later by (p){lx, stx}vp insns.
Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
target/ppc/helper.h | 8 +--
target/ppc/insn32.decode | 6 ++
target/ppc/mem_helper.c | 8 +--
target/ppc/translate.c | 18 ++++++
target/ppc/translate/vsx-impl.c.inc | 94 ++++++++++++++++++++---------
target/ppc/translate/vsx-ops.c.inc | 8 ---
6 files changed, 97 insertions(+), 45 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 3b4a0c4674..510ce76524 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -274,10 +274,10 @@ DEF_HELPER_3(stvebx, void, env, avr, tl)
DEF_HELPER_3(stvehx, void, env, avr, tl)
DEF_HELPER_3(stvewx, void, env, avr, tl)
#if defined(TARGET_PPC64)
-DEF_HELPER_4(lxvl, void, env, tl, vsr, tl)
-DEF_HELPER_4(lxvll, void, env, tl, vsr, tl)
-DEF_HELPER_4(stxvl, void, env, tl, vsr, tl)
-DEF_HELPER_4(stxvll, void, env, tl, vsr, tl)
+DEF_HELPER_4(LXVL, void, env, tl, vsr, tl)
+DEF_HELPER_4(LXVLL, void, env, tl, vsr, tl)
+DEF_HELPER_4(STXVL, void, env, tl, vsr, tl)
+DEF_HELPER_4(STXVLL, void, env, tl, vsr, tl)
#endif
DEF_HELPER_4(vsumsws, void, env, avr, avr, avr)
DEF_HELPER_4(vsum2sws, void, env, avr, avr, avr)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 88753c75e1..445fdb341f 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -805,6 +805,12 @@ STXSIHX 011111 ..... ..... ..... 1110101101 . @X_TSX
STXSIWX 011111 ..... ..... ..... 0010001100 . @X_TSX
STXSSPX 011111 ..... ..... ..... 1010001100 . @X_TSX
+LXVL 011111 ..... ..... ..... 0100001101 . @X_TSX
+LXVLL 011111 ..... ..... ..... 0100101101 . @X_TSX
+
+STXVL 011111 ..... ..... ..... 0110001101 . @X_TSX
+STXVLL 011111 ..... ..... ..... 0110101101 . @X_TSX
+
## VSX Vector Binary Floating-Point Sign Manipulation Instructions
XVABSDP 111100 ..... 00000 ..... 111011001 .. @XX2
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index ea7e8443a8..dec1b25eb8 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -467,8 +467,8 @@ void helper_##name(CPUPPCState *env, target_ulong addr, \
*xt = t; \
}
-VSX_LXVL(lxvl, 0)
-VSX_LXVL(lxvll, 1)
+VSX_LXVL(LXVL, 0)
+VSX_LXVL(LXVLL, 1)
#undef VSX_LXVL
#define VSX_STXVL(name, lj) \
@@ -496,8 +496,8 @@ void helper_##name(CPUPPCState *env, target_ulong addr, \
} \
}
-VSX_STXVL(stxvl, 0)
-VSX_STXVL(stxvll, 1)
+VSX_STXVL(STXVL, 0)
+VSX_STXVL(STXVLL, 1)
#undef VSX_STXVL
#undef GET_NB
#endif /* TARGET_PPC64 */
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 93ffec787c..a1f2f4fbda 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3096,6 +3096,7 @@ static inline void gen_align_no_le(DisasContext *ctx)
(ctx->opcode & 0x03FF0000) | POWERPC_EXCP_ALIGN_LE);
}
+/* EA <- {(ra == 0) ? 0 : GPR[ra]} + displ */
static TCGv do_ea_calc(DisasContext *ctx, int ra, TCGv displ)
{
TCGv ea = tcg_temp_new();
@@ -3110,6 +3111,23 @@ static TCGv do_ea_calc(DisasContext *ctx, int ra, TCGv displ)
return ea;
}
+/* EA <- (ra == 0) ? 0 : GPR[ra] */
+static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
+{
+ TCGv EA;
+ if (!ra) {
+ EA = tcg_constant_tl(0);
+ return EA;
+ }
+ EA = tcg_temp_new();
+ if (NARROW_MODE(ctx)) {
+ tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
+ } else {
+ tcg_gen_mov_tl(EA, cpu_gpr[ra]);
+ }
+ return EA;
+}
+
/*** Integer load ***/
#define DEF_MEMOP(op) ((op) | ctx->default_tcg_memop_mask)
#define BSWAP_MEMOP(op) ((op) | (ctx->default_tcg_memop_mask ^ MO_BSWAP))
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index de2a26a213..46bab49215 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -232,36 +232,72 @@ static void gen_lxvb16x(DisasContext *ctx)
set_cpu_vsr(xT(ctx->opcode), xtl, false);
}
-#ifdef TARGET_PPC64
-#define VSX_VECTOR_LOAD_STORE_LENGTH(name) \
-static void gen_##name(DisasContext *ctx) \
-{ \
- TCGv EA; \
- TCGv_ptr xt; \
- \
- if (xT(ctx->opcode) < 32) { \
- if (unlikely(!ctx->vsx_enabled)) { \
- gen_exception(ctx, POWERPC_EXCP_VSXU); \
- return; \
- } \
- } else { \
- if (unlikely(!ctx->altivec_enabled)) { \
- gen_exception(ctx, POWERPC_EXCP_VPU); \
- return; \
- } \
- } \
- EA = tcg_temp_new(); \
- xt = gen_vsr_ptr(xT(ctx->opcode)); \
- gen_set_access_type(ctx, ACCESS_INT); \
- gen_addr_register(ctx, EA); \
- gen_helper_##name(tcg_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
-}
-
-VSX_VECTOR_LOAD_STORE_LENGTH(lxvl)
-VSX_VECTOR_LOAD_STORE_LENGTH(lxvll)
-VSX_VECTOR_LOAD_STORE_LENGTH(stxvl)
-VSX_VECTOR_LOAD_STORE_LENGTH(stxvll)
+#if defined(TARGET_PPC64)
+static bool do_ld_st_vl(DisasContext *ctx, arg_X *a,
+ void (*helper)(TCGv_ptr, TCGv, TCGv_ptr, TCGv))
+{
+ TCGv EA;
+ TCGv_ptr xt;
+ if (a->rt < 32) {
+ REQUIRE_VSX(ctx);
+ } else {
+ REQUIRE_VECTOR(ctx);
+ }
+ xt = gen_vsr_ptr(a->rt);
+ gen_set_access_type(ctx, ACCESS_INT);
+ EA = do_ea_calc_ra(ctx, a->ra);
+ helper(tcg_env, EA, xt, cpu_gpr[a->rb]);
+ return true;
+}
+#endif
+
+static bool trans_LXVL(DisasContext *ctx, arg_LXVL *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+#if defined(TARGET_PPC64)
+ return do_ld_st_vl(ctx, a, gen_helper_LXVL);
+#else
+ qemu_build_not_reached();
#endif
+ return true;
+}
+
+static bool trans_LXVLL(DisasContext *ctx, arg_LXVLL *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+#if defined(TARGET_PPC64)
+ return do_ld_st_vl(ctx, a, gen_helper_LXVLL);
+#else
+ qemu_build_not_reached();
+#endif
+ return true;
+}
+
+static bool trans_STXVL(DisasContext *ctx, arg_STXVL *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+#if defined(TARGET_PPC64)
+ return do_ld_st_vl(ctx, a, gen_helper_STXVL);
+#else
+ qemu_build_not_reached();
+#endif
+ return true;
+}
+
+static bool trans_STXVLL(DisasContext *ctx, arg_STXVLL *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+#if defined(TARGET_PPC64)
+ return do_ld_st_vl(ctx, a, gen_helper_STXVLL);
+#else
+ qemu_build_not_reached();
+#endif
+ return true;
+}
static bool do_stxs(DisasContext *ctx, arg_X *a,
void (*op)(DisasContext *, TCGv_i64, TCGv))
diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc
index d44cb55836..7f4326c974 100644
--- a/target/ppc/translate/vsx-ops.c.inc
+++ b/target/ppc/translate/vsx-ops.c.inc
@@ -4,19 +4,11 @@ GEN_HANDLER_E(lxvdsx, 0x1F, 0x0C, 0x0A, 0, PPC_NONE, PPC2_VSX),
GEN_HANDLER_E(lxvw4x, 0x1F, 0x0C, 0x18, 0, PPC_NONE, PPC2_VSX),
GEN_HANDLER_E(lxvh8x, 0x1F, 0x0C, 0x19, 0, PPC_NONE, PPC2_ISA300),
GEN_HANDLER_E(lxvb16x, 0x1F, 0x0C, 0x1B, 0, PPC_NONE, PPC2_ISA300),
-#if defined(TARGET_PPC64)
-GEN_HANDLER_E(lxvl, 0x1F, 0x0D, 0x08, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(lxvll, 0x1F, 0x0D, 0x09, 0, PPC_NONE, PPC2_ISA300),
-#endif
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(stxvh8x, 0x1F, 0x0C, 0x1D, 0, PPC_NONE, PPC2_ISA300),
GEN_HANDLER_E(stxvb16x, 0x1F, 0x0C, 0x1F, 0, PPC_NONE, PPC2_ISA300),
-#if defined(TARGET_PPC64)
-GEN_HANDLER_E(stxvl, 0x1F, 0x0D, 0x0C, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(stxvll, 0x1F, 0x0D, 0x0D, 0, PPC_NONE, PPC2_ISA300),
-#endif
GEN_HANDLER_E(mfvsrwz, 0x1F, 0x13, 0x03, 0x0000F800, PPC_NONE, PPC2_VSX207),
GEN_HANDLER_E(mtvsrwa, 0x1F, 0x13, 0x06, 0x0000F800, PPC_NONE, PPC2_VSX207),
--
2.39.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] target/ppc: Move VSX vector storage access insns to decodetree.
2024-06-13 9:33 [PATCH v2 0/4] Move VSX storage access and compare insns to Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 1/4] target/ppc: Moving VSX scalar storage access insns to decodetree Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 2/4] target/ppc: Move VSX vector with length " Chinmay Rath
@ 2024-06-13 9:33 ` Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 4/4] target/ppc: Move VSX fp compare " Chinmay Rath
3 siblings, 0 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-13 9:33 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, richard.henderson, harshpb
Moving the following instructions to decodetree specification:
lxv{b16, d2, h8, w4, ds, ws}x : X-form
stxv{b16, d2, h8, w4}x : X-form
The changes were verified by validating that the tcg-ops generated for those
instructions remain the same, which were captured using the '-d in_asm,op' flag.
Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/insn32.decode | 10 ++
target/ppc/translate/vsx-impl.c.inc | 199 ++++++++++++----------------
target/ppc/translate/vsx-ops.c.inc | 12 --
3 files changed, 97 insertions(+), 124 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 445fdb341f..3d31ef52f8 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -805,9 +805,19 @@ STXSIHX 011111 ..... ..... ..... 1110101101 . @X_TSX
STXSIWX 011111 ..... ..... ..... 0010001100 . @X_TSX
STXSSPX 011111 ..... ..... ..... 1010001100 . @X_TSX
+LXVB16X 011111 ..... ..... ..... 1101101100 . @X_TSX
+LXVD2X 011111 ..... ..... ..... 1101001100 . @X_TSX
+LXVH8X 011111 ..... ..... ..... 1100101100 . @X_TSX
+LXVW4X 011111 ..... ..... ..... 1100001100 . @X_TSX
+LXVDSX 011111 ..... ..... ..... 0101001100 . @X_TSX
+LXVWSX 011111 ..... ..... ..... 0101101100 . @X_TSX
LXVL 011111 ..... ..... ..... 0100001101 . @X_TSX
LXVLL 011111 ..... ..... ..... 0100101101 . @X_TSX
+STXVB16X 011111 ..... ..... ..... 1111101100 . @X_TSX
+STXVD2X 011111 ..... ..... ..... 1111001100 . @X_TSX
+STXVH8X 011111 ..... ..... ..... 1110101100 . @X_TSX
+STXVW4X 011111 ..... ..... ..... 1110001100 . @X_TSX
STXVL 011111 ..... ..... ..... 0110001101 . @X_TSX
STXVLL 011111 ..... ..... ..... 0110101101 . @X_TSX
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index 46bab49215..e0fb4bad92 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -46,41 +46,37 @@ TRANS_FLAGS2(ISA300, LXSIHZX, do_lxs, gen_qemu_ld16u_i64);
TRANS_FLAGS2(VSX207, LXSIWZX, do_lxs, gen_qemu_ld32u_i64);
TRANS_FLAGS2(VSX207, LXSSPX, do_lxs, gen_qemu_ld32fs);
-static void gen_lxvd2x(DisasContext *ctx)
+static bool trans_LXVD2X(DisasContext *ctx, arg_LXVD2X *a)
{
TCGv EA;
TCGv_i64 t0;
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, VSX);
+
t0 = tcg_temp_new_i64();
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
gen_qemu_ld64_i64(ctx, t0, EA);
- set_cpu_vsr(xT(ctx->opcode), t0, true);
+ set_cpu_vsr(a->rt, t0, true);
tcg_gen_addi_tl(EA, EA, 8);
gen_qemu_ld64_i64(ctx, t0, EA);
- set_cpu_vsr(xT(ctx->opcode), t0, false);
+ set_cpu_vsr(a->rt, t0, false);
+ return true;
}
-static void gen_lxvw4x(DisasContext *ctx)
+static bool trans_LXVW4X(DisasContext *ctx, arg_LXVW4X *a)
{
TCGv EA;
- TCGv_i64 xth;
- TCGv_i64 xtl;
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
+ TCGv_i64 xth, xtl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, VSX);
+
xth = tcg_temp_new_i64();
xtl = tcg_temp_new_i64();
-
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
-
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
if (ctx->le_mode) {
TCGv_i64 t0 = tcg_temp_new_i64();
TCGv_i64 t1 = tcg_temp_new_i64();
@@ -97,55 +93,45 @@ static void gen_lxvw4x(DisasContext *ctx)
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEUQ);
}
- set_cpu_vsr(xT(ctx->opcode), xth, true);
- set_cpu_vsr(xT(ctx->opcode), xtl, false);
+ set_cpu_vsr(a->rt, xth, true);
+ set_cpu_vsr(a->rt, xtl, false);
+ return true;
}
-static void gen_lxvwsx(DisasContext *ctx)
+static bool trans_LXVWSX(DisasContext *ctx, arg_LXVWSX *a)
{
TCGv EA;
TCGv_i32 data;
- if (xT(ctx->opcode) < 32) {
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
+ if (a->rt < 32) {
+ REQUIRE_VSX(ctx);
} else {
- if (unlikely(!ctx->altivec_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VPU);
- return;
- }
+ REQUIRE_VECTOR(ctx);
}
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
-
- gen_addr_reg_index(ctx, EA);
-
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
data = tcg_temp_new_i32();
tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UL));
- tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
+ tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(a->rt), 16, 16, data);
+ return true;
}
-static void gen_lxvdsx(DisasContext *ctx)
+static bool trans_LXVDSX(DisasContext *ctx, arg_LXVDSX *a)
{
TCGv EA;
TCGv_i64 data;
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, VSX);
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
-
- gen_addr_reg_index(ctx, EA);
-
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
data = tcg_temp_new_i64();
tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UQ));
- tcg_gen_gvec_dup_i64(MO_UQ, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
+ tcg_gen_gvec_dup_i64(MO_UQ, vsr_full_offset(a->rt), 16, 16, data);
+ return true;
}
static void gen_bswap16x8(TCGv_i64 outh, TCGv_i64 outl,
@@ -184,52 +170,47 @@ static void gen_bswap32x4(TCGv_i64 outh, TCGv_i64 outl,
tcg_gen_deposit_i64(outl, outl, lo, 32, 32);
}
-static void gen_lxvh8x(DisasContext *ctx)
+static bool trans_LXVH8X(DisasContext *ctx, arg_LXVH8X *a)
{
TCGv EA;
- TCGv_i64 xth;
- TCGv_i64 xtl;
+ TCGv_i64 xth, xtl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
xth = tcg_temp_new_i64();
xtl = tcg_temp_new_i64();
gen_set_access_type(ctx, ACCESS_INT);
-
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEUQ);
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEUQ);
if (ctx->le_mode) {
gen_bswap16x8(xth, xtl, xth, xtl);
}
- set_cpu_vsr(xT(ctx->opcode), xth, true);
- set_cpu_vsr(xT(ctx->opcode), xtl, false);
+ set_cpu_vsr(a->rt, xth, true);
+ set_cpu_vsr(a->rt, xtl, false);
+ return true;
}
-static void gen_lxvb16x(DisasContext *ctx)
+static bool trans_LXVB16X(DisasContext *ctx, arg_LXVB16X *a)
{
TCGv EA;
- TCGv_i64 xth;
- TCGv_i64 xtl;
+ TCGv_i64 xth, xtl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
xth = tcg_temp_new_i64();
xtl = tcg_temp_new_i64();
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEUQ);
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEUQ);
- set_cpu_vsr(xT(ctx->opcode), xth, true);
- set_cpu_vsr(xT(ctx->opcode), xtl, false);
+ set_cpu_vsr(a->rt, xth, true);
+ set_cpu_vsr(a->rt, xtl, false);
+ return true;
}
#if defined(TARGET_PPC64)
@@ -319,42 +300,39 @@ TRANS_FLAGS2(ISA300, STXSIHX, do_stxs, gen_qemu_st16_i64);
TRANS_FLAGS2(VSX207, STXSIWX, do_stxs, gen_qemu_st32_i64);
TRANS_FLAGS2(VSX207, STXSSPX, do_stxs, gen_qemu_st32fs);
-static void gen_stxvd2x(DisasContext *ctx)
+static bool trans_STXVD2X(DisasContext *ctx, arg_STXVD2X *a)
{
TCGv EA;
TCGv_i64 t0;
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, VSX);
+
t0 = tcg_temp_new_i64();
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
- get_cpu_vsr(t0, xS(ctx->opcode), true);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
+ get_cpu_vsr(t0, a->rt, true);
gen_qemu_st64_i64(ctx, t0, EA);
tcg_gen_addi_tl(EA, EA, 8);
- get_cpu_vsr(t0, xS(ctx->opcode), false);
+ get_cpu_vsr(t0, a->rt, false);
gen_qemu_st64_i64(ctx, t0, EA);
+ return true;
}
-static void gen_stxvw4x(DisasContext *ctx)
+static bool trans_STXVW4X(DisasContext *ctx, arg_STXVW4X *a)
{
TCGv EA;
- TCGv_i64 xsh;
- TCGv_i64 xsl;
+ TCGv_i64 xsh, xsl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, VSX);
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
xsh = tcg_temp_new_i64();
xsl = tcg_temp_new_i64();
- get_cpu_vsr(xsh, xS(ctx->opcode), true);
- get_cpu_vsr(xsl, xS(ctx->opcode), false);
+ get_cpu_vsr(xsh, a->rt, true);
+ get_cpu_vsr(xsl, a->rt, false);
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
if (ctx->le_mode) {
TCGv_i64 t0 = tcg_temp_new_i64();
TCGv_i64 t1 = tcg_temp_new_i64();
@@ -371,25 +349,23 @@ static void gen_stxvw4x(DisasContext *ctx)
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_st_i64(xsl, EA, ctx->mem_idx, MO_BEUQ);
}
+ return true;
}
-static void gen_stxvh8x(DisasContext *ctx)
+static bool trans_STXVH8X(DisasContext *ctx, arg_STXVH8X *a)
{
TCGv EA;
- TCGv_i64 xsh;
- TCGv_i64 xsl;
+ TCGv_i64 xsh, xsl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
xsh = tcg_temp_new_i64();
xsl = tcg_temp_new_i64();
- get_cpu_vsr(xsh, xS(ctx->opcode), true);
- get_cpu_vsr(xsl, xS(ctx->opcode), false);
+ get_cpu_vsr(xsh, a->rt, true);
+ get_cpu_vsr(xsl, a->rt, false);
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
if (ctx->le_mode) {
TCGv_i64 outh = tcg_temp_new_i64();
TCGv_i64 outl = tcg_temp_new_i64();
@@ -403,28 +379,27 @@ static void gen_stxvh8x(DisasContext *ctx)
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_st_i64(xsl, EA, ctx->mem_idx, MO_BEUQ);
}
+ return true;
}
-static void gen_stxvb16x(DisasContext *ctx)
+static bool trans_STXVB16X(DisasContext *ctx, arg_STXVB16X *a)
{
TCGv EA;
- TCGv_i64 xsh;
- TCGv_i64 xsl;
+ TCGv_i64 xsh, xsl;
+
+ REQUIRE_VSX(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
- if (unlikely(!ctx->vsx_enabled)) {
- gen_exception(ctx, POWERPC_EXCP_VSXU);
- return;
- }
xsh = tcg_temp_new_i64();
xsl = tcg_temp_new_i64();
- get_cpu_vsr(xsh, xS(ctx->opcode), true);
- get_cpu_vsr(xsl, xS(ctx->opcode), false);
+ get_cpu_vsr(xsh, a->rt, true);
+ get_cpu_vsr(xsl, a->rt, false);
gen_set_access_type(ctx, ACCESS_INT);
- EA = tcg_temp_new();
- gen_addr_reg_index(ctx, EA);
+ EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
tcg_gen_qemu_st_i64(xsh, EA, ctx->mem_idx, MO_BEUQ);
tcg_gen_addi_tl(EA, EA, 8);
tcg_gen_qemu_st_i64(xsl, EA, ctx->mem_idx, MO_BEUQ);
+ return true;
}
static void gen_mfvsrwz(DisasContext *ctx)
diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc
index 7f4326c974..91cde088bc 100644
--- a/target/ppc/translate/vsx-ops.c.inc
+++ b/target/ppc/translate/vsx-ops.c.inc
@@ -1,15 +1,3 @@
-GEN_HANDLER_E(lxvd2x, 0x1F, 0x0C, 0x1A, 0, PPC_NONE, PPC2_VSX),
-GEN_HANDLER_E(lxvwsx, 0x1F, 0x0C, 0x0B, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(lxvdsx, 0x1F, 0x0C, 0x0A, 0, PPC_NONE, PPC2_VSX),
-GEN_HANDLER_E(lxvw4x, 0x1F, 0x0C, 0x18, 0, PPC_NONE, PPC2_VSX),
-GEN_HANDLER_E(lxvh8x, 0x1F, 0x0C, 0x19, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(lxvb16x, 0x1F, 0x0C, 0x1B, 0, PPC_NONE, PPC2_ISA300),
-
-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(stxvh8x, 0x1F, 0x0C, 0x1D, 0, PPC_NONE, PPC2_ISA300),
-GEN_HANDLER_E(stxvb16x, 0x1F, 0x0C, 0x1F, 0, PPC_NONE, PPC2_ISA300),
-
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),
--
2.39.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] target/ppc: Move VSX fp compare insns to decodetree.
2024-06-13 9:33 [PATCH v2 0/4] Move VSX storage access and compare insns to Chinmay Rath
` (2 preceding siblings ...)
2024-06-13 9:33 ` [PATCH v2 3/4] target/ppc: Move VSX vector " Chinmay Rath
@ 2024-06-13 9:33 ` Chinmay Rath
3 siblings, 0 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-13 9:33 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, richard.henderson, harshpb
Moving the following instructions to decodetree specification:
xvcmp{eq, gt, ge, ne}{s, d}p : XX3-form
The changes were verified by validating that the tcg-ops generated for those
instructions remain the same which were captured using the '-d in_asm,op' flag.
Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/helper.h | 16 +++++-----
target/ppc/insn32.decode | 12 ++++++++
target/ppc/fpu_helper.c | 16 +++++-----
target/ppc/translate/vsx-impl.c.inc | 46 +++++++++++++----------------
target/ppc/translate/vsx-ops.c.inc | 18 -----------
5 files changed, 48 insertions(+), 60 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 510ce76524..3fd849628a 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -473,10 +473,10 @@ DEF_HELPER_5(xvnmadddp, void, env, vsr, vsr, vsr, vsr)
DEF_HELPER_5(xvnmsubdp, void, env, vsr, vsr, vsr, vsr)
DEF_HELPER_4(XVMAXDP, void, env, vsr, vsr, vsr)
DEF_HELPER_4(XVMINDP, void, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpeqdp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpgedp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpgtdp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpnedp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPEQDP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPGEDP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPGTDP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPNEDP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
DEF_HELPER_3(xvcvdpsp, void, env, vsr, vsr)
DEF_HELPER_3(xvcvdpsxds, void, env, vsr, vsr)
DEF_HELPER_3(xvcvdpsxws, void, env, vsr, vsr)
@@ -507,10 +507,10 @@ DEF_HELPER_5(xvnmaddsp, void, env, vsr, vsr, vsr, vsr)
DEF_HELPER_5(xvnmsubsp, void, env, vsr, vsr, vsr, vsr)
DEF_HELPER_4(XVMAXSP, void, env, vsr, vsr, vsr)
DEF_HELPER_4(XVMINSP, void, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpeqsp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpgesp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpgtsp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
-DEF_HELPER_FLAGS_4(xvcmpnesp, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPEQSP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPGESP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPGTSP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
+DEF_HELPER_FLAGS_4(XVCMPNESP, TCG_CALL_NO_RWG, i32, env, vsr, vsr, vsr)
DEF_HELPER_3(xvcvspdp, void, env, vsr, vsr)
DEF_HELPER_3(xvcvsphp, void, env, vsr, vsr)
DEF_HELPER_3(xvcvhpsp, void, env, vsr, vsr)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 3d31ef52f8..bcaf03f24c 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -217,6 +217,9 @@
&XX3 xt xa xb
@XX3 ...... ..... ..... ..... ........ ... &XX3 xt=%xx_xt xa=%xx_xa xb=%xx_xb
+&XX3_rc xt xa xb rc:bool
+@XX3_rc ...... ..... ..... ..... rc:1 ....... ... &XX3_rc xt=%xx_xt xa=%xx_xa xb=%xx_xb
+
# 32 bit GER instructions have all mask bits considered 1
&MMIRR_XX3 xa xb xt pmsk xmsk ymsk
%xx_at 23:3
@@ -923,6 +926,15 @@ XSCMPEQQP 111111 ..... ..... ..... 0001000100 - @X
XSCMPGEQP 111111 ..... ..... ..... 0011000100 - @X
XSCMPGTQP 111111 ..... ..... ..... 0011100100 - @X
+XVCMPEQSP 111100 ..... ..... ..... . 1000011 ... @XX3_rc
+XVCMPGTSP 111100 ..... ..... ..... . 1001011 ... @XX3_rc
+XVCMPGESP 111100 ..... ..... ..... . 1010011 ... @XX3_rc
+XVCMPNESP 111100 ..... ..... ..... . 1011011 ... @XX3_rc
+XVCMPEQDP 111100 ..... ..... ..... . 1100011 ... @XX3_rc
+XVCMPGTDP 111100 ..... ..... ..... . 1101011 ... @XX3_rc
+XVCMPGEDP 111100 ..... ..... ..... . 1110011 ... @XX3_rc
+XVCMPNEDP 111100 ..... ..... ..... . 1111011 ... @XX3_rc
+
XSMAXDP 111100 ..... ..... ..... 10100000 ... @XX3
XSMINDP 111100 ..... ..... ..... 10101000 ... @XX3
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index a013160644..5a300a3c86 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2624,14 +2624,14 @@ uint32_t helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
return crf6; \
}
-VSX_CMP(xvcmpeqdp, 2, float64, VsrD(i), eq, 0, 1)
-VSX_CMP(xvcmpgedp, 2, float64, VsrD(i), le, 1, 1)
-VSX_CMP(xvcmpgtdp, 2, float64, VsrD(i), lt, 1, 1)
-VSX_CMP(xvcmpnedp, 2, float64, VsrD(i), eq, 0, 0)
-VSX_CMP(xvcmpeqsp, 4, float32, VsrW(i), eq, 0, 1)
-VSX_CMP(xvcmpgesp, 4, float32, VsrW(i), le, 1, 1)
-VSX_CMP(xvcmpgtsp, 4, float32, VsrW(i), lt, 1, 1)
-VSX_CMP(xvcmpnesp, 4, float32, VsrW(i), eq, 0, 0)
+VSX_CMP(XVCMPEQDP, 2, float64, VsrD(i), eq, 0, 1)
+VSX_CMP(XVCMPGEDP, 2, float64, VsrD(i), le, 1, 1)
+VSX_CMP(XVCMPGTDP, 2, float64, VsrD(i), lt, 1, 1)
+VSX_CMP(XVCMPNEDP, 2, float64, VsrD(i), eq, 0, 0)
+VSX_CMP(XVCMPEQSP, 4, float32, VsrW(i), eq, 0, 1)
+VSX_CMP(XVCMPGESP, 4, float32, VsrW(i), le, 1, 1)
+VSX_CMP(XVCMPGTSP, 4, float32, VsrW(i), lt, 1, 1)
+VSX_CMP(XVCMPNESP, 4, float32, VsrW(i), eq, 0, 0)
/*
* VSX_CVT_FP_TO_FP - VSX floating point/floating point conversion
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index e0fb4bad92..26ebf3fedf 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -792,34 +792,28 @@ static bool do_xvcpsgn(DisasContext *ctx, arg_XX3 *a, unsigned vece)
TRANS(XVCPSGNSP, do_xvcpsgn, MO_32)
TRANS(XVCPSGNDP, do_xvcpsgn, MO_64)
-#define VSX_CMP(name, op1, op2, inval, type) \
-static void gen_##name(DisasContext *ctx) \
-{ \
- TCGv_i32 ignored; \
- TCGv_ptr xt, xa, xb; \
- if (unlikely(!ctx->vsx_enabled)) { \
- gen_exception(ctx, POWERPC_EXCP_VSXU); \
- return; \
- } \
- xt = gen_vsr_ptr(xT(ctx->opcode)); \
- xa = gen_vsr_ptr(xA(ctx->opcode)); \
- xb = gen_vsr_ptr(xB(ctx->opcode)); \
- if ((ctx->opcode >> (31 - 21)) & 1) { \
- gen_helper_##name(cpu_crf[6], tcg_env, xt, xa, xb); \
- } else { \
- ignored = tcg_temp_new_i32(); \
- gen_helper_##name(ignored, tcg_env, xt, xa, xb); \
- } \
+static bool do_cmp(DisasContext *ctx, arg_XX3_rc *a,
+ void (*helper)(TCGv_i32, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_ptr))
+{
+ TCGv_i32 dest;
+ TCGv_ptr xt, xa, xb;
+ REQUIRE_VSX(ctx);
+ xt = gen_vsr_ptr(a->xt);
+ xa = gen_vsr_ptr(a->xa);
+ xb = gen_vsr_ptr(a->xb);
+ dest = a->rc ? cpu_crf[6] : tcg_temp_new_i32();
+ helper(dest, tcg_env, xt, xa, xb);
+ return true;
}
-VSX_CMP(xvcmpeqdp, 0x0C, 0x0C, 0, PPC2_VSX)
-VSX_CMP(xvcmpgedp, 0x0C, 0x0E, 0, PPC2_VSX)
-VSX_CMP(xvcmpgtdp, 0x0C, 0x0D, 0, PPC2_VSX)
-VSX_CMP(xvcmpnedp, 0x0C, 0x0F, 0, PPC2_ISA300)
-VSX_CMP(xvcmpeqsp, 0x0C, 0x08, 0, PPC2_VSX)
-VSX_CMP(xvcmpgesp, 0x0C, 0x0A, 0, PPC2_VSX)
-VSX_CMP(xvcmpgtsp, 0x0C, 0x09, 0, PPC2_VSX)
-VSX_CMP(xvcmpnesp, 0x0C, 0x0B, 0, PPC2_VSX)
+TRANS_FLAGS2(VSX, XVCMPEQSP, do_cmp, gen_helper_XVCMPEQSP);
+TRANS_FLAGS2(VSX, XVCMPGTSP, do_cmp, gen_helper_XVCMPGTSP);
+TRANS_FLAGS2(VSX, XVCMPGESP, do_cmp, gen_helper_XVCMPGESP);
+TRANS_FLAGS2(ISA300, XVCMPNESP, do_cmp, gen_helper_XVCMPNESP);
+TRANS_FLAGS2(VSX, XVCMPEQDP, do_cmp, gen_helper_XVCMPEQDP);
+TRANS_FLAGS2(VSX, XVCMPGTDP, do_cmp, gen_helper_XVCMPGTDP);
+TRANS_FLAGS2(VSX, XVCMPGEDP, do_cmp, gen_helper_XVCMPGEDP);
+TRANS_FLAGS2(ISA300, XVCMPNEDP, do_cmp, gen_helper_XVCMPNEDP);
static bool trans_XSCVQPDP(DisasContext *ctx, arg_X_tb_rc *a)
{
diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc
index 91cde088bc..e553b5b8fa 100644
--- a/target/ppc/translate/vsx-ops.c.inc
+++ b/target/ppc/translate/vsx-ops.c.inc
@@ -43,16 +43,6 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
-#define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x01, opc3 | 0x00, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x02, opc3 | 0x00, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x03, opc3 | 0x00, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x10, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x01, opc3 | 0x10, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x02, opc3 | 0x10, 0, PPC_NONE, fl2), \
-GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x03, opc3 | 0x10, 0, PPC_NONE, fl2)
-
#define GEN_XX3FORM_DM(name, opc2, opc3) \
GEN_HANDLER2_E(name, #name, 0x3C, opc2|0x00, opc3|0x00, 0, PPC_NONE, PPC2_VSX),\
GEN_HANDLER2_E(name, #name, 0x3C, opc2|0x01, opc3|0x00, 0, PPC_NONE, PPC2_VSX),\
@@ -175,10 +165,6 @@ GEN_XX3FORM_NAME(xvnmadddp, "xvnmaddadp", 0x04, 0x1C, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmadddp, "xvnmaddmdp", 0x04, 0x1D, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmsubdp, "xvnmsubadp", 0x04, 0x1E, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmsubdp, "xvnmsubmdp", 0x04, 0x1F, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpeqdp, 0x0C, 0x0C, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpgtdp, 0x0C, 0x0D, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpgedp, 0x0C, 0x0E, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpnedp, 0x0C, 0x0F, PPC2_ISA300),
GEN_XX2FORM(xvcvdpsp, 0x12, 0x18, PPC2_VSX),
GEN_XX2FORM(xvcvdpsxds, 0x10, 0x1D, PPC2_VSX),
GEN_XX2FORM(xvcvdpsxws, 0x10, 0x0D, PPC2_VSX),
@@ -207,10 +193,6 @@ GEN_XX3FORM_NAME(xvnmaddsp, "xvnmaddasp", 0x04, 0x18, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmaddsp, "xvnmaddmsp", 0x04, 0x19, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmsubsp, "xvnmsubasp", 0x04, 0x1A, PPC2_VSX),
GEN_XX3FORM_NAME(xvnmsubsp, "xvnmsubmsp", 0x04, 0x1B, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpeqsp, 0x0C, 0x08, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpgtsp, 0x0C, 0x09, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpgesp, 0x0C, 0x0A, PPC2_VSX),
-GEN_XX3_RC_FORM(xvcmpnesp, 0x0C, 0x0B, PPC2_ISA300),
GEN_XX2FORM(xvcvspdp, 0x12, 0x1C, PPC2_VSX),
GEN_XX2FORM(xvcvspsxds, 0x10, 0x19, PPC2_VSX),
GEN_XX2FORM(xvcvspsxws, 0x10, 0x09, PPC2_VSX),
--
2.39.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-13 9:33 ` [PATCH v2 2/4] target/ppc: Move VSX vector with length " Chinmay Rath
@ 2024-06-16 19:13 ` Richard Henderson
2024-06-17 10:40 ` Chinmay Rath
2024-06-17 11:51 ` Chinmay Rath
0 siblings, 2 replies; 12+ messages in thread
From: Richard Henderson @ 2024-06-16 19:13 UTC (permalink / raw)
To: Chinmay Rath, qemu-ppc; +Cc: qemu-devel, npiggin, danielhb413, harshpb
On 6/13/24 02:33, Chinmay Rath wrote:
> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
> +{
> + TCGv EA;
> + if (!ra) {
> + EA = tcg_constant_tl(0);
> + return EA;
> + }
> + EA = tcg_temp_new();
> + if (NARROW_MODE(ctx)) {
> + tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
> + } else {
> + tcg_gen_mov_tl(EA, cpu_gpr[ra]);
Why are you making a copy, rather than just returning cpu_gpr[ra]?
If you need to modify the resulting EA, then you also need to make a copy for 0.
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-16 19:13 ` Richard Henderson
@ 2024-06-17 10:40 ` Chinmay Rath
2024-06-17 17:45 ` Richard Henderson
2024-06-17 11:51 ` Chinmay Rath
1 sibling, 1 reply; 12+ messages in thread
From: Chinmay Rath @ 2024-06-17 10:40 UTC (permalink / raw)
To: Richard Henderson, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
Hi Richard,
On 6/17/24 00:43, Richard Henderson wrote:
> On 6/13/24 02:33, Chinmay Rath wrote:
>> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
>> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>> +{
>> + TCGv EA;
>> + if (!ra) {
>> + EA = tcg_constant_tl(0);
>> + return EA;
>> + }
>> + EA = tcg_temp_new();
>> + if (NARROW_MODE(ctx)) {
>> + tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>> + } else {
>> + tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>
> Why are you making a copy, rather than just returning cpu_gpr[ra]?
True, this tcg move is redundant. Was carried away to maintain
uniformity with the original do_ea_calc function. My bad!
This can rather just be :
/* ea <- (ra == 0) ? 0 : GPR[ra] */
static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
{
TCGv EA;
if (!ra) {
return tcg_constant_tl(0);
}
if (NARROW_MODE(ctx)) {
EA = tcg_temp_new();
tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
} else {
return cpu_gpr[ra];
}
return EA;
}
> If you need to modify the resulting EA, then you also need to make a
> copy for 0.
>
Hey, didn't properly get what you meant here.
Did you mean : Since I'm using a tcg_constant for 0, if the EA is to be
modified later, this constant would be an issue, in which case, I should
make a copy for it ??
Considering that, there are no tcg level modifications with this EA.
However, the underlying helper method, which considers this EA as a
target_ulong type does modify it, which I don't think should be an issue.
Please let me know if I missed something.
Thanks & Regards,
Chinmay
> r~
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-16 19:13 ` Richard Henderson
2024-06-17 10:40 ` Chinmay Rath
@ 2024-06-17 11:51 ` Chinmay Rath
2024-06-17 17:57 ` Richard Henderson
1 sibling, 1 reply; 12+ messages in thread
From: Chinmay Rath @ 2024-06-17 11:51 UTC (permalink / raw)
To: Richard Henderson, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
Hi Richard,
On 6/17/24 00:43, Richard Henderson wrote:
> On 6/13/24 02:33, Chinmay Rath wrote:
>> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
>> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>> +{
>> + TCGv EA;
>> + if (!ra) {
>> + EA = tcg_constant_tl(0);
>> + return EA;
>> + }
>> + EA = tcg_temp_new();
>> + if (NARROW_MODE(ctx)) {
>> + tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>> + } else {
>> + tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>
> Why are you making a copy, rather than just returning cpu_gpr[ra]?
> If you need to modify the resulting EA, then you also need to make a
> copy for 0.
>
Please ignore my previous response.
I think do_ea_calc_ra should allow modification to the resulting EA,
hence below change appears more appropriate to me :
/* EA <- (ra == 0) ? 0 : GPR[ra] */
static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
{
TCGv EA = tcg_temp_new();
if (!ra) {
tcg_gen_movi_tl(EA, 0);
return EA;
}
if (NARROW_MODE(ctx)) {
tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
} else {
tcg_gen_mov_tl(EA, cpu_gpr[ra]);
}
return EA;
}
Let me know your thoughts.
Thanks & Regards,
Chinmay
>
> r~
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-17 10:40 ` Chinmay Rath
@ 2024-06-17 17:45 ` Richard Henderson
2024-06-18 8:16 ` Chinmay Rath
0 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2024-06-17 17:45 UTC (permalink / raw)
To: Chinmay Rath, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
On 6/17/24 03:40, Chinmay Rath wrote:
> static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
> {
> TCGv EA;
> if (!ra) {
> return tcg_constant_tl(0);
> }
> if (NARROW_MODE(ctx)) {
> EA = tcg_temp_new();
> tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
> } else {
> return cpu_gpr[ra];
> }
> return EA;
> }
>
>> If you need to modify the resulting EA, then you also need to make a copy for 0.
>>
> Hey, didn't properly get what you meant here.
> Did you mean : Since I'm using a tcg_constant for 0, if the EA is to be modified later,
> this constant would be an issue, in which case, I should make a copy for it ??
Yes.
> Considering that, there are no tcg level modifications with this EA.
Ok, good.
> However, the
> underlying helper method, which considers this EA as a target_ulong type does modify it,
> which I don't think should be an issue.
Correct, that's fine.
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-17 11:51 ` Chinmay Rath
@ 2024-06-17 17:57 ` Richard Henderson
2024-06-18 8:19 ` Chinmay Rath
0 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2024-06-17 17:57 UTC (permalink / raw)
To: Chinmay Rath, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
On 6/17/24 04:51, Chinmay Rath wrote:
>
>
> Hi Richard,
> On 6/17/24 00:43, Richard Henderson wrote:
>> On 6/13/24 02:33, Chinmay Rath wrote:
>>> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
>>> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>>> +{
>>> + TCGv EA;
>>> + if (!ra) {
>>> + EA = tcg_constant_tl(0);
>>> + return EA;
>>> + }
>>> + EA = tcg_temp_new();
>>> + if (NARROW_MODE(ctx)) {
>>> + tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>>> + } else {
>>> + tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>>
>> Why are you making a copy, rather than just returning cpu_gpr[ra]?
>> If you need to modify the resulting EA, then you also need to make a copy for 0.
>>
> Please ignore my previous response.
> I think do_ea_calc_ra should allow modification to the resulting EA, hence below change
> appears more appropriate to me :
>
> /* EA <- (ra == 0) ? 0 : GPR[ra] */
> static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
> {
> TCGv EA = tcg_temp_new();
> if (!ra) {
> tcg_gen_movi_tl(EA, 0);
> return EA;
> }
> if (NARROW_MODE(ctx)) {
> tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
> } else {
> tcg_gen_mov_tl(EA, cpu_gpr[ra]);
> }
> return EA;
> }
If that's what's needed by the callers of do_ea_calc_ra, then yes.
You can drop the first return EA and use else if instead.
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-17 17:45 ` Richard Henderson
@ 2024-06-18 8:16 ` Chinmay Rath
0 siblings, 0 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-18 8:16 UTC (permalink / raw)
To: Richard Henderson, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
On 6/17/24 23:15, Richard Henderson wrote:
> On 6/17/24 03:40, Chinmay Rath wrote:
>> static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>> {
>> TCGv EA;
>> if (!ra) {
>> return tcg_constant_tl(0);
>> }
>> if (NARROW_MODE(ctx)) {
>> EA = tcg_temp_new();
>> tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>> } else {
>> return cpu_gpr[ra];
>> }
>> return EA;
>> }
>>
>>> If you need to modify the resulting EA, then you also need to make a
>>> copy for 0.
>>>
>> Hey, didn't properly get what you meant here.
>> Did you mean : Since I'm using a tcg_constant for 0, if the EA is to
>> be modified later, this constant would be an issue, in which case, I
>> should make a copy for it ??
>
> Yes.
>
>> Considering that, there are no tcg level modifications with this EA.
>
> Ok, good.
>
>
>> However, the underlying helper method, which considers this EA as a
>> target_ulong type does modify it, which I don't think should be an
>> issue.
>
> Correct, that's fine.
Awesome ! Thanks for the clarification.
Regards,
Chinmay
>
>
> r~
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.
2024-06-17 17:57 ` Richard Henderson
@ 2024-06-18 8:19 ` Chinmay Rath
0 siblings, 0 replies; 12+ messages in thread
From: Chinmay Rath @ 2024-06-18 8:19 UTC (permalink / raw)
To: Richard Henderson, Chinmay Rath, qemu-ppc
Cc: qemu-devel, npiggin, danielhb413, harshpb
On 6/17/24 23:27, Richard Henderson wrote:
> On 6/17/24 04:51, Chinmay Rath wrote:
>>
>>
>> Hi Richard,
>> On 6/17/24 00:43, Richard Henderson wrote:
>>> On 6/13/24 02:33, Chinmay Rath wrote:
>>>> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
>>>> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>>>> +{
>>>> + TCGv EA;
>>>> + if (!ra) {
>>>> + EA = tcg_constant_tl(0);
>>>> + return EA;
>>>> + }
>>>> + EA = tcg_temp_new();
>>>> + if (NARROW_MODE(ctx)) {
>>>> + tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>>>> + } else {
>>>> + tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>>>
>>> Why are you making a copy, rather than just returning cpu_gpr[ra]?
>>> If you need to modify the resulting EA, then you also need to make a
>>> copy for 0.
>>>
>> Please ignore my previous response.
>> I think do_ea_calc_ra should allow modification to the resulting EA,
>> hence below change appears more appropriate to me :
>>
>> /* EA <- (ra == 0) ? 0 : GPR[ra] */
>> static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>> {
>> TCGv EA = tcg_temp_new();
>> if (!ra) {
>> tcg_gen_movi_tl(EA, 0);
>> return EA;
>> }
>> if (NARROW_MODE(ctx)) {
>> tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>> } else {
>> tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>> }
>> return EA;
>> }
>
> If that's what's needed by the callers of do_ea_calc_ra, then yes.
> You can drop the first return EA and use else if instead.
Sure.
I shall stick to keeping EA modifiable, (even though it is not modified
by the callers in this patch),
to allow its proper usage by (p){lx, stx}vp insns in future.
Thanks & Regards,
Chinmay
>
>
> r~
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-06-18 8:20 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-13 9:33 [PATCH v2 0/4] Move VSX storage access and compare insns to Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 1/4] target/ppc: Moving VSX scalar storage access insns to decodetree Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 2/4] target/ppc: Move VSX vector with length " Chinmay Rath
2024-06-16 19:13 ` Richard Henderson
2024-06-17 10:40 ` Chinmay Rath
2024-06-17 17:45 ` Richard Henderson
2024-06-18 8:16 ` Chinmay Rath
2024-06-17 11:51 ` Chinmay Rath
2024-06-17 17:57 ` Richard Henderson
2024-06-18 8:19 ` Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 3/4] target/ppc: Move VSX vector " Chinmay Rath
2024-06-13 9:33 ` [PATCH v2 4/4] target/ppc: Move VSX fp compare " Chinmay Rath
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).