[PATCH 0/5] target/ppc: Move floating-point instructions to decodetree.

[PATCH 0/5] target/ppc: Move floating-point instructions to decodetree.

[Chinmay Rath]

Moving floating point rounding, conversion, compare and move 
instructions to decodetree specification.

Also proposing myself as a reviewer for PowerPC TCG CPUs.

Chinmay Rath (5):
  target/ppc: Move floating-point rounding and conversion instructions
    to decodetree.
  target/ppc: Move floating-point compare instructions to decodetree.
  target/ppc: Move floating-point move instructions to decodetree.
  target/ppc: Move remaining floating-point move instructions to
    decodetree.
  MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs

 MAINTAINERS                        |   1 +
 target/ppc/fpu_helper.c            |  38 ++--
 target/ppc/helper.h                |  38 ++--
 target/ppc/insn32.decode           |  40 ++++
 target/ppc/translate/fp-impl.c.inc | 289 ++++++++++-------------------
 target/ppc/translate/fp-ops.c.inc  |  30 ---
 6 files changed, 181 insertions(+), 255 deletions(-)

-- 
2.49.0

[PATCH 1/5] target/ppc: Move floating-point rounding and conversion instructions to decodetree.

[Chinmay Rath]

Move below instructions to decodetree specification :

	fr{sp, in, iz, im}[s][.],
	fcti{w, d}[u, z, uz][s][.],
	fcfid[s, u, us][s][.]           : X-form

The changes were verified by validating that the tcg ops generated by
those instructions remain the same, which were captured with the '-d
in_asm,op' flag.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 target/ppc/fpu_helper.c            | 34 +++++-----
 target/ppc/helper.h                | 34 +++++-----
 target/ppc/insn32.decode           | 24 ++++++++
 target/ppc/translate/fp-impl.c.inc | 99 ++++++++++++------------------
 target/ppc/translate/fp-ops.c.inc  | 21 -------
 5 files changed, 98 insertions(+), 114 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 07b782f971..503cbd98ad 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -562,14 +562,14 @@ uint64_t helper_##op(CPUPPCState *env, float64 arg)                    \
     return ret;                                                        \
 }
 
-FPU_FCTI(fctiw, int32, 0x80000000U)
-FPU_FCTI(fctiwz, int32_round_to_zero, 0x80000000U)
-FPU_FCTI(fctiwu, uint32, 0x00000000U)
-FPU_FCTI(fctiwuz, uint32_round_to_zero, 0x00000000U)
-FPU_FCTI(fctid, int64, 0x8000000000000000ULL)
-FPU_FCTI(fctidz, int64_round_to_zero, 0x8000000000000000ULL)
-FPU_FCTI(fctidu, uint64, 0x0000000000000000ULL)
-FPU_FCTI(fctiduz, uint64_round_to_zero, 0x0000000000000000ULL)
+FPU_FCTI(FCTIW, int32, 0x80000000U)
+FPU_FCTI(FCTIWZ, int32_round_to_zero, 0x80000000U)
+FPU_FCTI(FCTIWU, uint32, 0x00000000U)
+FPU_FCTI(FCTIWUZ, uint32_round_to_zero, 0x00000000U)
+FPU_FCTI(FCTID, int64, 0x8000000000000000ULL)
+FPU_FCTI(FCTIDZ, int64_round_to_zero, 0x8000000000000000ULL)
+FPU_FCTI(FCTIDU, uint64, 0x0000000000000000ULL)
+FPU_FCTI(FCTIDUZ, uint64_round_to_zero, 0x0000000000000000ULL)
 
 #define FPU_FCFI(op, cvtr, is_single)                      \
 uint64_t helper_##op(CPUPPCState *env, uint64_t arg)       \
@@ -586,10 +586,10 @@ uint64_t helper_##op(CPUPPCState *env, uint64_t arg)       \
     return farg.ll;                                        \
 }
 
-FPU_FCFI(fcfid, int64_to_float64, 0)
-FPU_FCFI(fcfids, int64_to_float32, 1)
-FPU_FCFI(fcfidu, uint64_to_float64, 0)
-FPU_FCFI(fcfidus, uint64_to_float32, 1)
+FPU_FCFI(FCFID, int64_to_float64, 0)
+FPU_FCFI(FCFIDS, int64_to_float32, 1)
+FPU_FCFI(FCFIDU, uint64_to_float64, 0)
+FPU_FCFI(FCFIDUS, uint64_to_float32, 1)
 
 static uint64_t do_fri(CPUPPCState *env, uint64_t arg,
                        FloatRoundMode rounding_mode)
@@ -613,22 +613,22 @@ static uint64_t do_fri(CPUPPCState *env, uint64_t arg,
     return arg;
 }
 
-uint64_t helper_frin(CPUPPCState *env, uint64_t arg)
+uint64_t helper_FRIN(CPUPPCState *env, uint64_t arg)
 {
     return do_fri(env, arg, float_round_ties_away);
 }
 
-uint64_t helper_friz(CPUPPCState *env, uint64_t arg)
+uint64_t helper_FRIZ(CPUPPCState *env, uint64_t arg)
 {
     return do_fri(env, arg, float_round_to_zero);
 }
 
-uint64_t helper_frip(CPUPPCState *env, uint64_t arg)
+uint64_t helper_FRIP(CPUPPCState *env, uint64_t arg)
 {
     return do_fri(env, arg, float_round_up);
 }
 
-uint64_t helper_frim(CPUPPCState *env, uint64_t arg)
+uint64_t helper_FRIM(CPUPPCState *env, uint64_t arg)
 {
     return do_fri(env, arg, float_round_down);
 }
@@ -697,7 +697,7 @@ static uint64_t do_frsp(CPUPPCState *env, uint64_t arg, uintptr_t retaddr)
     return helper_todouble(f32);
 }
 
-uint64_t helper_frsp(CPUPPCState *env, uint64_t arg)
+uint64_t helper_FRSP(CPUPPCState *env, uint64_t arg)
 {
     return do_frsp(env, arg, GETPC());
 }
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index ca414f2f43..96000f4f0d 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -97,23 +97,23 @@ DEF_HELPER_FLAGS_1(tosingle, TCG_CALL_NO_RWG_SE, i32, i64)
 DEF_HELPER_4(fcmpo, void, env, i64, i64, i32)
 DEF_HELPER_4(fcmpu, void, env, i64, i64, i32)
 
-DEF_HELPER_2(fctiw, i64, env, i64)
-DEF_HELPER_2(fctiwu, i64, env, i64)
-DEF_HELPER_2(fctiwz, i64, env, i64)
-DEF_HELPER_2(fctiwuz, i64, env, i64)
-DEF_HELPER_2(fcfid, i64, env, i64)
-DEF_HELPER_2(fcfidu, i64, env, i64)
-DEF_HELPER_2(fcfids, i64, env, i64)
-DEF_HELPER_2(fcfidus, i64, env, i64)
-DEF_HELPER_2(fctid, i64, env, i64)
-DEF_HELPER_2(fctidu, i64, env, i64)
-DEF_HELPER_2(fctidz, i64, env, i64)
-DEF_HELPER_2(fctiduz, i64, env, i64)
-DEF_HELPER_2(frsp, i64, env, i64)
-DEF_HELPER_2(frin, i64, env, i64)
-DEF_HELPER_2(friz, i64, env, i64)
-DEF_HELPER_2(frip, i64, env, i64)
-DEF_HELPER_2(frim, i64, env, i64)
+DEF_HELPER_2(FCTIW, i64, env, i64)
+DEF_HELPER_2(FCTIWU, i64, env, i64)
+DEF_HELPER_2(FCTIWZ, i64, env, i64)
+DEF_HELPER_2(FCTIWUZ, i64, env, i64)
+DEF_HELPER_2(FCFID, i64, env, i64)
+DEF_HELPER_2(FCFIDU, i64, env, i64)
+DEF_HELPER_2(FCFIDS, i64, env, i64)
+DEF_HELPER_2(FCFIDUS, i64, env, i64)
+DEF_HELPER_2(FCTID, i64, env, i64)
+DEF_HELPER_2(FCTIDU, i64, env, i64)
+DEF_HELPER_2(FCTIDZ, i64, env, i64)
+DEF_HELPER_2(FCTIDUZ, i64, env, i64)
+DEF_HELPER_2(FRSP, i64, env, i64)
+DEF_HELPER_2(FRIN, i64, env, i64)
+DEF_HELPER_2(FRIZ, i64, env, i64)
+DEF_HELPER_2(FRIP, i64, env, i64)
+DEF_HELPER_2(FRIM, i64, env, i64)
 
 DEF_HELPER_3(FADD, f64, env, f64, f64)
 DEF_HELPER_3(FADDS, f64, env, f64, f64)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index e53fd2840d..15dec702b9 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -541,6 +541,30 @@ FNMADDS         111011 ..... ..... ..... ..... 11111 .  @A
 FNMSUB          111111 ..... ..... ..... ..... 11110 .  @A
 FNMSUBS         111011 ..... ..... ..... ..... 11110 .  @A
 
+### Floating-Point Rounding and Conversion Instructions
+
+FRSP            111111 ..... ----- ..... 0000001100 .   @X_tb_rc
+
+FRIN            111111 ..... ----- ..... 0110001000 .   @X_tb_rc
+FRIZ            111111 ..... ----- ..... 0110101000 .   @X_tb_rc
+FRIP            111111 ..... ----- ..... 0111001000 .   @X_tb_rc
+FRIM            111111 ..... ----- ..... 0111101000 .   @X_tb_rc
+
+FCTIW           111111 ..... ----- ..... 0000001110 .   @X_tb_rc
+FCTIWU          111111 ..... ----- ..... 0010001110 .   @X_tb_rc
+FCTIWZ          111111 ..... ----- ..... 0000001111 .   @X_tb_rc
+FCTIWUZ         111111 ..... ----- ..... 0010001111 .   @X_tb_rc
+
+FCTID           111111 ..... ----- ..... 1100101110 .   @X_tb_rc
+FCTIDU          111111 ..... ----- ..... 1110101110 .   @X_tb_rc
+FCTIDZ          111111 ..... ----- ..... 1100101111 .   @X_tb_rc
+FCTIDUZ         111111 ..... ----- ..... 1110101111 .   @X_tb_rc
+
+FCFID           111111 ..... ----- ..... 1101001110 .   @X_tb_rc
+FCFIDS          111011 ..... ----- ..... 1101001110 .   @X_tb_rc
+FCFIDU          111111 ..... ----- ..... 1111001110 .   @X_tb_rc
+FCFIDUS         111011 ..... ----- ..... 1111001110 .   @X_tb_rc
+
 ### Floating-Point Select Instruction
 
 FSEL            111111 ..... ..... ..... ..... 10111 .  @A
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index a66b83398b..f296cfcdb0 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -98,28 +98,26 @@ static bool do_helper_ac(DisasContext *ctx, arg_A_tac *a,
     return true;
 }
 
-#define GEN_FLOAT_B(name, op2, op3, set_fprf, type)                           \
-static void gen_f##name(DisasContext *ctx)                                    \
-{                                                                             \
-    TCGv_i64 t0;                                                              \
-    TCGv_i64 t1;                                                              \
-    if (unlikely(!ctx->fpu_enabled)) {                                        \
-        gen_exception(ctx, POWERPC_EXCP_FPU);                                 \
-        return;                                                               \
-    }                                                                         \
-    t0 = tcg_temp_new_i64();                                                  \
-    t1 = tcg_temp_new_i64();                                                  \
-    gen_reset_fpstatus();                                                     \
-    get_fpr(t0, rB(ctx->opcode));                                             \
-    gen_helper_f##name(t1, tcg_env, t0);                                      \
-    set_fpr(rD(ctx->opcode), t1);                                             \
-    if (set_fprf) {                                                           \
-        gen_helper_compute_fprf_float64(tcg_env, t1);                         \
-    }                                                                         \
-    gen_helper_float_check_status(tcg_env);                                   \
-    if (unlikely(Rc(ctx->opcode) != 0)) {                                     \
-        gen_set_cr1_from_fpscr(ctx);                                          \
-    }                                                                         \
+static bool do_round_convert(DisasContext *ctx, arg_X_tb_rc *a,
+                             void (*helper)(TCGv_i64, TCGv_env, TCGv_i64),
+                             bool set_fprf)
+{
+    TCGv_i64 t0, t1;
+    REQUIRE_FPU(ctx);
+    t0 = tcg_temp_new_i64();
+    t1 = tcg_temp_new_i64();
+    gen_reset_fpstatus();
+    get_fpr(t0, a->rb);
+    helper(t1, tcg_env, t0);
+    set_fpr(a->rt, t1);
+    if (set_fprf) {
+        gen_helper_compute_fprf_float64(tcg_env, t1);
+    }
+    gen_helper_float_check_status(tcg_env);
+    if (unlikely(a->rc)) {
+        gen_set_cr1_from_fpscr(ctx);
+    }
+    return true;
 }
 
 static bool do_helper_bs(DisasContext *ctx, arg_A_tb *a,
@@ -213,41 +211,26 @@ TRANS(FSQRT, do_helper_fsqrt, gen_helper_FSQRT);
 TRANS(FSQRTS, do_helper_fsqrt, gen_helper_FSQRTS);
 
 /***                     Floating-Point round & convert                    ***/
-/* fctiw */
-GEN_FLOAT_B(ctiw, 0x0E, 0x00, 0, PPC_FLOAT);
-/* fctiwu */
-GEN_FLOAT_B(ctiwu, 0x0E, 0x04, 0, PPC2_FP_CVT_ISA206);
-/* fctiwz */
-GEN_FLOAT_B(ctiwz, 0x0F, 0x00, 0, PPC_FLOAT);
-/* fctiwuz */
-GEN_FLOAT_B(ctiwuz, 0x0F, 0x04, 0, PPC2_FP_CVT_ISA206);
-/* frsp */
-GEN_FLOAT_B(rsp, 0x0C, 0x00, 1, PPC_FLOAT);
-/* fcfid */
-GEN_FLOAT_B(cfid, 0x0E, 0x1A, 1, PPC2_FP_CVT_S64);
-/* fcfids */
-GEN_FLOAT_B(cfids, 0x0E, 0x1A, 0, PPC2_FP_CVT_ISA206);
-/* fcfidu */
-GEN_FLOAT_B(cfidu, 0x0E, 0x1E, 0, PPC2_FP_CVT_ISA206);
-/* fcfidus */
-GEN_FLOAT_B(cfidus, 0x0E, 0x1E, 0, PPC2_FP_CVT_ISA206);
-/* fctid */
-GEN_FLOAT_B(ctid, 0x0E, 0x19, 0, PPC2_FP_CVT_S64);
-/* fctidu */
-GEN_FLOAT_B(ctidu, 0x0E, 0x1D, 0, PPC2_FP_CVT_ISA206);
-/* fctidz */
-GEN_FLOAT_B(ctidz, 0x0F, 0x19, 0, PPC2_FP_CVT_S64);
-/* fctidu */
-GEN_FLOAT_B(ctiduz, 0x0F, 0x1D, 0, PPC2_FP_CVT_ISA206);
-
-/* frin */
-GEN_FLOAT_B(rin, 0x08, 0x0C, 1, PPC_FLOAT_EXT);
-/* friz */
-GEN_FLOAT_B(riz, 0x08, 0x0D, 1, PPC_FLOAT_EXT);
-/* frip */
-GEN_FLOAT_B(rip, 0x08, 0x0E, 1, PPC_FLOAT_EXT);
-/* frim */
-GEN_FLOAT_B(rim, 0x08, 0x0F, 1, PPC_FLOAT_EXT);
+TRANS_FLAGS(FLOAT, FRSP, do_round_convert, gen_helper_FRSP, true);
+TRANS_FLAGS(FLOAT_EXT, FRIN, do_round_convert, gen_helper_FRIN, true);
+TRANS_FLAGS(FLOAT_EXT, FRIZ, do_round_convert, gen_helper_FRIZ, true);
+TRANS_FLAGS(FLOAT_EXT, FRIP, do_round_convert, gen_helper_FRIP, true);
+TRANS_FLAGS(FLOAT_EXT, FRIM, do_round_convert, gen_helper_FRIM, true);
+
+TRANS_FLAGS(FLOAT, FCTIW, do_round_convert, gen_helper_FCTIW, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCTIWU, do_round_convert, gen_helper_FCTIWU, false);
+TRANS_FLAGS(FLOAT, FCTIWZ, do_round_convert, gen_helper_FCTIWZ, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCTIWUZ, do_round_convert, gen_helper_FCTIWUZ, false);
+
+TRANS_FLAGS2(FP_CVT_S64, FCTID, do_round_convert, gen_helper_FCTID, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCTIDU, do_round_convert, gen_helper_FCTIDU, false);
+TRANS_FLAGS2(FP_CVT_S64, FCTIDZ, do_round_convert, gen_helper_FCTIDZ, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCTIDUZ, do_round_convert, gen_helper_FCTIDUZ, false);
+
+TRANS_FLAGS2(FP_CVT_S64, FCFID, do_round_convert, gen_helper_FCFID, true);
+TRANS_FLAGS2(FP_CVT_ISA206, FCFIDS, do_round_convert, gen_helper_FCFIDS, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCFIDU, do_round_convert, gen_helper_FCFIDU, false);
+TRANS_FLAGS2(FP_CVT_ISA206, FCFIDUS, do_round_convert, gen_helper_FCFIDUS, false);
 
 static bool trans_FTDIV(DisasContext *ctx, arg_X_bf *a)
 {
@@ -1051,8 +1034,6 @@ TRANS(STFDX, do_lsfp_X, false, true, false)
 TRANS(STFDUX, do_lsfp_X, true, true, false)
 TRANS(PSTFD, do_lsfp_PLS_D, false, true, false)
 
-#undef GEN_FLOAT_B
-
 #undef GEN_LDF
 #undef GEN_LDUF
 #undef GEN_LDUXF
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index cef4b5dfcb..acb8ac32da 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -1,24 +1,3 @@
-#define GEN_FLOAT_B(name, op2, op3, set_fprf, type)                           \
-GEN_HANDLER(f##name, 0x3F, op2, op3, 0x001F0000, type)
-
-GEN_FLOAT_B(ctiw, 0x0E, 0x00, 0, PPC_FLOAT),
-GEN_HANDLER_E(fctiwu, 0x3F, 0x0E, 0x04, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_FLOAT_B(ctiwz, 0x0F, 0x00, 0, PPC_FLOAT),
-GEN_HANDLER_E(fctiwuz, 0x3F, 0x0F, 0x04, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_FLOAT_B(rsp, 0x0C, 0x00, 1, PPC_FLOAT),
-GEN_HANDLER_E(fcfid, 0x3F, 0x0E, 0x1A, 0x001F0000, PPC_NONE, PPC2_FP_CVT_S64),
-GEN_HANDLER_E(fcfids, 0x3B, 0x0E, 0x1A, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_HANDLER_E(fcfidu, 0x3F, 0x0E, 0x1E, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_HANDLER_E(fcfidus, 0x3B, 0x0E, 0x1E, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_HANDLER_E(fctid, 0x3F, 0x0E, 0x19, 0x001F0000, PPC_NONE, PPC2_FP_CVT_S64),
-GEN_HANDLER_E(fctidu, 0x3F, 0x0E, 0x1D, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_HANDLER_E(fctidz, 0x3F, 0x0F, 0x19, 0x001F0000, PPC_NONE, PPC2_FP_CVT_S64),
-GEN_HANDLER_E(fctiduz, 0x3F, 0x0F, 0x1D, 0, PPC_NONE, PPC2_FP_CVT_ISA206),
-GEN_FLOAT_B(rin, 0x08, 0x0C, 1, PPC_FLOAT_EXT),
-GEN_FLOAT_B(riz, 0x08, 0x0D, 1, PPC_FLOAT_EXT),
-GEN_FLOAT_B(rip, 0x08, 0x0E, 1, PPC_FLOAT_EXT),
-GEN_FLOAT_B(rim, 0x08, 0x0F, 1, PPC_FLOAT_EXT),
-
 GEN_HANDLER_E(lfdepx, 0x1F, 0x1F, 0x12, 0x00000001, PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER_E(lfiwax, 0x1f, 0x17, 0x1a, 0x00000001, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(lfiwzx, 0x1f, 0x17, 0x1b, 0x1, PPC_NONE, PPC2_FP_CVT_ISA206),
-- 
2.49.0

[PATCH 2/5] target/ppc: Move floating-point compare instructions to decodetree.

[Chinmay Rath]

Move below instructions to decodetree specification :

	fcmp{u, o}		: X-form

The changes were verified by validating that the tcg ops generated by
those instructions remain the same, which were captured with the '-d
in_asm,op' flag.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 target/ppc/fpu_helper.c            |  4 +--
 target/ppc/helper.h                |  4 +--
 target/ppc/insn32.decode           |  5 ++++
 target/ppc/translate/fp-impl.c.inc | 45 +++++++++---------------------
 target/ppc/translate/fp-ops.c.inc  |  2 --
 5 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 503cbd98ad..850aca6ed1 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -871,7 +871,7 @@ uint32_t helper_FTSQRT(uint64_t frb)
     return 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0);
 }
 
-void helper_fcmpu(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
+void helper_FCMPU(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
                   uint32_t crfD)
 {
     CPU_DoubleU farg1, farg2;
@@ -902,7 +902,7 @@ void helper_fcmpu(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
     }
 }
 
-void helper_fcmpo(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
+void helper_FCMPO(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
                   uint32_t crfD)
 {
     CPU_DoubleU farg1, farg2;
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 96000f4f0d..e99c8c824b 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -94,8 +94,8 @@ DEF_HELPER_2(fpscr_setbit, void, env, i32)
 DEF_HELPER_FLAGS_1(todouble, TCG_CALL_NO_RWG_SE, i64, i32)
 DEF_HELPER_FLAGS_1(tosingle, TCG_CALL_NO_RWG_SE, i32, i64)
 
-DEF_HELPER_4(fcmpo, void, env, i64, i64, i32)
-DEF_HELPER_4(fcmpu, void, env, i64, i64, i32)
+DEF_HELPER_4(FCMPO, void, env, i64, i64, i32)
+DEF_HELPER_4(FCMPU, void, env, i64, i64, i32)
 
 DEF_HELPER_2(FCTIW, i64, env, i64)
 DEF_HELPER_2(FCTIWU, i64, env, i64)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 15dec702b9..0ffd814471 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -565,6 +565,11 @@ FCFIDS          111011 ..... ----- ..... 1101001110 .   @X_tb_rc
 FCFIDU          111111 ..... ----- ..... 1111001110 .   @X_tb_rc
 FCFIDUS         111011 ..... ----- ..... 1111001110 .   @X_tb_rc
 
+### Floating-Point Compare Instructions
+
+FCMPU           111111 ... -- ..... ..... 0000000000 -  @X_bf
+FCMPO           111111 ... -- ..... ..... 0000100000 -  @X_bf
+
 ### Floating-Point Select Instruction
 
 FSEL            111111 ..... ..... ..... ..... 10111 .  @A
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index f296cfcdb0..4e18d350c0 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -257,46 +257,27 @@ static bool trans_FTSQRT(DisasContext *ctx, arg_X_bf_b *a)
 }
 
 /***                         Floating-Point compare                        ***/
-
-/* fcmpo */
-static void gen_fcmpo(DisasContext *ctx)
+static bool do_helper_cmp(DisasContext *ctx, arg_X_bf *a,
+                          void (*helper)(TCGv_env, TCGv_i64, TCGv_i64,
+                                         TCGv_i32))
 {
     TCGv_i32 crf;
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
+    TCGv_i64 t0, t1;
+    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
     t1 = tcg_temp_new_i64();
     gen_reset_fpstatus();
-    crf = tcg_constant_i32(crfD(ctx->opcode));
-    get_fpr(t0, rA(ctx->opcode));
-    get_fpr(t1, rB(ctx->opcode));
-    gen_helper_fcmpo(tcg_env, t0, t1, crf);
+    crf = tcg_constant_i32(a->bf);
+    get_fpr(t0, a->ra);
+    get_fpr(t1, a->rb);
+    helper(tcg_env, t0, t1, crf);
     gen_helper_float_check_status(tcg_env);
+    return true;
 }
 
-/* fcmpu */
-static void gen_fcmpu(DisasContext *ctx)
-{
-    TCGv_i32 crf;
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
-    t0 = tcg_temp_new_i64();
-    t1 = tcg_temp_new_i64();
-    gen_reset_fpstatus();
-    crf = tcg_constant_i32(crfD(ctx->opcode));
-    get_fpr(t0, rA(ctx->opcode));
-    get_fpr(t1, rB(ctx->opcode));
-    gen_helper_fcmpu(tcg_env, t0, t1, crf);
-    gen_helper_float_check_status(tcg_env);
-}
+TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
+TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);
 
 /***                         Floating-point move                           ***/
 /* fabs */
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index acb8ac32da..502453da35 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -10,8 +10,6 @@ GEN_STXF(stfiw, st32fiw, 0x17, 0x1E, PPC_FLOAT_STFIWX)
 GEN_HANDLER_E(stfdepx, 0x1F, 0x1F, 0x16, 0x00000001, PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER_E(stfdpx, 0x1F, 0x17, 0x1C, 0x00200001, PPC_NONE, PPC2_ISA205),
 
-GEN_HANDLER(fcmpo, 0x3F, 0x00, 0x01, 0x00600001, PPC_FLOAT),
-GEN_HANDLER(fcmpu, 0x3F, 0x00, 0x00, 0x00600001, PPC_FLOAT),
 GEN_HANDLER(fabs, 0x3F, 0x08, 0x08, 0x001F0000, PPC_FLOAT),
 GEN_HANDLER(fmr, 0x3F, 0x08, 0x02, 0x001F0000, PPC_FLOAT),
 GEN_HANDLER(fnabs, 0x3F, 0x08, 0x04, 0x001F0000, PPC_FLOAT),
-- 
2.49.0

[PATCH 3/5] target/ppc: Move floating-point move instructions to decodetree.

[Chinmay Rath]

Move below instructions to decodetree specification:

	f{mr, neg, abs, nabs}		: X-form

The changes were verified by validating that the tcg ops generated by
those instructions remain the same, which were captured with the '-d
in_asm,op' flag.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 target/ppc/insn32.decode           |  7 +++
 target/ppc/translate/fp-impl.c.inc | 80 ++++++++----------------------
 target/ppc/translate/fp-ops.c.inc  |  4 --
 3 files changed, 28 insertions(+), 63 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 0ffd814471..abbdb9fd82 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -503,6 +503,13 @@ STFDU           110111 ..... ...... ...............     @D
 STFDX           011111 ..... ...... .... 1011010111 -   @X
 STFDUX          011111 ..... ...... .... 1011110111 -   @X
 
+### Floating-Point Move Instructions
+
+FMR             111111 ..... ----- ..... 0001001000 .   @X_tb_rc
+FNEG            111111 ..... ----- ..... 0000101000 .   @X_tb_rc
+FABS            111111 ..... ----- ..... 0100001000 .   @X_tb_rc
+FNABS           111111 ..... ----- ..... 0010001000 .   @X_tb_rc
+
 ### Floating-Point Arithmetic Instructions
 
 FADD            111111 ..... ..... ..... ----- 10101 .  @A_tab
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index 4e18d350c0..2843f71122 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -280,82 +280,44 @@ TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
 TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);
 
 /***                         Floating-point move                           ***/
-/* fabs */
-/* XXX: beware that fabs never checks for NaNs nor update FPSCR */
-static void gen_fabs(DisasContext *ctx)
-{
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
-    t0 = tcg_temp_new_i64();
-    t1 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    tcg_gen_andi_i64(t1, t0, ~(1ULL << 63));
-    set_fpr(rD(ctx->opcode), t1);
-    if (unlikely(Rc(ctx->opcode))) {
-        gen_set_cr1_from_fpscr(ctx);
-    }
-}
 
 /* fmr  - fmr. */
 /* XXX: beware that fmr never checks for NaNs nor update FPSCR */
-static void gen_fmr(DisasContext *ctx)
+static bool trans_FMR(DisasContext *ctx, arg_FMR *a)
 {
     TCGv_i64 t0;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
+    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    set_fpr(rD(ctx->opcode), t0);
-    if (unlikely(Rc(ctx->opcode))) {
+    get_fpr(t0, a->rb);
+    set_fpr(a->rt, t0);
+    if (unlikely(a->rc)) {
         gen_set_cr1_from_fpscr(ctx);
     }
+    return true;
 }
 
-/* fnabs */
-/* XXX: beware that fnabs never checks for NaNs nor update FPSCR */
-static void gen_fnabs(DisasContext *ctx)
+/* XXX: beware that f{neg, abs, nabs} never checks for NaNs nor update FPSCR */
+static bool do_move_b(DisasContext *ctx, arg_X_tb_rc *a, int64_t val,
+                      void (*tcg_op)(TCGv_i64, TCGv_i64, int64_t))
 {
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
+    TCGv_i64 t0, t1;
+    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
     t1 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    tcg_gen_ori_i64(t1, t0, 1ULL << 63);
-    set_fpr(rD(ctx->opcode), t1);
-    if (unlikely(Rc(ctx->opcode))) {
+    get_fpr(t0, a->rb);
+    tcg_op(t1, t0, val);
+    set_fpr(a->rt, t1);
+    if (unlikely(a->rc)) {
         gen_set_cr1_from_fpscr(ctx);
     }
+    return true;
 }
 
-/* fneg */
-/* XXX: beware that fneg never checks for NaNs nor update FPSCR */
-static void gen_fneg(DisasContext *ctx)
-{
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
-    t0 = tcg_temp_new_i64();
-    t1 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    tcg_gen_xori_i64(t1, t0, 1ULL << 63);
-    set_fpr(rD(ctx->opcode), t1);
-    if (unlikely(Rc(ctx->opcode))) {
-        gen_set_cr1_from_fpscr(ctx);
-    }
-}
+TRANS(FNEG, do_move_b, 1ULL << 63, tcg_gen_xori_i64);
+TRANS(FABS, do_move_b, ~(1ULL << 63), tcg_gen_andi_i64);
+TRANS(FNABS, do_move_b, 1ULL << 63, tcg_gen_ori_i64);
 
 /* fcpsgn: PowerPC 2.05 specification */
 /* XXX: beware that fcpsgn never checks for NaNs nor update FPSCR */
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index 502453da35..5053cb135c 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -10,10 +10,6 @@ GEN_STXF(stfiw, st32fiw, 0x17, 0x1E, PPC_FLOAT_STFIWX)
 GEN_HANDLER_E(stfdepx, 0x1F, 0x1F, 0x16, 0x00000001, PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER_E(stfdpx, 0x1F, 0x17, 0x1C, 0x00200001, PPC_NONE, PPC2_ISA205),
 
-GEN_HANDLER(fabs, 0x3F, 0x08, 0x08, 0x001F0000, PPC_FLOAT),
-GEN_HANDLER(fmr, 0x3F, 0x08, 0x02, 0x001F0000, PPC_FLOAT),
-GEN_HANDLER(fnabs, 0x3F, 0x08, 0x04, 0x001F0000, PPC_FLOAT),
-GEN_HANDLER(fneg, 0x3F, 0x08, 0x01, 0x001F0000, PPC_FLOAT),
 GEN_HANDLER_E(fcpsgn, 0x3F, 0x08, 0x00, 0x00000000, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(fmrgew, 0x3F, 0x06, 0x1E, 0x00000001, PPC_NONE, PPC2_VSX207),
 GEN_HANDLER_E(fmrgow, 0x3F, 0x06, 0x1A, 0x00000001, PPC_NONE, PPC2_VSX207),
-- 
2.49.0

[PATCH 4/5] target/ppc: Move remaining floating-point move instructions to decodetree.

[Chinmay Rath]

Move below instructions to decodetree specification:

	fcpsgn, fmrg{e, o}w	: X-form

The changes were verified by validating that the tcg ops generated by
those instructions remain the same, which were captured with the '-d
in_asm,op' flag.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 target/ppc/insn32.decode           |  4 ++
 target/ppc/translate/fp-impl.c.inc | 65 +++++++++++++-----------------
 target/ppc/translate/fp-ops.c.inc  |  3 --
 3 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index abbdb9fd82..84f9fcd519 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -510,6 +510,10 @@ FNEG            111111 ..... ----- ..... 0000101000 .   @X_tb_rc
 FABS            111111 ..... ----- ..... 0100001000 .   @X_tb_rc
 FNABS           111111 ..... ----- ..... 0010001000 .   @X_tb_rc
 
+FCPSGN          111111 ..... ..... ..... 0000001000 .   @X_rc
+FMRGEW          111111 ..... ..... ..... 1111000110 -   @X
+FMRGOW          111111 ..... ..... ..... 1101000110 -   @X
+
 ### Floating-Point Arithmetic Instructions
 
 FADD            111111 ..... ..... ..... ----- 10101 .  @A_tab
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index 2843f71122..28dda15040 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -321,62 +321,53 @@ TRANS(FNABS, do_move_b, 1ULL << 63, tcg_gen_ori_i64);
 
 /* fcpsgn: PowerPC 2.05 specification */
 /* XXX: beware that fcpsgn never checks for NaNs nor update FPSCR */
-static void gen_fcpsgn(DisasContext *ctx)
+static bool trans_FCPSGN(DisasContext *ctx, arg_FCPSGN *a)
 {
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    TCGv_i64 t2;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
+    TCGv_i64 t0, t1, t2;
+    REQUIRE_INSNS_FLAGS2(ctx, ISA205);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
     t1 = tcg_temp_new_i64();
     t2 = tcg_temp_new_i64();
-    get_fpr(t0, rA(ctx->opcode));
-    get_fpr(t1, rB(ctx->opcode));
+    get_fpr(t0, a->ra);
+    get_fpr(t1, a->rb);
     tcg_gen_deposit_i64(t2, t0, t1, 0, 63);
-    set_fpr(rD(ctx->opcode), t2);
-    if (unlikely(Rc(ctx->opcode))) {
+    set_fpr(a->rt, t2);
+    if (unlikely(a->rc)) {
         gen_set_cr1_from_fpscr(ctx);
     }
+    return true;
 }
 
-static void gen_fmrgew(DisasContext *ctx)
+static bool trans_FMRGEW(DisasContext *ctx, arg_FMRGEW *a)
 {
-    TCGv_i64 b0;
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
-    b0 = tcg_temp_new_i64();
+    TCGv_i64 t0, t1, t2;
+    REQUIRE_INSNS_FLAGS2(ctx, VSX207);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
     t1 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    tcg_gen_shri_i64(b0, t0, 32);
-    get_fpr(t0, rA(ctx->opcode));
-    tcg_gen_deposit_i64(t1, t0, b0, 0, 32);
-    set_fpr(rD(ctx->opcode), t1);
+    t2 = tcg_temp_new_i64();
+    get_fpr(t1, a->rb);
+    tcg_gen_shri_i64(t0, t1, 32);
+    get_fpr(t1, a->ra);
+    tcg_gen_deposit_i64(t2, t1, t0, 0, 32);
+    set_fpr(a->rt, t2);
+    return true;
 }
 
-static void gen_fmrgow(DisasContext *ctx)
+static bool trans_FMRGOW(DisasContext *ctx, arg_FMRGOW *a)
 {
-    TCGv_i64 t0;
-    TCGv_i64 t1;
-    TCGv_i64 t2;
-    if (unlikely(!ctx->fpu_enabled)) {
-        gen_exception(ctx, POWERPC_EXCP_FPU);
-        return;
-    }
+    TCGv_i64 t0, t1, t2;
+    REQUIRE_INSNS_FLAGS2(ctx, VSX207);
+    REQUIRE_FPU(ctx);
     t0 = tcg_temp_new_i64();
     t1 = tcg_temp_new_i64();
     t2 = tcg_temp_new_i64();
-    get_fpr(t0, rB(ctx->opcode));
-    get_fpr(t1, rA(ctx->opcode));
+    get_fpr(t0, a->rb);
+    get_fpr(t1, a->ra);
     tcg_gen_deposit_i64(t2, t0, t1, 32, 32);
-    set_fpr(rD(ctx->opcode), t2);
+    set_fpr(a->rt, t2);
+    return true;
 }
 
 /***                  Floating-Point status & ctrl register                ***/
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index 5053cb135c..9bc9c3a3c3 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -10,9 +10,6 @@ GEN_STXF(stfiw, st32fiw, 0x17, 0x1E, PPC_FLOAT_STFIWX)
 GEN_HANDLER_E(stfdepx, 0x1F, 0x1F, 0x16, 0x00000001, PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER_E(stfdpx, 0x1F, 0x17, 0x1C, 0x00200001, PPC_NONE, PPC2_ISA205),
 
-GEN_HANDLER_E(fcpsgn, 0x3F, 0x08, 0x00, 0x00000000, PPC_NONE, PPC2_ISA205),
-GEN_HANDLER_E(fmrgew, 0x3F, 0x06, 0x1E, 0x00000001, PPC_NONE, PPC2_VSX207),
-GEN_HANDLER_E(fmrgow, 0x3F, 0x06, 0x1A, 0x00000001, PPC_NONE, PPC2_VSX207),
 GEN_HANDLER(mcrfs, 0x3F, 0x00, 0x02, 0x0063F801, PPC_FLOAT),
 GEN_HANDLER(mtfsb0, 0x3F, 0x06, 0x02, 0x001FF800, PPC_FLOAT),
 GEN_HANDLER(mtfsb1, 0x3F, 0x06, 0x01, 0x001FF800, PPC_FLOAT),
-- 
2.49.0

[PATCH 5/5] MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs

[Chinmay Rath]

I have been working on Power ISA for a long time now and have mostly
contributed in TCG instruction translation area (moved 300+ instructions to
decodetree as of yet) and would like to continue contributing to PPC TCG in
best possible ways I can. I think it's time to step up and assist in reviewing
related patches to enable myself contribute more effectively in this direction.

Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 16af37986a..a2db7d72aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -295,6 +295,7 @@ F: tests/tcg/openrisc/
 PowerPC TCG CPUs
 M: Nicholas Piggin <npiggin@gmail.com>
 M: Daniel Henrique Barboza <danielhb413@gmail.com>
+R: Chinmay Rath <rathc@linux.ibm.com>
 L: qemu-ppc@nongnu.org
 S: Odd Fixes
 F: target/ppc/
-- 
2.49.0

Re: [PATCH 5/5] MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs

[Cédric Le Goater]

On 6/19/25 11:58, Chinmay Rath wrote:
> I have been working on Power ISA for a long time now and have mostly
> contributed in TCG instruction translation area (moved 300+ instructions to
> decodetree as of yet) and would like to continue contributing to PPC TCG in
> best possible ways I can. I think it's time to step up and assist in reviewing
> related patches to enable myself contribute more effectively in this direction.
> 
> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 16af37986a..a2db7d72aa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -295,6 +295,7 @@ F: tests/tcg/openrisc/
>   PowerPC TCG CPUs
>   M: Nicholas Piggin <npiggin@gmail.com>
>   M: Daniel Henrique Barboza <danielhb413@gmail.com>
> +R: Chinmay Rath <rathc@linux.ibm.com>
>   L: qemu-ppc@nongnu.org
>   S: Odd Fixes
>   F: target/ppc/


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH 5/5] MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs

[Chinmay Rath]

On 6/19/25 17:27, Cédric Le Goater wrote:
> On 6/19/25 11:58, Chinmay Rath wrote:
>> I have been working on Power ISA for a long time now and have mostly
>> contributed in TCG instruction translation area (moved 300+ 
>> instructions to
>> decodetree as of yet) and would like to continue contributing to PPC 
>> TCG in
>> best possible ways I can. I think it's time to step up and assist in 
>> reviewing
>> related patches to enable myself contribute more effectively in this 
>> direction.
>>
>> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
>> ---
>>   MAINTAINERS | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 16af37986a..a2db7d72aa 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -295,6 +295,7 @@ F: tests/tcg/openrisc/
>>   PowerPC TCG CPUs
>>   M: Nicholas Piggin <npiggin@gmail.com>
>>   M: Daniel Henrique Barboza <danielhb413@gmail.com>
>> +R: Chinmay Rath <rathc@linux.ibm.com>
>>   L: qemu-ppc@nongnu.org
>>   S: Odd Fixes
>>   F: target/ppc/
>
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
> Thanks,
>
> C. 

Thanks Cedric !

Thanks,

Chinmay

Re: [PATCH 0/5] target/ppc: Move floating-point instructions to decodetree.

[Chinmay Rath]

On 6/19/25 15:28, Chinmay Rath wrote:
> Moving floating point rounding, conversion, compare and move
> instructions to decodetree specification.

Hi Richard,

Could you please share your review comments on this series ?

Thanks,

Chinmay

> Also proposing myself as a reviewer for PowerPC TCG CPUs.
>
> Chinmay Rath (5):
>    target/ppc: Move floating-point rounding and conversion instructions
>      to decodetree.
>    target/ppc: Move floating-point compare instructions to decodetree.
>    target/ppc: Move floating-point move instructions to decodetree.
>    target/ppc: Move remaining floating-point move instructions to
>      decodetree.
>    MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs
>
>   MAINTAINERS                        |   1 +
>   target/ppc/fpu_helper.c            |  38 ++--
>   target/ppc/helper.h                |  38 ++--
>   target/ppc/insn32.decode           |  40 ++++
>   target/ppc/translate/fp-impl.c.inc | 289 ++++++++++-------------------
>   target/ppc/translate/fp-ops.c.inc  |  30 ---
>   6 files changed, 181 insertions(+), 255 deletions(-)
>

Re: [PATCH 2/5] target/ppc: Move floating-point compare instructions to decodetree.

[Richard Henderson]

On 6/19/25 19:58, Chinmay Rath wrote:
> +static bool do_helper_cmp(DisasContext *ctx, arg_X_bf *a,
> +                          void (*helper)(TCGv_env, TCGv_i64, TCGv_i64,
> +                                         TCGv_i32))
> +    REQUIRE_INSNS_FLAGS(ctx, FLOAT);

...

> +TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
> +TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);

It's probably better to standardize on TRANS_FLAGS even though the flags checked is the 
same for both of these.

But anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 0/5] target/ppc: Move floating-point instructions to decodetree.

[Richard Henderson]

On 6/19/25 19:58, Chinmay Rath wrote:
> Chinmay Rath (5):
>    target/ppc: Move floating-point rounding and conversion instructions
>      to decodetree.
>    target/ppc: Move floating-point compare instructions to decodetree.
>    target/ppc: Move floating-point move instructions to decodetree.
>    target/ppc: Move remaining floating-point move instructions to
>      decodetree.
>    MAINTAINERS: Add myself as reviewer for PowerPC TCG CPUs

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

r~

Re: [PATCH 2/5] target/ppc: Move floating-point compare instructions to decodetree.

[Chinmay Rath]

On 8/27/25 12:49, Richard Henderson wrote:
> On 6/19/25 19:58, Chinmay Rath wrote:
>> +static bool do_helper_cmp(DisasContext *ctx, arg_X_bf *a,
>> +                          void (*helper)(TCGv_env, TCGv_i64, TCGv_i64,
>> +                                         TCGv_i32))
>> +    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
>
> ...
>
>> +TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
>> +TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);
>
> It's probably better to standardize on TRANS_FLAGS even though the 
> flags checked is the same for both of these.
Oooh yeah, should absolutely do this. My bad I missed.

Thanks,
Chinmay
>
> But anyway,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~

Re: [PATCH 2/5] target/ppc: Move floating-point compare instructions to decodetree.

[Chinmay Rath]

On 8/27/25 12:49, Richard Henderson wrote:
> On 6/19/25 19:58, Chinmay Rath wrote:
>> +static bool do_helper_cmp(DisasContext *ctx, arg_X_bf *a,
>> +                          void (*helper)(TCGv_env, TCGv_i64, TCGv_i64,
>> +                                         TCGv_i32))
>> +    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
>
> ...
>
>> +TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
>> +TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);
>
> It's probably better to standardize on TRANS_FLAGS even though the 
> flags checked is the same for both of these.
>
Hi Richard,

I did notice in the code that there are quite many instructions using 
TRANS and doing flag check in the common helper.
For example, in target/ppc/translate/fixedpoint-impl.c.inc :

/static bool do_set_bool_cond(DisasContext *ctx, arg_X_bi *a, bool neg, 
bool rev)
{
     REQUIRE_INSNS_FLAGS2(ctx, ISA310);
     uint32_t mask = 0x08 >> (a->bi & 0x03);
     ....
     return true;
}

TRANS(SETBC, do_set_bool_cond, false, false)
TRANS(SETBCR, do_set_bool_cond, false, true)
TRANS(SETNBC, do_set_bool_cond, true, false)
TRANS(SETNBCR, do_set_bool_cond, true, true)
/

Do we want to standardize the use of TRANS_FLAGS in all such existing 
insns ?

I can send a followup patch doing the same for such insns (including the 
ones in this patch)

Thanks,

Chinmay

> But anyway,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~

Re: [PATCH 2/5] target/ppc: Move floating-point compare instructions to decodetree.

[Richard Henderson]

On 9/11/25 02:29, Chinmay Rath wrote:
> 
> On 8/27/25 12:49, Richard Henderson wrote:
>> On 6/19/25 19:58, Chinmay Rath wrote:
>>> +static bool do_helper_cmp(DisasContext *ctx, arg_X_bf *a,
>>> +                          void (*helper)(TCGv_env, TCGv_i64, TCGv_i64,
>>> +                                         TCGv_i32))
>>> +    REQUIRE_INSNS_FLAGS(ctx, FLOAT);
>>
>> ...
>>
>>> +TRANS(FCMPU, do_helper_cmp, gen_helper_FCMPU);
>>> +TRANS(FCMPO, do_helper_cmp, gen_helper_FCMPO);
>>
>> It's probably better to standardize on TRANS_FLAGS even though the flags checked is the 
>> same for both of these.
>>
> Hi Richard,
> 
> I did notice in the code that there are quite many instructions using TRANS and doing flag 
> check in the common helper.
> For example, in target/ppc/translate/fixedpoint-impl.c.inc :
> 
> /static bool do_set_bool_cond(DisasContext *ctx, arg_X_bi *a, bool neg, bool rev)
> {
>      REQUIRE_INSNS_FLAGS2(ctx, ISA310);
>      uint32_t mask = 0x08 >> (a->bi & 0x03);
>      ....
>      return true;
> }
> 
> TRANS(SETBC, do_set_bool_cond, false, false)
> TRANS(SETBCR, do_set_bool_cond, false, true)
> TRANS(SETNBC, do_set_bool_cond, true, false)
> TRANS(SETNBCR, do_set_bool_cond, true, true)
> /
> 
> Do we want to standardize the use of TRANS_FLAGS in all such existing insns ?

Yes, I think so.


r~