| Message ID | 20220509124836.27819-3-victor.colombo@eldorado.org.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: Fix FPSCR.FI bit | expand |
On 5/9/22 07:48, 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 passing an argument to float_overflow_excp() indicating > if the FI should be set. > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > --- > target/ppc/fpu_helper.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index 773c80e12d..ee1259ede1 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -329,7 +329,7 @@ static inline void float_zero_divide_excp(CPUPPCState *env, uintptr_t raddr) > } > } > > -static inline void float_overflow_excp(CPUPPCState *env) > +static inline void float_overflow_excp(CPUPPCState *env, bool set_fi) > { > CPUState *cs = env_cpu(env); > > @@ -345,7 +345,9 @@ static inline void float_overflow_excp(CPUPPCState *env) > env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_OX; > } else { > env->fpscr |= FP_XX; > - env->fpscr |= FP_FI; > + if (set_fi) { > + env->fpscr |= FP_FI; > + } Again, I believe setting FP_FI here is wrong, it should only be set later in do_float_check_status. Indeed, setting XX here probably isn't best... .. > @@ -471,7 +473,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); > + float_overflow_excp(env, change_fi); I think the ideal solution would be to return an update to status from float_overflow_excp so that all of the inexact handling happens below. Since inexact is the last bit to be processed, this could be as simple as if (status & overflow) { status = float_overflow_excp(env); } else if (status & underflow) { ... } if (status & inexact) { ... returning OE ? 0 : float_flag_inexact, without trying to merge inexact into the full set of status flags. r~
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index 773c80e12d..ee1259ede1 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -329,7 +329,7 @@ static inline void float_zero_divide_excp(CPUPPCState *env, uintptr_t raddr) } } -static inline void float_overflow_excp(CPUPPCState *env) +static inline void float_overflow_excp(CPUPPCState *env, bool set_fi) { CPUState *cs = env_cpu(env); @@ -345,7 +345,9 @@ static inline void float_overflow_excp(CPUPPCState *env) env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_OX; } else { env->fpscr |= FP_XX; - env->fpscr |= FP_FI; + if (set_fi) { + env->fpscr |= FP_FI; + } } } @@ -471,7 +473,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); + float_overflow_excp(env, change_fi); } 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 passing an argument to float_overflow_excp() indicating if the FI should be set. Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> --- target/ppc/fpu_helper.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)