* [PATCH 1/3] target/ppc: Move fsqrt to decodetree
2022-09-05 12:37 [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Víctor Colombo
@ 2022-09-05 12:37 ` Víctor Colombo
2022-09-05 15:48 ` Richard Henderson
2022-09-05 12:37 ` [PATCH 2/3] target/ppc: Move fsqrts " Víctor Colombo
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-05 12:37 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson,
victor.colombo, matheus.ferst, lucas.araujo, leandro.lupori,
lucas.coutinho
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
target/ppc/insn32.decode | 7 +++++++
target/ppc/translate/fp-impl.c.inc | 29 +++++++++++++++++------------
target/ppc/translate/fp-ops.c.inc | 1 -
3 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index eb41efc100..b55d1550f3 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -20,6 +20,9 @@
&A frt fra frb frc rc:bool
@A ...... frt:5 fra:5 frb:5 frc:5 ..... rc:1 &A
+&A_tb frt frb rc:bool
+@A_tb ...... frt:5 ..... frb:5 ..... ..... rc:1 &A_tb
+
&D rt ra si:int64_t
@D ...... rt:5 ra:5 si:s16 &D
@@ -353,6 +356,10 @@ STFDU 110111 ..... ...... ............... @D
STFDX 011111 ..... ...... .... 1011010111 - @X
STFDUX 011111 ..... ...... .... 1011110111 - @X
+### Floating-Point Arithmetic Instructions
+
+FSQRT 111111 ..... ----- ..... ----- 10110 . @A_tb
+
### 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 0e893eafa7..e8359af005 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -254,29 +254,34 @@ static bool trans_FSEL(DisasContext *ctx, arg_A *a)
GEN_FLOAT_AB(sub, 0x14, 0x000007C0, 1, PPC_FLOAT);
/* Optional: */
-/* fsqrt */
-static void gen_fsqrt(DisasContext *ctx)
+static bool do_helper_fsqrt(DisasContext *ctx, arg_A_tb *a,
+ void (*helper)(TCGv_i64, TCGv_ptr, TCGv_i64))
{
- 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_FSQRT);
+ REQUIRE_FPU(ctx);
+
t0 = tcg_temp_new_i64();
t1 = tcg_temp_new_i64();
+
gen_reset_fpstatus();
- get_fpr(t0, rB(ctx->opcode));
- gen_helper_fsqrt(t1, cpu_env, t0);
- set_fpr(rD(ctx->opcode), t1);
+ get_fpr(t0, a->frb);
+ helper(t1, cpu_env, t0);
+ set_fpr(a->frt, t1);
gen_compute_fprf_float64(t1);
- if (unlikely(Rc(ctx->opcode) != 0)) {
+ if (unlikely(a->rc != 0)) {
gen_set_cr1_from_fpscr(ctx);
}
+
tcg_temp_free_i64(t0);
tcg_temp_free_i64(t1);
+
+ return true;
}
+TRANS(FSQRT, do_helper_fsqrt, gen_helper_fsqrt);
+
static void gen_fsqrts(DisasContext *ctx)
{
TCGv_i64 t0;
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index 1b65f5ab73..38759f5939 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -62,7 +62,6 @@ 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(frsqrtes, 0x3B, 0x1A, 0xFF, 0x001F07C0, PPC_FLOAT_FRSQRTES),
-GEN_HANDLER(fsqrt, 0x3F, 0x16, 0xFF, 0x001F07C0, PPC_FLOAT_FSQRT),
GEN_HANDLER(fsqrts, 0x3B, 0x16, 0xFF, 0x001F07C0, PPC_FLOAT_FSQRT),
GEN_HANDLER(fcmpo, 0x3F, 0x00, 0x01, 0x00600001, PPC_FLOAT),
GEN_HANDLER(fcmpu, 0x3F, 0x00, 0x00, 0x00600001, PPC_FLOAT),
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] target/ppc: Move fsqrt to decodetree
2022-09-05 12:37 ` [PATCH 1/3] target/ppc: Move fsqrt " Víctor Colombo
@ 2022-09-05 15:48 ` Richard Henderson
0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-09-05 15:48 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/5/22 13:37, Víctor Colombo wrote:
> Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br>
> ---
> target/ppc/insn32.decode | 7 +++++++
> target/ppc/translate/fp-impl.c.inc | 29 +++++++++++++++++------------
> target/ppc/translate/fp-ops.c.inc | 1 -
> 3 files changed, 24 insertions(+), 13 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] target/ppc: Move fsqrts to decodetree
2022-09-05 12:37 [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Víctor Colombo
2022-09-05 12:37 ` [PATCH 1/3] target/ppc: Move fsqrt " Víctor Colombo
@ 2022-09-05 12:37 ` Víctor Colombo
2022-09-05 15:55 ` Richard Henderson
2022-09-05 12:37 ` [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers Víctor Colombo
2022-09-06 20:07 ` [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Daniel Henrique Barboza
3 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-05 12:37 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson,
victor.colombo, matheus.ferst, lucas.araujo, leandro.lupori,
lucas.coutinho
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
target/ppc/insn32.decode | 1 +
target/ppc/translate/fp-impl.c.inc | 23 +----------------------
target/ppc/translate/fp-ops.c.inc | 1 -
3 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index b55d1550f3..7eb808a43f 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -359,6 +359,7 @@ STFDUX 011111 ..... ...... .... 1011110111 - @X
### Floating-Point Arithmetic Instructions
FSQRT 111111 ..... ----- ..... ----- 10110 . @A_tb
+FSQRTS 111011 ..... ----- ..... ----- 10110 . @A_tb
### Floating-Point Select Instruction
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index e8359af005..7a90c0e350 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -281,28 +281,7 @@ static bool do_helper_fsqrt(DisasContext *ctx, arg_A_tb *a,
}
TRANS(FSQRT, do_helper_fsqrt, gen_helper_fsqrt);
-
-static void gen_fsqrts(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_fsqrts(t1, cpu_env, t0);
- set_fpr(rD(ctx->opcode), t1);
- gen_compute_fprf_float64(t1);
- if (unlikely(Rc(ctx->opcode) != 0)) {
- gen_set_cr1_from_fpscr(ctx);
- }
- tcg_temp_free_i64(t0);
- tcg_temp_free_i64(t1);
-}
+TRANS(FSQRTS, do_helper_fsqrt, gen_helper_fsqrts);
/*** Floating-Point multiply-and-add ***/
/* fmadd - fmadds */
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index 38759f5939..d4c6c4bed1 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -62,7 +62,6 @@ 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(frsqrtes, 0x3B, 0x1A, 0xFF, 0x001F07C0, PPC_FLOAT_FRSQRTES),
-GEN_HANDLER(fsqrts, 0x3B, 0x16, 0xFF, 0x001F07C0, PPC_FLOAT_FSQRT),
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),
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] target/ppc: Move fsqrts to decodetree
2022-09-05 12:37 ` [PATCH 2/3] target/ppc: Move fsqrts " Víctor Colombo
@ 2022-09-05 15:55 ` Richard Henderson
0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-09-05 15:55 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/5/22 13:37, Víctor Colombo wrote:
> Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br>
> ---
> target/ppc/insn32.decode | 1 +
> target/ppc/translate/fp-impl.c.inc | 23 +----------------------
> target/ppc/translate/fp-ops.c.inc | 1 -
> 3 files changed, 2 insertions(+), 23 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 12:37 [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Víctor Colombo
2022-09-05 12:37 ` [PATCH 1/3] target/ppc: Move fsqrt " Víctor Colombo
2022-09-05 12:37 ` [PATCH 2/3] target/ppc: Move fsqrts " Víctor Colombo
@ 2022-09-05 12:37 ` Víctor Colombo
2022-09-05 15:56 ` Richard Henderson
2022-09-06 20:07 ` [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Daniel Henrique Barboza
3 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-05 12:37 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson,
victor.colombo, matheus.ferst, lucas.araujo, leandro.lupori,
lucas.coutinho
These two helpers are almost identical, differing only by the softfloat
operation it calls. Merge them into one using a macro.
Also, take this opportunity to capitalize the helper name as we moved
the instruction to decodetree in a previous patch.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
target/ppc/fpu_helper.c | 35 +++++++++++-------------------
target/ppc/helper.h | 4 ++--
target/ppc/translate/fp-impl.c.inc | 4 ++--
3 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0f045b70f8..32995179b5 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -830,30 +830,21 @@ static void float_invalid_op_sqrt(CPUPPCState *env, int flags,
}
}
-/* fsqrt - fsqrt. */
-float64 helper_fsqrt(CPUPPCState *env, float64 arg)
-{
- float64 ret = float64_sqrt(arg, &env->fp_status);
- int flags = get_float_exception_flags(&env->fp_status);
-
- if (unlikely(flags & float_flag_invalid)) {
- float_invalid_op_sqrt(env, flags, 1, GETPC());
- }
-
- return ret;
+#define FPU_FSQRT(name, op) \
+float64 helper_##name(CPUPPCState *env, float64 arg) \
+{ \
+ float64 ret = op(arg, &env->fp_status); \
+ int flags = get_float_exception_flags(&env->fp_status); \
+ \
+ if (unlikely(flags & float_flag_invalid)) { \
+ float_invalid_op_sqrt(env, flags, 1, GETPC()); \
+ } \
+ \
+ return ret; \
}
-/* fsqrts - fsqrts. */
-float64 helper_fsqrts(CPUPPCState *env, float64 arg)
-{
- float64 ret = float64r32_sqrt(arg, &env->fp_status);
- int flags = get_float_exception_flags(&env->fp_status);
-
- if (unlikely(flags & float_flag_invalid)) {
- float_invalid_op_sqrt(env, flags, 1, GETPC());
- }
- return ret;
-}
+FPU_FSQRT(FSQRT, float64_sqrt)
+FPU_FSQRT(FSQRTS, float64r32_sqrt)
/* fre - fre. */
float64 helper_fre(CPUPPCState *env, float64 arg)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 159b352f6e..68610896b8 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -116,8 +116,8 @@ DEF_HELPER_4(fmadds, i64, env, i64, i64, i64)
DEF_HELPER_4(fmsubs, i64, env, i64, i64, i64)
DEF_HELPER_4(fnmadds, i64, env, i64, i64, i64)
DEF_HELPER_4(fnmsubs, i64, env, i64, i64, i64)
-DEF_HELPER_2(fsqrt, f64, env, f64)
-DEF_HELPER_2(fsqrts, f64, env, f64)
+DEF_HELPER_2(FSQRT, f64, env, f64)
+DEF_HELPER_2(FSQRTS, f64, env, f64)
DEF_HELPER_2(fre, i64, env, i64)
DEF_HELPER_2(fres, i64, env, i64)
DEF_HELPER_2(frsqrte, i64, env, i64)
diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index 7a90c0e350..8d5cf0f982 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -280,8 +280,8 @@ static bool do_helper_fsqrt(DisasContext *ctx, arg_A_tb *a,
return true;
}
-TRANS(FSQRT, do_helper_fsqrt, gen_helper_fsqrt);
-TRANS(FSQRTS, do_helper_fsqrt, gen_helper_fsqrts);
+TRANS(FSQRT, do_helper_fsqrt, gen_helper_FSQRT);
+TRANS(FSQRTS, do_helper_fsqrt, gen_helper_FSQRTS);
/*** Floating-Point multiply-and-add ***/
/* fmadd - fmadds */
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 12:37 ` [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers Víctor Colombo
@ 2022-09-05 15:56 ` Richard Henderson
2022-09-05 16:19 ` Víctor Colombo
0 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2022-09-05 15:56 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/5/22 13:37, Víctor Colombo wrote:
> These two helpers are almost identical, differing only by the softfloat
> operation it calls. Merge them into one using a macro.
> Also, take this opportunity to capitalize the helper name as we moved
> the instruction to decodetree in a previous patch.
>
> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
> ---
> target/ppc/fpu_helper.c | 35 +++++++++++-------------------
> target/ppc/helper.h | 4 ++--
> target/ppc/translate/fp-impl.c.inc | 4 ++--
> 3 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 0f045b70f8..32995179b5 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -830,30 +830,21 @@ static void float_invalid_op_sqrt(CPUPPCState *env, int flags,
> }
> }
>
> -/* fsqrt - fsqrt. */
> -float64 helper_fsqrt(CPUPPCState *env, float64 arg)
> -{
> - float64 ret = float64_sqrt(arg, &env->fp_status);
> - int flags = get_float_exception_flags(&env->fp_status);
> -
> - if (unlikely(flags & float_flag_invalid)) {
> - float_invalid_op_sqrt(env, flags, 1, GETPC());
> - }
> -
> - return ret;
> +#define FPU_FSQRT(name, op) \
> +float64 helper_##name(CPUPPCState *env, float64 arg) \
> +{ \
> + float64 ret = op(arg, &env->fp_status); \
> + int flags = get_float_exception_flags(&env->fp_status); \
> + \
> + if (unlikely(flags & float_flag_invalid)) { \
> + float_invalid_op_sqrt(env, flags, 1, GETPC()); \
> + } \
Existing bug, but this is missing to clear fp status to start.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
> + \
> + return ret; \
> }
>
> -/* fsqrts - fsqrts. */
> -float64 helper_fsqrts(CPUPPCState *env, float64 arg)
> -{
> - float64 ret = float64r32_sqrt(arg, &env->fp_status);
> - int flags = get_float_exception_flags(&env->fp_status);
> -
> - if (unlikely(flags & float_flag_invalid)) {
> - float_invalid_op_sqrt(env, flags, 1, GETPC());
> - }
> - return ret;
> -}
> +FPU_FSQRT(FSQRT, float64_sqrt)
> +FPU_FSQRT(FSQRTS, float64r32_sqrt)
>
> /* fre - fre. */
> float64 helper_fre(CPUPPCState *env, float64 arg)
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 159b352f6e..68610896b8 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -116,8 +116,8 @@ DEF_HELPER_4(fmadds, i64, env, i64, i64, i64)
> DEF_HELPER_4(fmsubs, i64, env, i64, i64, i64)
> DEF_HELPER_4(fnmadds, i64, env, i64, i64, i64)
> DEF_HELPER_4(fnmsubs, i64, env, i64, i64, i64)
> -DEF_HELPER_2(fsqrt, f64, env, f64)
> -DEF_HELPER_2(fsqrts, f64, env, f64)
> +DEF_HELPER_2(FSQRT, f64, env, f64)
> +DEF_HELPER_2(FSQRTS, f64, env, f64)
> DEF_HELPER_2(fre, i64, env, i64)
> DEF_HELPER_2(fres, i64, env, i64)
> DEF_HELPER_2(frsqrte, i64, env, i64)
> diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
> index 7a90c0e350..8d5cf0f982 100644
> --- a/target/ppc/translate/fp-impl.c.inc
> +++ b/target/ppc/translate/fp-impl.c.inc
> @@ -280,8 +280,8 @@ static bool do_helper_fsqrt(DisasContext *ctx, arg_A_tb *a,
> return true;
> }
>
> -TRANS(FSQRT, do_helper_fsqrt, gen_helper_fsqrt);
> -TRANS(FSQRTS, do_helper_fsqrt, gen_helper_fsqrts);
> +TRANS(FSQRT, do_helper_fsqrt, gen_helper_FSQRT);
> +TRANS(FSQRTS, do_helper_fsqrt, gen_helper_FSQRTS);
>
> /*** Floating-Point multiply-and-add ***/
> /* fmadd - fmadds */
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 15:56 ` Richard Henderson
@ 2022-09-05 16:19 ` Víctor Colombo
2022-09-05 16:21 ` Richard Henderson
0 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-05 16:19 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 05/09/2022 12:56, Richard Henderson wrote:
> On 9/5/22 13:37, Víctor Colombo wrote:
>> These two helpers are almost identical, differing only by the softfloat
>> operation it calls. Merge them into one using a macro.
>> Also, take this opportunity to capitalize the helper name as we moved
>> the instruction to decodetree in a previous patch.
>>
>> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
>> ---
>> target/ppc/fpu_helper.c | 35 +++++++++++-------------------
>> target/ppc/helper.h | 4 ++--
>> target/ppc/translate/fp-impl.c.inc | 4 ++--
>> 3 files changed, 17 insertions(+), 26 deletions(-)
>>
>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> index 0f045b70f8..32995179b5 100644
>> --- a/target/ppc/fpu_helper.c
>> +++ b/target/ppc/fpu_helper.c
>> @@ -830,30 +830,21 @@ static void float_invalid_op_sqrt(CPUPPCState
>> *env, int flags,
>> }
>> }
>>
>> -/* fsqrt - fsqrt. */
>> -float64 helper_fsqrt(CPUPPCState *env, float64 arg)
>> -{
>> - float64 ret = float64_sqrt(arg, &env->fp_status);
>> - int flags = get_float_exception_flags(&env->fp_status);
>> -
>> - if (unlikely(flags & float_flag_invalid)) {
>> - float_invalid_op_sqrt(env, flags, 1, GETPC());
>> - }
>> -
>> - return ret;
>> +#define FPU_FSQRT(name,
>> op) \
>> +float64 helper_##name(CPUPPCState *env, float64
>> arg) \
>> +{
>> \
>> + float64 ret = op(arg,
>> &env->fp_status); \
>> + int flags =
>> get_float_exception_flags(&env->fp_status); \
>> +
>> \
>> + if (unlikely(flags & float_flag_invalid))
>> { \
>> + float_invalid_op_sqrt(env, flags, 1,
>> GETPC()); \
>> +
>> }
>> \
>
> Existing bug, but this is missing to clear fp status to start.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> r~
>
Hello Richard, thanks for your review!
gen_reset_fpstatus() is called by the inline implementation in
do_helper_fsqrt() before calling the helper (patch 1).
It's probably better to move the call to inside the helper.
>> +
>> \
>> + return
>> ret; \
>> }
>>
>> -/* fsqrts - fsqrts. */
>> -float64 helper_fsqrts(CPUPPCState *env, float64 arg)
>> -{
>> - float64 ret = float64r32_sqrt(arg, &env->fp_status);
>> - int flags = get_float_exception_flags(&env->fp_status);
>> -
>> - if (unlikely(flags & float_flag_invalid)) {
>> - float_invalid_op_sqrt(env, flags, 1, GETPC());
>> - }
>> - return ret;
>> -}
>> +FPU_FSQRT(FSQRT, float64_sqrt)
>> +FPU_FSQRT(FSQRTS, float64r32_sqrt)
>>
>> /* fre - fre. */
>> float64 helper_fre(CPUPPCState *env, float64 arg)
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index 159b352f6e..68610896b8 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -116,8 +116,8 @@ DEF_HELPER_4(fmadds, i64, env, i64, i64, i64)
>> DEF_HELPER_4(fmsubs, i64, env, i64, i64, i64)
>> DEF_HELPER_4(fnmadds, i64, env, i64, i64, i64)
>> DEF_HELPER_4(fnmsubs, i64, env, i64, i64, i64)
>> -DEF_HELPER_2(fsqrt, f64, env, f64)
>> -DEF_HELPER_2(fsqrts, f64, env, f64)
>> +DEF_HELPER_2(FSQRT, f64, env, f64)
>> +DEF_HELPER_2(FSQRTS, f64, env, f64)
>> DEF_HELPER_2(fre, i64, env, i64)
>> DEF_HELPER_2(fres, i64, env, i64)
>> DEF_HELPER_2(frsqrte, i64, env, i64)
>> diff --git a/target/ppc/translate/fp-impl.c.inc
>> b/target/ppc/translate/fp-impl.c.inc
>> index 7a90c0e350..8d5cf0f982 100644
>> --- a/target/ppc/translate/fp-impl.c.inc
>> +++ b/target/ppc/translate/fp-impl.c.inc
>> @@ -280,8 +280,8 @@ static bool do_helper_fsqrt(DisasContext *ctx,
>> arg_A_tb *a,
>> return true;
>> }
>>
>> -TRANS(FSQRT, do_helper_fsqrt, gen_helper_fsqrt);
>> -TRANS(FSQRTS, do_helper_fsqrt, gen_helper_fsqrts);
>> +TRANS(FSQRT, do_helper_fsqrt, gen_helper_FSQRT);
>> +TRANS(FSQRTS, do_helper_fsqrt, gen_helper_FSQRTS);
>>
>> /*** Floating-Point
>> multiply-and-add ***/
>> /* fmadd - fmadds */
>
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 16:19 ` Víctor Colombo
@ 2022-09-05 16:21 ` Richard Henderson
2022-09-05 16:31 ` Víctor Colombo
0 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2022-09-05 16:21 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/5/22 17:19, Víctor Colombo wrote:
>> Existing bug, but this is missing to clear fp status to start.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> r~
>>
>
> Hello Richard, thanks for your review!
> gen_reset_fpstatus() is called by the inline implementation in
> do_helper_fsqrt() before calling the helper (patch 1).
Oops, ok.
> It's probably better to move the call to inside the helper.
I did write about a scheme by which all of these calls should go away. I guess it has
been a while...
r~
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 16:21 ` Richard Henderson
@ 2022-09-05 16:31 ` Víctor Colombo
2022-09-05 17:20 ` Richard Henderson
0 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-05 16:31 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 05/09/2022 13:21, Richard Henderson wrote:
> On 9/5/22 17:19, Víctor Colombo wrote:
>>> Existing bug, but this is missing to clear fp status to start.
>>>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>
>>> r~
>>>
>>
>> Hello Richard, thanks for your review!
>> gen_reset_fpstatus() is called by the inline implementation in
>> do_helper_fsqrt() before calling the helper (patch 1).
>
> Oops, ok.
>
>
>> It's probably better to move the call to inside the helper.
>
> I did write about a scheme by which all of these calls should go away.
> I guess it has
> been a while...
>
>
> r~
I have a message bookmarked here ([1]), but I don't know if there is a
previous one with a more in depth scheme.
Anyway, I was also analyzing recently the idea of removing all these
reset_fpstatus() calls from instructions helpers. I think this would
require to actually call it from the end of the (previous) instructions
instead of the beginning? Like adding the call to
do_float_check_status() and float_invalid_op_*() as a focal point to
'hide' the calls to reset_fpstatus(). However there are also insns
helpers that don't call these auxiliary functions, which I think would
cause the refactor to not be worthy overall.
Did you have another idea that could be simpler?
[1] https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg00064.html
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 16:31 ` Víctor Colombo
@ 2022-09-05 17:20 ` Richard Henderson
2022-09-06 14:22 ` Víctor Colombo
0 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2022-09-05 17:20 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/5/22 17:31, Víctor Colombo wrote:
> I have a message bookmarked here ([1]), but I don't know if there is a
> previous one with a more in depth scheme.
There may have been a previous, but that's the one I was thinking of.
> Anyway, I was also analyzing recently the idea of removing all these
> reset_fpstatus() calls from instructions helpers. I think this would
> require to actually call it from the end of the (previous) instructions instead of the
> beginning? Like adding the call to
> do_float_check_status() and float_invalid_op_*() as a focal point to
> 'hide' the calls to reset_fpstatus().
Well, there would of course be no separate call, but do_float_check_status would:
int status = get_float_exception_flags(&env->fp_status);
set_float_exception_flags(0, &env->fp_status);
straight away. No extra call overhead, and the steady-state of softfp exception flags
outside of an in-progress fp operation is 0.
> However there are also insns
> helpers that don't call these auxiliary functions, which I think would
> cause the refactor to not be worthy overall.
Anything that can raise a softfp exception and doesn't do something with it, either
immediately within the same helper, or just afterward with helper_float_check_status, is
buggy. With those fixed, helper_reset_fpstatus may be removed entirely.
r~
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-05 17:20 ` Richard Henderson
@ 2022-09-06 14:22 ` Víctor Colombo
2022-09-08 7:52 ` Richard Henderson
0 siblings, 1 reply; 14+ messages in thread
From: Víctor Colombo @ 2022-09-06 14:22 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 05/09/2022 14:20, Richard Henderson wrote:
> Well, there would of course be no separate call, but
I didn't understand what you meant here with 'no separate call'...
> do_float_check_status would:
>
> int status = get_float_exception_flags(&env->fp_status);
>
> set_float_exception_flags(0, &env->fp_status);
>
> straight away. No extra call overhead, and the steady-state of softfp
> exception flags
> outside of an in-progress fp operation is 0.
>
Right, makes sense. And what about when an invalid operation occurs,
with the corresponding exception enabled bit set?
float_invalid_op_* would stop the execution and do_float_check_status
would not be called, right? So it would require to call
set_float_exception_flags there too, correct?
If that's all that's necessary, I might be able to take a look at it and
come with a possible patch.
> Anything that can raise a softfp exception and doesn't do something with
> it, either
> immediately within the same helper, or just afterward with
> helper_float_check_status, is
> buggy. With those fixed, helper_reset_fpstatus may be removed entirely.
>
Oh, that makes sense. It's easier to implement the idea using this
assumption.
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers
2022-09-06 14:22 ` Víctor Colombo
@ 2022-09-08 7:52 ` Richard Henderson
0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-09-08 7:52 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/6/22 15:22, Víctor Colombo wrote:
> On 05/09/2022 14:20, Richard Henderson wrote:
>> Well, there would of course be no separate call, but
>
> I didn't understand what you meant here with 'no separate call'...
We generate a separate call from tcg to helper_reset_fpstatus sometimes.
> Right, makes sense. And what about when an invalid operation occurs,
> with the corresponding exception enabled bit set?
> float_invalid_op_* would stop the execution and do_float_check_status
> would not be called, right? So it would require to call
> set_float_exception_flags there too, correct?
Correct.
r~
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree
2022-09-05 12:37 [PATCH 0/3] target/ppc: Move fsqrt[s] to decodetree Víctor Colombo
` (2 preceding siblings ...)
2022-09-05 12:37 ` [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers Víctor Colombo
@ 2022-09-06 20:07 ` Daniel Henrique Barboza
3 siblings, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2022-09-06 20:07 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, david, groug, richard.henderson, matheus.ferst,
lucas.araujo, leandro.lupori, lucas.coutinho
Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,
Daniel
On 9/5/22 09:37, Víctor Colombo wrote:
> Move fsqrt and fsqrts instructions from decode legacy to decodetree.
>
> Víctor Colombo (3):
> target/ppc: Move fsqrt to decodetree
> target/ppc: Move fsqrts to decodetree
> target/ppc: Merge fsqrt and fsqrts helpers
>
> target/ppc/fpu_helper.c | 35 ++++++++-------------
> target/ppc/helper.h | 4 +--
> target/ppc/insn32.decode | 8 +++++
> target/ppc/translate/fp-impl.c.inc | 50 ++++++++++--------------------
> target/ppc/translate/fp-ops.c.inc | 2 --
> 5 files changed, 40 insertions(+), 59 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread