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.0On 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~
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~
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~
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~
© 2016 - 2025 Red Hat, Inc.