some common code needs to store information in fpscr, but this function
relies on TCG cde to work. This patch adds a kvm way to do it, and a
transparent way to call it when TCG is not compiled
Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
---
target/ppc/gdbstub.c | 1 +
target/ppc/kvm.c | 14 ++++++++++++++
target/ppc/kvm_ppc.h | 6 ++++++
3 files changed, 21 insertions(+)
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 17e41fc113..65759e0c1c 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -23,6 +23,7 @@
#include "exec/helper-proto.h"
#include "internal.h"
#include "helper_regs.h"
+#include "kvm_ppc.h"
static int ppc_gdb_register_len_apple(int n)
{
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 104a308abb..a8a720eb48 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
{
return true;
}
+
+void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
+{
+ CPUState *cs = env_cpu(env);
+ struct kvm_one_reg reg;
+ int ret;
+ reg.id = KVM_REG_PPC_FPSCR;
+ reg.addr = (uintptr_t) &env->fpscr;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret < 0)
+ {
+ fprintf(stderr, "Unable to set FPSCR to KVM: %s", strerror(errno));
+ }
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 989f61ace0..ff365e57a6 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -86,6 +86,12 @@ void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
+#ifndef CONFIG_TCG
+# define store_fpscr kvmppc_store_fpscr
+#endif
+void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask);
+
+
#else
static inline uint32_t kvmppc_get_tbfreq(void)
--
2.17.1
On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb..a8a720eb48 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
> {
> return true;
> }
> +
> +void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
> +{
> + CPUState *cs = env_cpu(env);
> + struct kvm_one_reg reg;
> + int ret;
> + reg.id = KVM_REG_PPC_FPSCR;
> + reg.addr = (uintptr_t) &env->fpscr;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if (ret < 0)
> + {
> + fprintf(stderr, "Unable to set FPSCR to KVM: %s", strerror(errno));
> + }
> +}
This is all unnecessary. All you need to do is store to env->fpscr and the
value will be synced back with kvm_put_fp.
I'll note that some of the trouble you may be having with extracting
helper_store_fpscr to a ppc_store_fpscr function is due to an existing bug in
the tcg code:
Storing to fpscr should *never* raise an exception -- see MTFSF, MTFSB0,
MTFSB1. Thus the mucking about with cs->exception_index and env->error_code is
incorrect.
In addition, the masking is being done weirdly and could use a complete overhaul.
This could all be rewritten as
-- %< -- cpu.h
/* Invalid operation exception summary */
- #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
+ #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
-- %< -- cpu.c
// move fpscr_set_rounding_mode here
void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
{
/* Recompute exception summary state. */
val &= ~(FP_VX | FP_FEX);
if (val & FPSCR_IX) {
val |= FP_VX;
}
if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
val |= FP_FEX;
}
env->fpscr = val;
if (tcg_enabled()) {
fpscr_set_rounding_mode(env);
}
}
-- %< -- fpu_helper.c
void helper_store_fpscr(CPUPPCState *env, target_ulong val,
uint32_t nibbles)
{
target_ulong mask = 0;
/* TODO: Push this expansion back to translation time. */
for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
if (nibbles & (1 << i)) {
mask |= (target_ulong)0xf << (4 * i);
}
}
val = (val & mask) | (env->fpscr & ~mask);
ppc_store_fpscr(env, val);
}
void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
{
uint32_t mask = 1u << bit;
if (env->fpscr & mask) {
ppc_store_fpscr(env, env->fpscr & ~mask);
}
}
void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
{
uint32_t mask = 1u << bit;
if (!(env->fpscr & mask)) {
ppc_store_fpscr(env, env->fpscr | mask);
}
}
There are a couple of other uses of fpscr_set_rounding_mode, where the
softfloat value is changed temporarily (do_fri, VSX_ROUND). These should
simply save the previous softfloat value (using get_float_rounding_mode) around
the operation instead of re-computing from fpscr.
Which leaves us with exactly one use of fpscr_set_rounding_mode, which can then
be moved to cpu.c next to ppc_store_fpscr.
r~
On 12/05/2021 15:20, Richard Henderson wrote:
> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 104a308abb..a8a720eb48 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
>> {
>> return true;
>> }
>> +
>> +void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
>> +{
>> + CPUState *cs = env_cpu(env);
>> + struct kvm_one_reg reg;
>> + int ret;
>> + reg.id = KVM_REG_PPC_FPSCR;
>> + reg.addr = (uintptr_t) &env->fpscr;
>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>> + if (ret < 0)
>> + {
>> + fprintf(stderr, "Unable to set FPSCR to KVM: %s",
>> strerror(errno));
>> + }
>> +}
>
> This is all unnecessary. All you need to do is store to env->fpscr
> and the value will be synced back with kvm_put_fp.
I figured this was too complicated again, but didn't have a better idea
>
> I'll note that some of the trouble you may be having with extracting
> helper_store_fpscr to a ppc_store_fpscr function is due to an existing
> bug in the tcg code:
>
> Storing to fpscr should *never* raise an exception -- see MTFSF,
> MTFSB0, MTFSB1. Thus the mucking about with cs->exception_index and
> env->error_code is incorrect.
>
> In addition, the masking is being done weirdly and could use a
> complete overhaul.
>
> This could all be rewritten as
>
> -- %< -- cpu.h
>
> /* Invalid operation exception summary */
> - #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
> + #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
>
> -- %< -- cpu.c
>
> // move fpscr_set_rounding_mode here
>
> void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
> {
> /* Recompute exception summary state. */
> val &= ~(FP_VX | FP_FEX);
> if (val & FPSCR_IX) {
> val |= FP_VX;
> }
> if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
> val |= FP_FEX;
> }
> env->fpscr = val;
> if (tcg_enabled()) {
> fpscr_set_rounding_mode(env);
> }
> }
>
> -- %< -- fpu_helper.c
>
> void helper_store_fpscr(CPUPPCState *env, target_ulong val,
> uint32_t nibbles)
> {
> target_ulong mask = 0;
>
> /* TODO: Push this expansion back to translation time. */
> for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
> if (nibbles & (1 << i)) {
> mask |= (target_ulong)0xf << (4 * i);
> }
> }
>
> val = (val & mask) | (env->fpscr & ~mask);
> ppc_store_fpscr(env, val);
> }
>
> void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
> {
> uint32_t mask = 1u << bit;
> if (env->fpscr & mask) {
> ppc_store_fpscr(env, env->fpscr & ~mask);
> }
> }
>
> void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
> {
> uint32_t mask = 1u << bit;
> if (!(env->fpscr & mask)) {
> ppc_store_fpscr(env, env->fpscr | mask);
> }
> }
>
> There are a couple of other uses of fpscr_set_rounding_mode, where the
> softfloat value is changed temporarily (do_fri, VSX_ROUND). These
> should simply save the previous softfloat value (using
> get_float_rounding_mode) around the operation instead of re-computing
> from fpscr.
>
> Which leaves us with exactly one use of fpscr_set_rounding_mode, which
> can then be moved to cpu.c next to ppc_store_fpscr.
>
ok, yeah, I can definitely try this.
>
> r~
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
On 12/05/2021 15:20, Richard Henderson wrote:
> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 104a308abb..a8a720eb48 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
>> {
>> return true;
>> }
>> +
>> +void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
>> +{
>> + CPUState *cs = env_cpu(env);
>> + struct kvm_one_reg reg;
>> + int ret;
>> + reg.id = KVM_REG_PPC_FPSCR;
>> + reg.addr = (uintptr_t) &env->fpscr;
>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>> + if (ret < 0)
>> + {
>> + fprintf(stderr, "Unable to set FPSCR to KVM: %s",
>> strerror(errno));
>> + }
>> +}
>
> This is all unnecessary. All you need to do is store to env->fpscr
> and the value will be synced back with kvm_put_fp.
>
> I'll note that some of the trouble you may be having with extracting
> helper_store_fpscr to a ppc_store_fpscr function is due to an existing
> bug in the tcg code:
>
> Storing to fpscr should *never* raise an exception -- see MTFSF,
> MTFSB0, MTFSB1. Thus the mucking about with cs->exception_index and
> env->error_code is incorrect.
>
> In addition, the masking is being done weirdly and could use a
> complete overhaul.
>
> This could all be rewritten as
>
> -- %< -- cpu.h
>
> /* Invalid operation exception summary */
> - #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
> + #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
>
> -- %< -- cpu.c
>
> // move fpscr_set_rounding_mode here
>
> void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
> {
> /* Recompute exception summary state. */
> val &= ~(FP_VX | FP_FEX);
> if (val & FPSCR_IX) {
> val |= FP_VX;
> }
> if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
> val |= FP_FEX;
> }
> env->fpscr = val;
> if (tcg_enabled()) {
> fpscr_set_rounding_mode(env);
> }
> }
>
> -- %< -- fpu_helper.c
>
> void helper_store_fpscr(CPUPPCState *env, target_ulong val,
> uint32_t nibbles)
> {
> target_ulong mask = 0;
>
> /* TODO: Push this expansion back to translation time. */
> for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
> if (nibbles & (1 << i)) {
> mask |= (target_ulong)0xf << (4 * i);
> }
> }
>
> val = (val & mask) | (env->fpscr & ~mask);
> ppc_store_fpscr(env, val);
> }
That expansion can't be moved to translation time, because gdbstub would
also need that code in a variety of functions, so better to keep it in
that central location,
>
> void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
> {
> uint32_t mask = 1u << bit;
> if (env->fpscr & mask) {
> ppc_store_fpscr(env, env->fpscr & ~mask);
> }
> }
>
> void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
> {
> uint32_t mask = 1u << bit;
> if (!(env->fpscr & mask)) {
> ppc_store_fpscr(env, env->fpscr | mask);
> }
> }
>
> There are a couple of other uses of fpscr_set_rounding_mode, where the
> softfloat value is changed temporarily (do_fri, VSX_ROUND). These
> should simply save the previous softfloat value (using
> get_float_rounding_mode) around the operation instead of re-computing
> from fpscr.
>
> Which leaves us with exactly one use of fpscr_set_rounding_mode, which
> can then be moved to cpu.c next to ppc_store_fpscr.
>
>
> r~
I was implementing this solution, but ran into a problem: We needed
store_fpscr for gdbstub.c, that's the original reason we made that new
function to begin with. This solution, although it improves the handling
of fpscr, doesn't fix the original problem.
What I think we can do is put the logic that is in helper_store_fpscr
into store_fpscr, move it to cpu.c, and have the helper call the
non-helper function. That way we conserve helper_* as TCG-specific and
have the overhaul.
Toughts?
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
On 5/13/21 11:36 AM, Bruno Piazera Larsen wrote:
>
> On 12/05/2021 15:20, Richard Henderson wrote:
>> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>>> index 104a308abb..a8a720eb48 100644
>>> --- a/target/ppc/kvm.c
>>> +++ b/target/ppc/kvm.c
>>> @@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
>>> {
>>> return true;
>>> }
>>> +
>>> +void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
>>> +{
>>> + CPUState *cs = env_cpu(env);
>>> + struct kvm_one_reg reg;
>>> + int ret;
>>> + reg.id = KVM_REG_PPC_FPSCR;
>>> + reg.addr = (uintptr_t) &env->fpscr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>> + if (ret < 0)
>>> + {
>>> + fprintf(stderr, "Unable to set FPSCR to KVM: %s", strerror(errno));
>>> + }
>>> +}
>>
>> This is all unnecessary. All you need to do is store to env->fpscr and the
>> value will be synced back with kvm_put_fp.
>>
>> I'll note that some of the trouble you may be having with extracting
>> helper_store_fpscr to a ppc_store_fpscr function is due to an existing bug in
>> the tcg code:
>>
>> Storing to fpscr should *never* raise an exception -- see MTFSF, MTFSB0,
>> MTFSB1. Thus the mucking about with cs->exception_index and env->error_code
>> is incorrect.
>>
>> In addition, the masking is being done weirdly and could use a complete
>> overhaul.
>>
>> This could all be rewritten as
>>
>> -- %< -- cpu.h
>>
>> /* Invalid operation exception summary */
>> - #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
>> + #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
>>
>> -- %< -- cpu.c
>>
>> // move fpscr_set_rounding_mode here
>>
>> void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
>> {
>> /* Recompute exception summary state. */
>> val &= ~(FP_VX | FP_FEX);
>> if (val & FPSCR_IX) {
>> val |= FP_VX;
>> }
>> if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
>> val |= FP_FEX;
>> }
>> env->fpscr = val;
>> if (tcg_enabled()) {
>> fpscr_set_rounding_mode(env);
>> }
>> }
>>
>> -- %< -- fpu_helper.c
>>
>> void helper_store_fpscr(CPUPPCState *env, target_ulong val,
>> uint32_t nibbles)
>> {
>> target_ulong mask = 0;
>>
>> /* TODO: Push this expansion back to translation time. */
>> for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
>> if (nibbles & (1 << i)) {
>> mask |= (target_ulong)0xf << (4 * i);
>> }
>> }
>>
>> val = (val & mask) | (env->fpscr & ~mask);
>> ppc_store_fpscr(env, val);
>> }
> That expansion can't be moved to translation time, because gdbstub would also
> need that code in a variety of functions, so better to keep it in that central
> location,
>>
>> void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
>> {
>> uint32_t mask = 1u << bit;
>> if (env->fpscr & mask) {
>> ppc_store_fpscr(env, env->fpscr & ~mask);
>> }
>> }
>>
>> void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
>> {
>> uint32_t mask = 1u << bit;
>> if (!(env->fpscr & mask)) {
>> ppc_store_fpscr(env, env->fpscr | mask);
>> }
>> }
>>
>> There are a couple of other uses of fpscr_set_rounding_mode, where the
>> softfloat value is changed temporarily (do_fri, VSX_ROUND). These should
>> simply save the previous softfloat value (using get_float_rounding_mode)
>> around the operation instead of re-computing from fpscr.
>>
>> Which leaves us with exactly one use of fpscr_set_rounding_mode, which can
>> then be moved to cpu.c next to ppc_store_fpscr.
>>
>>
>> r~
>
> I was implementing this solution, but ran into a problem: We needed store_fpscr
> for gdbstub.c, that's the original reason we made that new function to begin
> with. This solution, although it improves the handling of fpscr, doesn't fix
> the original problem.
Why not? Did you miss the cpu.c cut at the very top?
> What I think we can do is put the logic that is in helper_store_fpscr into
> store_fpscr, move it to cpu.c, and have the helper call the non-helper
> function. That way we conserve helper_* as TCG-specific and have the overhaul.
That is exactly what I have written above.
r~
On 13/05/2021 19:45, Richard Henderson wrote:
> On 5/13/21 11:36 AM, Bruno Piazera Larsen wrote:
>>
>> On 12/05/2021 15:20, Richard Henderson wrote:
>>> On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
>>>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>>>> index 104a308abb..a8a720eb48 100644
>>>> --- a/target/ppc/kvm.c
>>>> +++ b/target/ppc/kvm.c
>>>> @@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
>>>> {
>>>> return true;
>>>> }
>>>> +
>>>> +void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t
>>>> mask)
>>>> +{
>>>> + CPUState *cs = env_cpu(env);
>>>> + struct kvm_one_reg reg;
>>>> + int ret;
>>>> + reg.id = KVM_REG_PPC_FPSCR;
>>>> + reg.addr = (uintptr_t) &env->fpscr;
>>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>>> + if (ret < 0)
>>>> + {
>>>> + fprintf(stderr, "Unable to set FPSCR to KVM: %s",
>>>> strerror(errno));
>>>> + }
>>>> +}
>>>
>>> This is all unnecessary. All you need to do is store to env->fpscr
>>> and the value will be synced back with kvm_put_fp.
>>>
>>> I'll note that some of the trouble you may be having with extracting
>>> helper_store_fpscr to a ppc_store_fpscr function is due to an
>>> existing bug in the tcg code:
>>>
>>> Storing to fpscr should *never* raise an exception -- see MTFSF,
>>> MTFSB0, MTFSB1. Thus the mucking about with cs->exception_index and
>>> env->error_code is incorrect.
>>>
>>> In addition, the masking is being done weirdly and could use a
>>> complete overhaul.
>>>
>>> This could all be rewritten as
>>>
>>> -- %< -- cpu.h
>>>
>>> /* Invalid operation exception summary */
>>> - #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
>>> + #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
>>>
>>> -- %< -- cpu.c
>>>
>>> // move fpscr_set_rounding_mode here
>>>
>>> void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
>>> {
>>> /* Recompute exception summary state. */
>>> val &= ~(FP_VX | FP_FEX);
>>> if (val & FPSCR_IX) {
>>> val |= FP_VX;
>>> }
>>> if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
>>> val |= FP_FEX;
>>> }
>>> env->fpscr = val;
>>> if (tcg_enabled()) {
>>> fpscr_set_rounding_mode(env);
>>> }
>>> }
>>>
>>> -- %< -- fpu_helper.c
>>>
>>> void helper_store_fpscr(CPUPPCState *env, target_ulong val,
>>> uint32_t nibbles)
>>> {
>>> target_ulong mask = 0;
>>>
>>> /* TODO: Push this expansion back to translation time. */
>>> for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
>>> if (nibbles & (1 << i)) {
>>> mask |= (target_ulong)0xf << (4 * i);
>>> }
>>> }
>>>
>>> val = (val & mask) | (env->fpscr & ~mask);
>>> ppc_store_fpscr(env, val);
>>> }
>> That expansion can't be moved to translation time, because gdbstub
>> would also need that code in a variety of functions, so better to
>> keep it in that central location,
>>>
>>> void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
>>> {
>>> uint32_t mask = 1u << bit;
>>> if (env->fpscr & mask) {
>>> ppc_store_fpscr(env, env->fpscr & ~mask);
>>> }
>>> }
>>>
>>> void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
>>> {
>>> uint32_t mask = 1u << bit;
>>> if (!(env->fpscr & mask)) {
>>> ppc_store_fpscr(env, env->fpscr | mask);
>>> }
>>> }
>>>
>>> There are a couple of other uses of fpscr_set_rounding_mode, where
>>> the softfloat value is changed temporarily (do_fri, VSX_ROUND).
>>> These should simply save the previous softfloat value (using
>>> get_float_rounding_mode) around the operation instead of
>>> re-computing from fpscr.
>>>
>>> Which leaves us with exactly one use of fpscr_set_rounding_mode,
>>> which can then be moved to cpu.c next to ppc_store_fpscr.
>>>
>>>
>>> r~
>>
>> I was implementing this solution, but ran into a problem: We needed
>> store_fpscr for gdbstub.c, that's the original reason we made that
>> new function to begin with. This solution, although it improves the
>> handling of fpscr, doesn't fix the original problem.
>
> Why not? Did you miss the cpu.c cut at the very top?
So the plan was to have gdbstub call ppc_store_fpscr? I assumed it
wasn't since there is one less parameter in the new function. Now that I
took another look, gdbstub has the mask as 0xffffffff, so it's easier
than I thought.
>
>> What I think we can do is put the logic that is in helper_store_fpscr
>> into store_fpscr, move it to cpu.c, and have the helper call the
>> non-helper function. That way we conserve helper_* as TCG-specific
>> and have the overhaul.
>
> That is exactly what I have written above.
Not exactly, because the expansion of nibbles into mask is still in
helper_store_fpscr, and that's what I meant.
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
© 2016 - 2026 Red Hat, Inc.