| Message ID | 20220510204610.100867-3-victor.colombo@eldorado.org.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: Fix FPSCR.FI bit | expand |
On 5/10/22 13:46, Víctor Colombo wrote: > This patch fixes another not-so-clear situation in Power ISA > regarding the inexact bits in FPSCR. The ISA states that: > > """ > When Overflow Exception is disabled (OE=0) and an > Overflow Exception occurs, the following actions are > taken: > ... > 2. Inexact Exception is set > XX <- 1 > ... > FI is set to 1 > ... > """ > > However, when tested on a Power 9 hardware, some instructions that > trigger an OX don't set the FI bit: > > xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> CLEARED > xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> CLEARED > (just a few examples. Other instructions are also affected) > > The root cause for this seems to be that only instructions that list > the bit FI in the "Special Registers Altered" should modify it. > > QEMU is, today, not working like the hardware: > > xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> SET > xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> SET > > (all tests assume FI is cleared beforehand) > > Fix this by making float_overflow_excp() return float_flag_inexact > if it should update the inexact flags. > > Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br> > > --- > > v2: > - remove the setting of FI from float_overflow_excp, making > do_float_check_status() the only responsible for it. > - make float_overflow_excp() return float_flag_inexact if it should > update the inexact flags. > --- > target/ppc/fpu_helper.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Tue, 2022-05-10 at 17:46 -0300, Víctor Colombo wrote: > This patch fixes another not-so-clear situation in Power ISA > regarding the inexact bits in FPSCR. The ISA states that: > > """ > When Overflow Exception is disabled (OE=0) and an > Overflow Exception occurs, the following actions are > taken: > ... > 2. Inexact Exception is set > XX <- 1 > ... > FI is set to 1 > ... > """ > > However, when tested on a Power 9 hardware, some instructions that > trigger an OX don't set the FI bit: > > xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> CLEARED > xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> CLEARED > (just a few examples. Other instructions are also affected) I agree that the ISA could be clearer. These instructions are actually listed in '7.4.3 Floating-Point Overflow Exception' as instructions that don't modify FR, FI and FPRF. It would be ideal if the ISA mentioned that there were exceptions in the part that you quoted! This patch makes sense to me. Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com> > > The root cause for this seems to be that only instructions that list > the bit FI in the "Special Registers Altered" should modify it. > > QEMU is, today, not working like the hardware: > > xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> SET > xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> SET > > (all tests assume FI is cleared beforehand) > > Fix this by making float_overflow_excp() return float_flag_inexact > if it should update the inexact flags. > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > > --- > > v2: > - remove the setting of FI from float_overflow_excp, making > do_float_check_status() the only responsible for it. > - make float_overflow_excp() return float_flag_inexact if it should > update the inexact flags. > --- > target/ppc/fpu_helper.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index f1ea4aa10e..88f9e756a5 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -329,24 +329,25 @@ static inline void > float_zero_divide_excp(CPUPPCState *env, uintptr_t raddr) > } > } > > -static inline void float_overflow_excp(CPUPPCState *env) > +static inline int float_overflow_excp(CPUPPCState *env) > { > CPUState *cs = env_cpu(env); > > env->fpscr |= FP_OX; > /* Update the floating-point exception summary */ > env->fpscr |= FP_FX; > - if (env->fpscr & FP_OE) { > + > + bool overflow_enabled = !!(env->fpscr & FP_OE); > + if (overflow_enabled) { > /* XXX: should adjust the result */ > /* Update the floating-point enabled exception summary */ > env->fpscr |= FP_FEX; > /* We must update the target FPR before raising the > exception */ > cs->exception_index = POWERPC_EXCP_PROGRAM; > env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_OX; > - } else { > - env->fpscr |= FP_XX; > - env->fpscr |= FP_FI; > } > + > + return overflow_enabled ? 0 : float_flag_inexact; > } > > static inline void float_underflow_excp(CPUPPCState *env) > @@ -468,7 +469,7 @@ static void do_float_check_status(CPUPPCState > *env, bool change_fi, > int status = get_float_exception_flags(&env->fp_status); > > if (status & float_flag_overflow) { > - float_overflow_excp(env); > + status |= float_overflow_excp(env); > } else if (status & float_flag_underflow) { > float_underflow_excp(env); > }
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index f1ea4aa10e..88f9e756a5 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -329,24 +329,25 @@ static inline void float_zero_divide_excp(CPUPPCState *env, uintptr_t raddr) } } -static inline void float_overflow_excp(CPUPPCState *env) +static inline int float_overflow_excp(CPUPPCState *env) { CPUState *cs = env_cpu(env); env->fpscr |= FP_OX; /* Update the floating-point exception summary */ env->fpscr |= FP_FX; - if (env->fpscr & FP_OE) { + + bool overflow_enabled = !!(env->fpscr & FP_OE); + if (overflow_enabled) { /* XXX: should adjust the result */ /* Update the floating-point enabled exception summary */ env->fpscr |= FP_FEX; /* We must update the target FPR before raising the exception */ cs->exception_index = POWERPC_EXCP_PROGRAM; env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_OX; - } else { - env->fpscr |= FP_XX; - env->fpscr |= FP_FI; } + + return overflow_enabled ? 0 : float_flag_inexact; } static inline void float_underflow_excp(CPUPPCState *env) @@ -468,7 +469,7 @@ static void do_float_check_status(CPUPPCState *env, bool change_fi, int status = get_float_exception_flags(&env->fp_status); if (status & float_flag_overflow) { - float_overflow_excp(env); + status |= float_overflow_excp(env); } else if (status & float_flag_underflow) { float_underflow_excp(env); }
This patch fixes another not-so-clear situation in Power ISA regarding the inexact bits in FPSCR. The ISA states that: """ When Overflow Exception is disabled (OE=0) and an Overflow Exception occurs, the following actions are taken: ... 2. Inexact Exception is set XX <- 1 ... FI is set to 1 ... """ However, when tested on a Power 9 hardware, some instructions that trigger an OX don't set the FI bit: xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> CLEARED xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> CLEARED (just a few examples. Other instructions are also affected) The root cause for this seems to be that only instructions that list the bit FI in the "Special Registers Altered" should modify it. QEMU is, today, not working like the hardware: xvcvdpsp(0x4050533fcdb7b95ff8d561c40bf90996) = FI: CLEARED -> SET xvnmsubmsp(0xf3c0c1fc8f3230, 0xbeaab9c5) = FI: CLEARED -> SET (all tests assume FI is cleared beforehand) Fix this by making float_overflow_excp() return float_flag_inexact if it should update the inexact flags. Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> --- v2: - remove the setting of FI from float_overflow_excp, making do_float_check_status() the only responsible for it. - make float_overflow_excp() return float_flag_inexact if it should update the inexact flags. --- target/ppc/fpu_helper.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)