* [PATCH v2 0/3] target/ppc: Fix FPSCR.FI bit
@ 2022-05-10 20:46 Víctor Colombo
2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-05-10 20:46 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo
Hello everyone,
The FI bit in FPSCR is said to be a non-sticky bit on Power ISA.
One could think this means that, if an instruction is said to modify
the FPSCR register, the bit FI should be cleared. This is what QEMU
does today.
This is not, however, what the real hardware appears to do. It looks
like QEMU's interpretation of Power ISA was not correct for the
implementation of this bit.
This patch set fixes inconsistencies found in QEMU's handling of the
FPSCR.FI bit.
I found this while investigating how to enable Hardfpu for Power
guests. This change in the understanding on how the Power arch
handles the inexact bit makes it trivial to enable hardfpu for
affected instructions (mostly vsx-vector), but actually seems to
create even more complexity for the changes that will be required
to enable hardfpu for all float instructions. I'll instigate this
discussion more in the next few weeks.
Thanks!
v2:
- move the FI change from float_inexact_excp to do_float_check_status
- 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.
- Add patch 3, moving the renaming of sfprf to sfifprf to it
(previously on patch 1)
Víctor Colombo (3):
target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't
target/ppc: Fix FPSCR.FI changing in float_overflow_excp()
target/ppc: Rename sfprf to sfifprf where it's also used as set fi
flag
target/ppc/cpu.h | 2 +
target/ppc/fpu_helper.c | 223 +++++++++++++++++++++-------------------
2 files changed, 117 insertions(+), 108 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't 2022-05-10 20:46 [PATCH v2 0/3] target/ppc: Fix FPSCR.FI bit Víctor Colombo @ 2022-05-10 20:46 ` Víctor Colombo 2022-05-10 23:56 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 2022-05-10 20:46 ` [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag Víctor Colombo 2 siblings, 2 replies; 11+ messages in thread From: Víctor Colombo @ 2022-05-10 20:46 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo The FI bit in FPSCR is said to be a non-sticky bit on Power ISA. One could think this means that, if an instruction is said to modify the FPSCR register, the bit FI should be cleared. This is what QEMU does today. However, the following inconsistency was found when comparing results from the hardware (tested on both a Power 9 processor and in Power 10 Mambo): (FI bit is set before the execution of the instruction) Hardware: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> SET QEMU: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> CLEARED This is happening to multiple instructions in the vsx implementations. As the FI bit is non-sticky, one could think that xscmpeqdp, a instruction the ISA states doesn't change FI bit, should result in a cleared FI. However, this is not happening on hardware. An investigation resulted in the following conclusion: If the ISA does not list the FI bit as altered for a particular instruction, then it should be kept as it was before the instruction. QEMU is not following this behavior. Affected instructions include: - xv* (all vsx-vector instructions); - xscmp*, xsmax*, xsmin*; - xstdivdp and similars; (to identify the affected instructions, just search in the ISA for the instructions that does not list FI in "Special Registers Altered") Most instructions use the function do_float_check_status() to commit changes in the inexact flag. So the fix is to add a parameter to it that will control if the bit FI should be changed or not. All users of do_float_check_status() are then modified to provide this argument, controlling if that specific instruction changes bit FI or not. Some macro helpers are responsible for both instructions that change and instructions that aren't suposed to change FI. This seems to always overlap with the sfprf flag. So, reuse this flag for this purpose when applicable. Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> --- v2: - move the FI change from float_inexact_excp to do_float_check_status - sfprf will be renamed to sfifprf in another patch, as suggested by Richard --- target/ppc/cpu.h | 2 + target/ppc/fpu_helper.c | 122 +++++++++++++++++++++------------------- 2 files changed, 66 insertions(+), 58 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 48596cfb25..901ded79e9 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -735,6 +735,8 @@ enum { (1 << FPSCR_VXSOFT) | (1 << FPSCR_VXSQRT) | \ (1 << FPSCR_VXCVI)) +FIELD(FPSCR, FI, FPSCR_FI, 1) + #define FP_DRN2 (1ull << FPSCR_DRN2) #define FP_DRN1 (1ull << FPSCR_DRN1) #define FP_DRN0 (1ull << FPSCR_DRN0) diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index f6c8318a71..f1ea4aa10e 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -370,7 +370,6 @@ static inline void float_inexact_excp(CPUPPCState *env) { CPUState *cs = env_cpu(env); - env->fpscr |= FP_FI; env->fpscr |= FP_XX; /* Update the floating-point exception summary */ env->fpscr |= FP_FX; @@ -462,7 +461,8 @@ void helper_fpscr_check_status(CPUPPCState *env) } } -static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) +static void do_float_check_status(CPUPPCState *env, bool change_fi, + uintptr_t raddr) { CPUState *cs = env_cpu(env); int status = get_float_exception_flags(&env->fp_status); @@ -474,8 +474,10 @@ static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) } if (status & float_flag_inexact) { float_inexact_excp(env); - } else { - env->fpscr &= ~FP_FI; /* clear the FPSCR[FI] bit */ + } + if (change_fi) { + env->fpscr = FIELD_DP64(env->fpscr, FPSCR, FI, + !!(status & float_flag_inexact)); } if (cs->exception_index == POWERPC_EXCP_PROGRAM && @@ -490,7 +492,7 @@ static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) void helper_float_check_status(CPUPPCState *env) { - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } void helper_reset_fpstatus(CPUPPCState *env) @@ -684,7 +686,7 @@ uint64_t helper_##op(CPUPPCState *env, uint64_t arg) \ } else { \ farg.d = cvtr(arg, &env->fp_status); \ } \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ return farg.ll; \ } @@ -710,7 +712,7 @@ static uint64_t do_fri(CPUPPCState *env, uint64_t arg, /* fri* does not set FPSCR[XX] */ set_float_exception_flags(flags & ~float_flag_inexact, &env->fp_status); - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); return arg; } @@ -1721,7 +1723,7 @@ void helper_##name(CPUPPCState *env, ppc_vsr_t *xt, \ } \ } \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_ADD_SUB(xsadddp, add, 1, float64, VsrD(0), 1, 0) @@ -1757,7 +1759,7 @@ void helper_xsaddqp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } /* @@ -1798,7 +1800,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_MUL(xsmuldp, 1, float64, VsrD(0), 1, 0) @@ -1828,7 +1830,7 @@ void helper_xsmulqp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } /* @@ -1872,7 +1874,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_DIV(xsdivdp, 1, float64, VsrD(0), 1, 0) @@ -1905,7 +1907,7 @@ void helper_xsdivqp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } /* @@ -1940,7 +1942,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_RE(xsredp, 1, float64, VsrD(0), 1, 0) @@ -1985,7 +1987,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_SQRT(xssqrtdp, 1, float64, VsrD(0), 1, 0) @@ -2029,7 +2031,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_RSQRTE(xsrsqrtedp, 1, float64, VsrD(0), 1, 0) @@ -2182,7 +2184,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ } \ } \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_MADD(XSMADDDP, 1, float64, VsrD(0), MADD_FLGS, 1) @@ -2234,7 +2236,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *s1, ppc_vsr_t *s2,\ \ helper_compute_fprf_float128(env, t.f128); \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_MADDQ(XSMADDQP, MADD_FLGS, 0) @@ -2283,7 +2285,7 @@ VSX_MADDQ(XSNMSUBQPO, NMSUB_FLGS, 0) \ memset(xt, 0, sizeof(*xt)); \ memset(&xt->fld, -r, sizeof(xt->fld)); \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, false, GETPC()); \ } VSX_SCALAR_CMP(XSCMPEQDP, float64, eq, VsrD(0), 0) @@ -2319,7 +2321,7 @@ void helper_xscmpexpdp(CPUPPCState *env, uint32_t opcode, env->fpscr |= cc << FPSCR_FPCC; env->crf[BF(opcode)] = cc; - do_float_check_status(env, GETPC()); + do_float_check_status(env, false, GETPC()); } void helper_xscmpexpqp(CPUPPCState *env, uint32_t opcode, @@ -2348,7 +2350,7 @@ void helper_xscmpexpqp(CPUPPCState *env, uint32_t opcode, env->fpscr |= cc << FPSCR_FPCC; env->crf[BF(opcode)] = cc; - do_float_check_status(env, GETPC()); + do_float_check_status(env, false, GETPC()); } static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb, @@ -2401,7 +2403,7 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb, float_invalid_op_vxvc(env, 0, GETPC()); } - do_float_check_status(env, GETPC()); + do_float_check_status(env, false, GETPC()); } void helper_xscmpodp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa, @@ -2466,7 +2468,7 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa, float_invalid_op_vxvc(env, 0, GETPC()); } - do_float_check_status(env, GETPC()); + do_float_check_status(env, false, GETPC()); } void helper_xscmpoqp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa, @@ -2505,7 +2507,7 @@ void helper_##name(CPUPPCState *env, ppc_vsr_t *xt, \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, false, GETPC()); \ } VSX_MAX_MIN(xsmaxdp, maxnum, 1, float64, VsrD(0)) @@ -2688,7 +2690,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_CVT_FP_TO_FP(xscvspdp, 1, float32, float64, VsrW(0), VsrD(0), 1) @@ -2714,7 +2716,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_CVT_FP_TO_FP2(xvcvdpsp, 2, float64, float32, 0) @@ -2750,7 +2752,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_CVT_FP_TO_FP_VECTOR(xscvdpqp, 1, float64, float128, VsrD(0), f128, 1) @@ -2785,7 +2787,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_CVT_FP_TO_FP_HP(xscvdphp, 1, float64, float16, VsrD(0), VsrH(3), 1) @@ -2810,7 +2812,7 @@ void helper_XVCVSPBF16(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) } *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, false, GETPC()); } void helper_XSCVQPDP(CPUPPCState *env, uint32_t ro, ppc_vsr_t *xt, @@ -2833,7 +2835,7 @@ void helper_XSCVQPDP(CPUPPCState *env, uint32_t ro, ppc_vsr_t *xt, helper_compute_fprf_float64(env, t.VsrD(0)); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb) @@ -2889,9 +2891,10 @@ uint64_t helper_xscvspdpn(CPUPPCState *env, uint64_t xb) * ttp - target type (int32, uint32, int64 or uint64) * sfld - source vsr_t field * tfld - target vsr_t field + * sfi - set FI * rnan - resulting NaN */ -#define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, rnan) \ +#define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, sfi, rnan) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ int all_flags = env->fp_status.float_exception_flags, flags; \ @@ -2910,20 +2913,23 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ \ *xt = t; \ env->fp_status.float_exception_flags = all_flags; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfi, GETPC()); \ } -VSX_CVT_FP_TO_INT(xscvdpsxds, 1, float64, int64, VsrD(0), VsrD(0), \ +VSX_CVT_FP_TO_INT(xscvdpsxds, 1, float64, int64, VsrD(0), VsrD(0), true, \ 0x8000000000000000ULL) -VSX_CVT_FP_TO_INT(xscvdpuxds, 1, float64, uint64, VsrD(0), VsrD(0), 0ULL) -VSX_CVT_FP_TO_INT(xvcvdpsxds, 2, float64, int64, VsrD(i), VsrD(i), \ +VSX_CVT_FP_TO_INT(xscvdpuxds, 1, float64, uint64, VsrD(0), VsrD(0), true, 0ULL) +VSX_CVT_FP_TO_INT(xvcvdpsxds, 2, float64, int64, VsrD(i), VsrD(i), false, \ 0x8000000000000000ULL) -VSX_CVT_FP_TO_INT(xvcvdpuxds, 2, float64, uint64, VsrD(i), VsrD(i), 0ULL) -VSX_CVT_FP_TO_INT(xvcvspsxds, 2, float32, int64, VsrW(2 * i), VsrD(i), \ +VSX_CVT_FP_TO_INT(xvcvdpuxds, 2, float64, uint64, VsrD(i), VsrD(i), false, \ + 0ULL) +VSX_CVT_FP_TO_INT(xvcvspsxds, 2, float32, int64, VsrW(2 * i), VsrD(i), false, \ 0x8000000000000000ULL) -VSX_CVT_FP_TO_INT(xvcvspsxws, 4, float32, int32, VsrW(i), VsrW(i), 0x80000000U) -VSX_CVT_FP_TO_INT(xvcvspuxds, 2, float32, uint64, VsrW(2 * i), VsrD(i), 0ULL) -VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, VsrW(i), VsrW(i), 0U) +VSX_CVT_FP_TO_INT(xvcvspsxws, 4, float32, int32, VsrW(i), VsrW(i), false, \ + 0x80000000ULL) +VSX_CVT_FP_TO_INT(xvcvspuxds, 2, float32, uint64, VsrW(2 * i), VsrD(i), \ + false, 0ULL) +VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, VsrW(i), VsrW(i), false, 0U) #define VSX_CVT_FP_TO_INT128(op, tp, rnan) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ @@ -2940,7 +2946,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_CVT_FP_TO_INT128(XSCVQPUQZ, uint128, 0) @@ -2955,7 +2961,7 @@ VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000000000000000ULL); * words 0 and 1 (and words 2 and 3) of the result register, as * is required by this version of the architecture. */ -#define VSX_CVT_FP_TO_INT2(op, nels, stp, ttp, rnan) \ +#define VSX_CVT_FP_TO_INT2(op, nels, stp, ttp, sfi, rnan) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ int all_flags = env->fp_status.float_exception_flags, flags; \ @@ -2977,13 +2983,13 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ \ *xt = t; \ env->fp_status.float_exception_flags = all_flags; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfi, GETPC()); \ } -VSX_CVT_FP_TO_INT2(xscvdpsxws, 1, float64, int32, 0x80000000U) -VSX_CVT_FP_TO_INT2(xscvdpuxws, 1, float64, uint32, 0U) -VSX_CVT_FP_TO_INT2(xvcvdpsxws, 2, float64, int32, 0x80000000U) -VSX_CVT_FP_TO_INT2(xvcvdpuxws, 2, float64, uint32, 0U) +VSX_CVT_FP_TO_INT2(xscvdpsxws, 1, float64, int32, true, 0x80000000U) +VSX_CVT_FP_TO_INT2(xscvdpuxws, 1, float64, uint32, true, 0U) +VSX_CVT_FP_TO_INT2(xvcvdpsxws, 2, float64, int32, false, 0x80000000U) +VSX_CVT_FP_TO_INT2(xvcvdpuxws, 2, float64, uint32, false, 0U) /* * VSX_CVT_FP_TO_INT_VECTOR - VSX floating point to integer conversion @@ -3008,7 +3014,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_CVT_FP_TO_INT_VECTOR(xscvqpsdz, float128, int64, f128, VsrD(0), \ @@ -3047,7 +3053,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, VsrD(0), VsrD(0), 1, 0) @@ -3073,7 +3079,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, false, GETPC()); \ } VSX_CVT_INT_TO_FP2(xvcvsxdsp, int64, float32) @@ -3085,7 +3091,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb)\ helper_reset_fpstatus(env); \ xt->f128 = tp##_to_float128(xb->s128, &env->fp_status); \ helper_compute_fprf_float128(env, xt->f128); \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_CVT_INT128_TO_FP(XSCVUQQP, uint128); @@ -3109,7 +3115,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \ helper_compute_fprf_##ttp(env, t.tfld); \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, true, GETPC()); \ } VSX_CVT_INT_TO_FP_VECTOR(xscvsdqp, int64, float128, VsrD(0), f128) @@ -3167,7 +3173,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, GETPC()); \ + do_float_check_status(env, sfprf, GETPC()); \ } VSX_ROUND(xsrdpi, 1, float64, VsrD(0), float_round_ties_away, 1) @@ -3195,7 +3201,7 @@ uint64_t helper_xsrsp(CPUPPCState *env, uint64_t xb) uint64_t xt = do_frsp(env, xb, GETPC()); helper_compute_fprf_float64(env, xt); - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); return xt; } @@ -3355,7 +3361,7 @@ void helper_xsrqpi(CPUPPCState *env, uint32_t opcode, } helper_compute_fprf_float128(env, t.f128); - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); *xt = t; } @@ -3408,7 +3414,7 @@ void helper_xsrqpxp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode, @@ -3434,7 +3440,7 @@ void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } void helper_xssubqp(CPUPPCState *env, uint32_t opcode, @@ -3460,5 +3466,5 @@ void helper_xssubqp(CPUPPCState *env, uint32_t opcode, helper_compute_fprf_float128(env, t.f128); *xt = t; - do_float_check_status(env, GETPC()); + do_float_check_status(env, true, GETPC()); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't 2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo @ 2022-05-10 23:56 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 1 sibling, 0 replies; 11+ messages in thread From: Richard Henderson @ 2022-05-10 23:56 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc; +Cc: clg, danielhb413, david, groug On 5/10/22 13:46, Víctor Colombo wrote: > The FI bit in FPSCR is said to be a non-sticky bit on Power ISA. > One could think this means that, if an instruction is said to modify > the FPSCR register, the bit FI should be cleared. This is what QEMU > does today. > > However, the following inconsistency was found when comparing results > from the hardware (tested on both a Power 9 processor and in > Power 10 Mambo): > > (FI bit is set before the execution of the instruction) > Hardware: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> SET > QEMU: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> CLEARED > > This is happening to multiple instructions in the vsx > implementations. As the FI bit is non-sticky, one could think that > xscmpeqdp, a instruction the ISA states doesn't change FI bit, should > result in a cleared FI. However, this is not happening on hardware. > > An investigation resulted in the following conclusion: > If the ISA does not list the FI bit as altered for a particular > instruction, then it should be kept as it was before the instruction. > > QEMU is not following this behavior. Affected instructions include: > - xv* (all vsx-vector instructions); > - xscmp*, xsmax*, xsmin*; > - xstdivdp and similars; > (to identify the affected instructions, just search in the ISA for > the instructions that does not list FI in "Special Registers Altered") > > Most instructions use the function do_float_check_status() to commit > changes in the inexact flag. So the fix is to add a parameter to it > that will control if the bit FI should be changed or not. > All users of do_float_check_status() are then modified to provide this > argument, controlling if that specific instruction changes bit FI or > not. > Some macro helpers are responsible for both instructions that change > and instructions that aren't suposed to change FI. This seems to always > overlap with the sfprf flag. So, reuse this flag for this purpose when > applicable. > > Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br> > > --- > > v2: > - move the FI change from float_inexact_excp to do_float_check_status > - sfprf will be renamed to sfifprf in another patch, as suggested by > Richard > --- > target/ppc/cpu.h | 2 + > target/ppc/fpu_helper.c | 122 +++++++++++++++++++++------------------- > 2 files changed, 66 insertions(+), 58 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't 2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo 2022-05-10 23:56 ` Richard Henderson @ 2022-05-11 10:12 ` Rashmica Gupta 2022-05-11 18:29 ` Víctor Colombo 1 sibling, 1 reply; 11+ messages in thread From: Rashmica Gupta @ 2022-05-11 10:12 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson, Nicholas Piggin, Paul Mackerras [-- Attachment #1: Type: text/plain, Size: 7721 bytes --] Hello, cc'ing Paul and Nick for clarification on the behaviour of xsrsp (see below) On Tue, 2022-05-10 at 17:46 -0300, Víctor Colombo wrote: > The FI bit in FPSCR is said to be a non-sticky bit on Power ISA. > One could think this means that, if an instruction is said to modify > the FPSCR register, the bit FI should be cleared. This is what QEMU > does today. > > However, the following inconsistency was found when comparing results > from the hardware (tested on both a Power 9 processor and in > Power 10 Mambo): > > (FI bit is set before the execution of the instruction) > Hardware: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> SET > QEMU: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> CLEARED > > This is happening to multiple instructions in the vsx > implementations. As the FI bit is non-sticky, one could think that > xscmpeqdp, a instruction the ISA states doesn't change FI bit, should > result in a cleared FI. However, this is not happening on hardware. I would think that if an instruction doesn't change a bit then that instruction wouldn't clear or set that bit, regardless of if that bit is sticky or not. > > An investigation resulted in the following conclusion: > If the ISA does not list the FI bit as altered for a particular > instruction, then it should be kept as it was before the instruction. > > QEMU is not following this behavior. Affected instructions include: > - xv* (all vsx-vector instructions); > - xscmp*, xsmax*, xsmin*; > - xstdivdp and similars; > (to identify the affected instructions, just search in the ISA for > the instructions that does not list FI in "Special Registers > Altered") The ISA does state "Floating-Point Fraction Inexact (FI) This bit is set to 0 or 1 by VSX Scalar Floating-Point Arithmetic, VSX Scalar Integer Conversion, and VSX Scalar Round to Floating-Point Integer class instructions to indicate whether or not the rounded result is inexact or the instruction caused a disabled Overflow exception. See Section 7.3.2.6 on page 524. This bit is not sticky." So it seems that instruction classes like VSX Vector Round and VSX Vector convert don't touch the FI bit. > > Most instructions use the function do_float_check_status() to commit > changes in the inexact flag. So the fix is to add a parameter to it > that will control if the bit FI should be changed or not. > All users of do_float_check_status() are then modified to provide > this > argument, controlling if that specific instruction changes bit FI or > not. > Some macro helpers are responsible for both instructions that change > and instructions that aren't suposed to change FI. This seems to > always > overlap with the sfprf flag. So, reuse this flag for this purpose > when > applicable. > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > > --- > > v2: > - move the FI change from float_inexact_excp to do_float_check_status > - sfprf will be renamed to sfifprf in another patch, as suggested by > Richard > --- > target/ppc/cpu.h | 2 + > target/ppc/fpu_helper.c | 122 +++++++++++++++++++++----------------- > -- > 2 files changed, 66 insertions(+), 58 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 48596cfb25..901ded79e9 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -735,6 +735,8 @@ enum { > (1 << FPSCR_VXSOFT) | (1 << FPSCR_VXSQRT) | \ > (1 << FPSCR_VXCVI)) > > +FIELD(FPSCR, FI, FPSCR_FI, 1) > + > #define FP_DRN2 (1ull << FPSCR_DRN2) > #define FP_DRN1 (1ull << FPSCR_DRN1) > #define FP_DRN0 (1ull << FPSCR_DRN0) > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index f6c8318a71..f1ea4aa10e 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -370,7 +370,6 @@ static inline void float_inexact_excp(CPUPPCState > *env) > { > CPUState *cs = env_cpu(env); > > - env->fpscr |= FP_FI; > env->fpscr |= FP_XX; > /* Update the floating-point exception summary */ > env->fpscr |= FP_FX; > @@ -462,7 +461,8 @@ void helper_fpscr_check_status(CPUPPCState *env) > } > } > > -static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) > +static void do_float_check_status(CPUPPCState *env, bool change_fi, > + uintptr_t raddr) > { > CPUState *cs = env_cpu(env); > int status = get_float_exception_flags(&env->fp_status); > @@ -474,8 +474,10 @@ static void do_float_check_status(CPUPPCState > *env, uintptr_t raddr) > } > if (status & float_flag_inexact) { > float_inexact_excp(env); > - } else { > - env->fpscr &= ~FP_FI; /* clear the FPSCR[FI] bit */ > + } > + if (change_fi) { > + env->fpscr = FIELD_DP64(env->fpscr, FPSCR, FI, > + !!(status & float_flag_inexact)); > } According to the ISA not all instructions that affect FI, set FI on an inexact exception (xsrqpi apparently clears FI on an inexact exception -- I have no idea if this actually happens on hardware). > > if (cs->exception_index == POWERPC_EXCP_PROGRAM && > @@ -490,7 +492,7 @@ static void do_float_check_status(CPUPPCState > *env, uintptr_t raddr) > > void helper_float_check_status(CPUPPCState *env) > { > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > } > ... snip ... > @@ -3195,7 +3201,7 @@ uint64_t helper_xsrsp(CPUPPCState *env, > uint64_t xb) > uint64_t xt = do_frsp(env, xb, GETPC()); > > helper_compute_fprf_float64(env, xt); > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > return xt; > } I'm not clear what the behaviour of xsrsp should be. Section 7.4.5 Floating-Point Inexact Exception leads me to think it shouldn't affect FI but the instruction definition indicates the opposite. Maybe Paul or Nick can weigh in on this? > > @@ -3355,7 +3361,7 @@ void helper_xsrqpi(CPUPPCState *env, uint32_t > opcode, > } > > helper_compute_fprf_float128(env, t.f128); > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > *xt = t; > } > > @@ -3408,7 +3414,7 @@ void helper_xsrqpxp(CPUPPCState *env, uint32_t > opcode, > > helper_compute_fprf_float128(env, t.f128); > *xt = t; > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > } > > void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode, > @@ -3434,7 +3440,7 @@ void helper_xssqrtqp(CPUPPCState *env, uint32_t > opcode, > > helper_compute_fprf_float128(env, t.f128); > *xt = t; > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > } > > void helper_xssubqp(CPUPPCState *env, uint32_t opcode, > @@ -3460,5 +3466,5 @@ void helper_xssubqp(CPUPPCState *env, uint32_t > opcode, > > helper_compute_fprf_float128(env, t.f128); > *xt = t; > - do_float_check_status(env, GETPC()); > + do_float_check_status(env, true, GETPC()); > } All the other instruction helpers and definitions look correct to me. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't 2022-05-11 10:12 ` Rashmica Gupta @ 2022-05-11 18:29 ` Víctor Colombo 0 siblings, 0 replies; 11+ messages in thread From: Víctor Colombo @ 2022-05-11 18:29 UTC (permalink / raw) To: Rashmica Gupta, qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson, Nicholas Piggin, Paul Mackerras Hello! Thanks everyone for your kind reviews On 11/05/2022 07:12, Rashmica Gupta wrote: > Hello, > > cc'ing Paul and Nick for clarification on the behaviour of xsrsp (see below) > > > On Tue, 2022-05-10 at 17:46 -0300, Víctor Colombo wrote: >> The FI bit in FPSCR is said to be a non-sticky bit on Power ISA. >> One could think this means that, if an instruction is said to modify >> the FPSCR register, the bit FI should be cleared. This is what QEMU >> does today. >> >> However, the following inconsistency was found when comparing results >> from the hardware (tested on both a Power 9 processor and in >> Power 10 Mambo): >> >> (FI bit is set before the execution of the instruction) >> Hardware: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> SET >> QEMU: xscmpeqdp(0xff..ff, 0xff..ff) = FI: SET -> CLEARED >> >> This is happening to multiple instructions in the vsx >> implementations. As the FI bit is non-sticky, one could think that >> xscmpeqdp, a instruction the ISA states doesn't change FI bit, should >> result in a cleared FI. However, this is not happening on hardware. > I would think that if an instruction doesn't change a bit > then that instruction wouldn't clear or set that bit, regardless of if > that bit is sticky or not. I think I might have over-generalized this commit message. I, as well as other developers of the Power ISA instructions in QEMU, didn't notice this behavior before, so it's at least confusing > >> An investigation resulted in the following conclusion: >> If the ISA does not list the FI bit as altered for a particular >> instruction, then it should be kept as it was before the instruction. >> >> QEMU is not following this behavior. Affected instructions include: >> - xv* (all vsx-vector instructions); >> - xscmp*, xsmax*, xsmin*; >> - xstdivdp and similars; >> (to identify the affected instructions, just search in the ISA for >> the instructions that does not list FI in "Special Registers >> Altered") > The ISA does state > "Floating-Point Fraction Inexact (FI) > This bit is set to 0 or 1 by VSX Scalar > Floating-Point Arithmetic, VSX Scalar Integer > Conversion, and VSX Scalar Round to > Floating-Point Integer class instructions to > indicate whether or not the rounded result is > inexact or the instruction caused a disabled > Overflow exception. See Section 7.3.2.6 on > page 524. This bit is not sticky." > > So it seems that instruction classes like VSX Vector Round and VSX Vector > convert don't touch the FI bit. > >> Most instructions use the function do_float_check_status() to commit >> changes in the inexact flag. So the fix is to add a parameter to it >> that will control if the bit FI should be changed or not. >> All users of do_float_check_status() are then modified to provide >> this >> argument, controlling if that specific instruction changes bit FI or >> not. >> Some macro helpers are responsible for both instructions that change >> and instructions that aren't suposed to change FI. This seems to >> always >> overlap with the sfprf flag. So, reuse this flag for this purpose >> when >> applicable. >> >> Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br> >> >> --- >> >> v2: >> - move the FI change from float_inexact_excp to do_float_check_status >> - sfprf will be renamed to sfifprf in another patch, as suggested by >> Richard >> --- >> target/ppc/cpu.h | 2 + >> target/ppc/fpu_helper.c | 122 +++++++++++++++++++++----------------- >> -- >> 2 files changed, 66 insertions(+), 58 deletions(-) >> >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h >> index 48596cfb25..901ded79e9 100644 >> --- a/target/ppc/cpu.h >> +++ b/target/ppc/cpu.h >> @@ -735,6 +735,8 @@ enum { >> (1 << FPSCR_VXSOFT) | (1 << FPSCR_VXSQRT) | \ >> (1 << FPSCR_VXCVI)) >> >> +FIELD(FPSCR, FI, FPSCR_FI, 1) >> + >> #define FP_DRN2 (1ull << FPSCR_DRN2) >> #define FP_DRN1 (1ull << FPSCR_DRN1) >> #define FP_DRN0 (1ull << FPSCR_DRN0) >> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c >> index f6c8318a71..f1ea4aa10e 100644 >> --- a/target/ppc/fpu_helper.c >> +++ b/target/ppc/fpu_helper.c >> @@ -370,7 +370,6 @@ static inline void float_inexact_excp(CPUPPCState >> *env) >> { >> CPUState *cs = env_cpu(env); >> >> - env->fpscr |= FP_FI; >> env->fpscr |= FP_XX; >> /* Update the floating-point exception summary */ >> env->fpscr |= FP_FX; >> @@ -462,7 +461,8 @@ void helper_fpscr_check_status(CPUPPCState *env) >> } >> } >> >> -static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) >> +static void do_float_check_status(CPUPPCState *env, bool change_fi, >> + uintptr_t raddr) >> { >> CPUState *cs = env_cpu(env); >> int status = get_float_exception_flags(&env->fp_status); >> @@ -474,8 +474,10 @@ static void do_float_check_status(CPUPPCState >> *env, uintptr_t raddr) >> } >> if (status & float_flag_inexact) { >> float_inexact_excp(env); >> - } else { >> - env->fpscr &= ~FP_FI; /* clear the FPSCR[FI] bit */ >> + } >> + if (change_fi) { >> + env->fpscr = FIELD_DP64(env->fpscr, FPSCR, FI, >> + !!(status & float_flag_inexact)); >> } > > According to the ISA not all instructions that affect FI, set FI on an > inexact exception (xsrqpi apparently clears FI on an inexact exception > -- I have no idea if this actually happens on hardware). good point, xsrqpi is handled as it was before already: the inexact flag is being cleared for fp_status in helper_xsrqpi, causing do_float_check_status to clear it for fpscr. I think we should keep it like this. The change I made to do_float_check_status allow for its callers to request if the FI field in FPSCR should be updated. What value should be updated to is then responsibility of these callers, provided as an argument to do_float_check_status, like xsrqpi is doing. > > >> >> if (cs->exception_index == POWERPC_EXCP_PROGRAM && >> @@ -490,7 +492,7 @@ static void do_float_check_status(CPUPPCState >> *env, uintptr_t raddr) >> >> void helper_float_check_status(CPUPPCState *env) >> { >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> } >> > > ... snip ... > >> @@ -3195,7 +3201,7 @@ uint64_t helper_xsrsp(CPUPPCState *env, >> uint64_t xb) >> uint64_t xt = do_frsp(env, xb, GETPC()); >> >> helper_compute_fprf_float64(env, xt); >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> return xt; >> } > I'm not clear what the behaviour of xsrsp should be. > > Section 7.4.5 Floating-Point Inexact Exception leads me to think it > shouldn't affect FI but the instruction definition indicates the > opposite. Maybe Paul or Nick can weigh in on this? > I tested on a Power9 hardware and FI is altered by xsrsp when XE=0. But indeed, the ISA seems a bit contradictory on this one, section 7.4.5 only states about changing XX in this situation. > >> >> @@ -3355,7 +3361,7 @@ void helper_xsrqpi(CPUPPCState *env, uint32_t >> opcode, >> } >> >> helper_compute_fprf_float128(env, t.f128); >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> *xt = t; >> } >> >> @@ -3408,7 +3414,7 @@ void helper_xsrqpxp(CPUPPCState *env, uint32_t >> opcode, >> >> helper_compute_fprf_float128(env, t.f128); >> *xt = t; >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> } >> >> void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode, >> @@ -3434,7 +3440,7 @@ void helper_xssqrtqp(CPUPPCState *env, uint32_t >> opcode, >> >> helper_compute_fprf_float128(env, t.f128); >> *xt = t; >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> } >> >> void helper_xssubqp(CPUPPCState *env, uint32_t opcode, >> @@ -3460,5 +3466,5 @@ void helper_xssubqp(CPUPPCState *env, uint32_t >> opcode, >> >> helper_compute_fprf_float128(env, t.f128); >> *xt = t; >> - do_float_check_status(env, GETPC()); >> + do_float_check_status(env, true, GETPC()); >> } > > All the other instruction helpers and definitions look correct to me. Thank you very much! -- Víctor Cora Colombo Instituto de Pesquisas ELDORADO Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() 2022-05-10 20:46 [PATCH v2 0/3] target/ppc: Fix FPSCR.FI bit Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo @ 2022-05-10 20:46 ` Víctor Colombo 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 2022-05-10 20:46 ` [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag Víctor Colombo 2 siblings, 2 replies; 11+ messages in thread From: Víctor Colombo @ 2022-05-10 20:46 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo 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(-) 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); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() 2022-05-10 20:46 ` [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() Víctor Colombo @ 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 1 sibling, 0 replies; 11+ messages in thread From: Richard Henderson @ 2022-05-10 23:58 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc; +Cc: clg, danielhb413, david, groug 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~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() 2022-05-10 20:46 ` [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() Víctor Colombo 2022-05-10 23:58 ` Richard Henderson @ 2022-05-11 10:12 ` Rashmica Gupta 1 sibling, 0 replies; 11+ messages in thread From: Rashmica Gupta @ 2022-05-11 10:12 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson [-- Attachment #1: Type: text/plain, Size: 3858 bytes --] 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); > } [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag 2022-05-10 20:46 [PATCH v2 0/3] target/ppc: Fix FPSCR.FI bit Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() Víctor Colombo @ 2022-05-10 20:46 ` Víctor Colombo 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 2 siblings, 2 replies; 11+ messages in thread From: Víctor Colombo @ 2022-05-10 20:46 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo The bit FI fix used the sfprf flag as a flag for the set_fi parameter in do_float_check_status where applicable. Now, this patch rename this flag to sfifprf to state this dual usage. Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> --- v2: Add this patch --- target/ppc/fpu_helper.c | 112 ++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index 88f9e756a5..7209150d1a 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -1693,9 +1693,9 @@ uint32_t helper_efdcmpeq(CPUPPCState *env, uint64_t op1, uint64_t op2) * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_ADD_SUB(name, op, nels, tp, fld, sfifprf, r2sp) \ void helper_##name(CPUPPCState *env, ppc_vsr_t *xt, \ ppc_vsr_t *xa, ppc_vsr_t *xb) \ { \ @@ -1712,19 +1712,19 @@ void helper_##name(CPUPPCState *env, ppc_vsr_t *xt, \ \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_addsub(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ \ if (r2sp) { \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_ADD_SUB(xsadddp, add, 1, float64, VsrD(0), 1, 0) @@ -1769,9 +1769,9 @@ void helper_xsaddqp(CPUPPCState *env, uint32_t opcode, * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_MUL(op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_MUL(op, nels, tp, fld, sfifprf, r2sp) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ ppc_vsr_t *xa, ppc_vsr_t *xb) \ { \ @@ -1788,20 +1788,20 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_mul(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ \ if (r2sp) { \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_MUL(xsmuldp, 1, float64, VsrD(0), 1, 0) @@ -1840,9 +1840,9 @@ void helper_xsmulqp(CPUPPCState *env, uint32_t opcode, * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_DIV(op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_DIV(op, nels, tp, fld, sfifprf, r2sp) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ ppc_vsr_t *xa, ppc_vsr_t *xb) \ { \ @@ -1859,7 +1859,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_div(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ if (unlikely(tstat.float_exception_flags & float_flag_divbyzero)) { \ float_zero_divide_excp(env, GETPC()); \ @@ -1869,13 +1869,13 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_DIV(xsdivdp, 1, float64, VsrD(0), 1, 0) @@ -1917,9 +1917,9 @@ void helper_xsdivqp(CPUPPCState *env, uint32_t opcode, * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_RE(op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_RE(op, nels, tp, fld, sfifprf, r2sp) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -1937,13 +1937,13 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_RE(xsredp, 1, float64, VsrD(0), 1, 0) @@ -1957,9 +1957,9 @@ VSX_RE(xvresp, 4, float32, VsrW(i), 0, 0) * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_SQRT(op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_SQRT(op, nels, tp, fld, sfifprf, r2sp) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -1975,20 +1975,20 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_sqrt(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ \ if (r2sp) { \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_SQRT(xssqrtdp, 1, float64, VsrD(0), 1, 0) @@ -2002,9 +2002,9 @@ VSX_SQRT(xvsqrtsp, 4, float32, VsrW(i), 0, 0) * nels - number of elements (1, 2 or 4) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_RSQRTE(op, nels, tp, fld, sfprf, r2sp) \ +#define VSX_RSQRTE(op, nels, tp, fld, sfifprf, r2sp) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -2020,19 +2020,19 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ env->fp_status.float_exception_flags |= tstat.float_exception_flags; \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_sqrt(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ if (r2sp) { \ t.fld = do_frsp(env, t.fld, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_RSQRTE(xsrsqrtedp, 1, float64, VsrD(0), 1, 0) @@ -2158,9 +2158,9 @@ VSX_TSQRT(xvtsqrtsp, 4, float32, VsrW(i), -126, 23) * fld - vsr_t field (VsrD(*) or VsrW(*)) * maddflgs - flags for the float*muladd routine that control the * various forms (madd, msub, nmadd, nmsub) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_MADD(op, nels, tp, fld, maddflgs, sfprf) \ +#define VSX_MADD(op, nels, tp, fld, maddflgs, sfifprf) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ ppc_vsr_t *s1, ppc_vsr_t *s2, ppc_vsr_t *s3) \ { \ @@ -2177,15 +2177,15 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ \ if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ float_invalid_op_madd(env, tstat.float_exception_flags, \ - sfprf, GETPC()); \ + sfifprf, GETPC()); \ } \ \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_MADD(XSMADDDP, 1, float64, VsrD(0), MADD_FLGS, 1) @@ -2670,9 +2670,9 @@ VSX_CMP(xvcmpnesp, 4, float32, VsrW(i), eq, 0, 0) * ttp - target type (float32 or float64) * sfld - source vsr_t field * tfld - target vsr_t field (f32 or f64) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_CVT_FP_TO_FP(op, nels, stp, ttp, sfld, tfld, sfprf) \ +#define VSX_CVT_FP_TO_FP(op, nels, stp, ttp, sfld, tfld, sfifprf) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -2685,19 +2685,19 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ float_invalid_op_vxsnan(env, GETPC()); \ t.tfld = ttp##_snan_to_qnan(t.tfld); \ } \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_##ttp(env, t.tfld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_CVT_FP_TO_FP(xscvspdp, 1, float32, float64, VsrW(0), VsrD(0), 1) VSX_CVT_FP_TO_FP(xvcvspdp, 2, float32, float64, VsrW(2 * i), VsrD(i), 0) -#define VSX_CVT_FP_TO_FP2(op, nels, stp, ttp, sfprf) \ +#define VSX_CVT_FP_TO_FP2(op, nels, stp, ttp, sfifprf) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -2710,14 +2710,14 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ float_invalid_op_vxsnan(env, GETPC()); \ t.VsrW(2 * i) = ttp##_snan_to_qnan(t.VsrW(2 * i)); \ } \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_##ttp(env, t.VsrW(2 * i)); \ } \ t.VsrW(2 * i + 1) = t.VsrW(2 * i); \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_CVT_FP_TO_FP2(xvcvdpsp, 2, float64, float32, 0) @@ -2733,9 +2733,9 @@ VSX_CVT_FP_TO_FP2(xscvdpsp, 1, float64, float32, 1) * tfld - target vsr_t field (f32 or f64) * sfprf - set FPRF */ -#define VSX_CVT_FP_TO_FP_VECTOR(op, nels, stp, ttp, sfld, tfld, sfprf) \ -void helper_##op(CPUPPCState *env, uint32_t opcode, \ - ppc_vsr_t *xt, ppc_vsr_t *xb) \ +#define VSX_CVT_FP_TO_FP_VECTOR(op, nels, stp, ttp, sfld, tfld, sfprf) \ +void helper_##op(CPUPPCState *env, uint32_t opcode, \ + ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = *xt; \ int i; \ @@ -2767,9 +2767,9 @@ VSX_CVT_FP_TO_FP_VECTOR(xscvdpqp, 1, float64, float128, VsrD(0), f128, 1) * ttp - target type * sfld - source vsr_t field * tfld - target vsr_t field - * sfprf - set FPRF + * sfifprf - set FPRF */ -#define VSX_CVT_FP_TO_FP_HP(op, nels, stp, ttp, sfld, tfld, sfprf) \ +#define VSX_CVT_FP_TO_FP_HP(op, nels, stp, ttp, sfld, tfld, sfifprf) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -2782,13 +2782,13 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ float_invalid_op_vxsnan(env, GETPC()); \ t.tfld = ttp##_snan_to_qnan(t.tfld); \ } \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_##ttp(env, t.tfld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_CVT_FP_TO_FP_HP(xscvdphp, 1, float64, float16, VsrD(0), VsrH(3), 1) @@ -3035,9 +3035,9 @@ VSX_CVT_FP_TO_INT_VECTOR(xscvqpuwz, float128, uint32, f128, VsrD(0), 0x0ULL) * sfld - source vsr_t field * tfld - target vsr_t field * jdef - definition of the j index (i or 2*i) - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, sfprf, r2sp) \ +#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, sfifprf, r2sp)\ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -3048,13 +3048,13 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ if (r2sp) { \ t.tfld = do_frsp(env, t.tfld, GETPC()); \ } \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.tfld); \ } \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, VsrD(0), VsrD(0), 1, 0) @@ -3136,9 +3136,9 @@ VSX_CVT_INT_TO_FP_VECTOR(xscvudqp, uint64, float128, VsrD(0), f128) * tp - type (float32 or float64) * fld - vsr_t field (VsrD(*) or VsrW(*)) * rmode - rounding mode - * sfprf - set FPRF + * sfifprf - set FI and FPRF */ -#define VSX_ROUND(op, nels, tp, fld, rmode, sfprf) \ +#define VSX_ROUND(op, nels, tp, fld, rmode, sfifprf) \ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ { \ ppc_vsr_t t = { }; \ @@ -3158,7 +3158,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } else { \ t.fld = tp##_round_to_int(xb->fld, &env->fp_status); \ } \ - if (sfprf) { \ + if (sfifprf) { \ helper_compute_fprf_float64(env, t.fld); \ } \ } \ @@ -3174,7 +3174,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ } \ \ *xt = t; \ - do_float_check_status(env, sfprf, GETPC()); \ + do_float_check_status(env, sfifprf, GETPC()); \ } VSX_ROUND(xsrdpi, 1, float64, VsrD(0), float_round_ties_away, 1) -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag 2022-05-10 20:46 ` [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag Víctor Colombo @ 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 1 sibling, 0 replies; 11+ messages in thread From: Richard Henderson @ 2022-05-10 23:58 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc; +Cc: clg, danielhb413, david, groug On 5/10/22 13:46, Víctor Colombo wrote: > The bit FI fix used the sfprf flag as a flag for the set_fi parameter > in do_float_check_status where applicable. Now, this patch rename this > flag to sfifprf to state this dual usage. > > Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br> > > --- > > v2: Add this patch > --- > target/ppc/fpu_helper.c | 112 ++++++++++++++++++++-------------------- > 1 file changed, 56 insertions(+), 56 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag 2022-05-10 20:46 ` [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag Víctor Colombo 2022-05-10 23:58 ` Richard Henderson @ 2022-05-11 10:12 ` Rashmica Gupta 1 sibling, 0 replies; 11+ messages in thread From: Rashmica Gupta @ 2022-05-11 10:12 UTC (permalink / raw) To: Víctor Colombo, qemu-devel, qemu-ppc Cc: clg, danielhb413, david, groug, richard.henderson [-- Attachment #1: Type: text/plain, Size: 2044 bytes --] On Tue, 2022-05-10 at 17:46 -0300, Víctor Colombo wrote: > The bit FI fix used the sfprf flag as a flag for the set_fi parameter > in do_float_check_status where applicable. Now, this patch rename > this > flag to sfifprf to state this dual usage. > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > > --- > > v2: Add this patch > --- > target/ppc/fpu_helper.c | 112 ++++++++++++++++++++------------------ > -- > 1 file changed, 56 insertions(+), 56 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index 88f9e756a5..7209150d1a 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -1693,9 +1693,9 @@ uint32_t helper_efdcmpeq(CPUPPCState *env, > uint64_t op1, uint64_t op2) > * nels - number of elements (1, 2 or 4) > * tp - type (float32 or float64) > * fld - vsr_t field (VsrD(*) or VsrW(*)) > - * sfprf - set FPRF > + * sfifprf - set FI and FPRF > */ > -#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf, > r2sp) \ > +#define VSX_ADD_SUB(name, op, nels, tp, fld, sfifprf, > r2sp) \ > void helper_##name(CPUPPCState *env, ppc_vsr_t > *xt, \ > ppc_vsr_t *xa, ppc_vsr_t > *xb) \ > { > \ ... \ > @@ -2767,9 +2767,9 @@ VSX_CVT_FP_TO_FP_VECTOR(xscvdpqp, 1, float64, > float128, VsrD(0), f128, 1) > * ttp - target type > * sfld - source vsr_t field > * tfld - target vsr_t field > - * sfprf - set FPRF > + * sfifprf - set FPRF set FI and FPRF? otherwise, Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com> [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-05-11 18:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-10 20:46 [PATCH v2 0/3] target/ppc: Fix FPSCR.FI bit Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 1/3] target/ppc: Fix FPSCR.FI bit being cleared when it shouldn't Víctor Colombo 2022-05-10 23:56 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 2022-05-11 18:29 ` Víctor Colombo 2022-05-10 20:46 ` [PATCH v2 2/3] target/ppc: Fix FPSCR.FI changing in float_overflow_excp() Víctor Colombo 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta 2022-05-10 20:46 ` [PATCH v2 3/3] target/ppc: Rename sfprf to sfifprf where it's also used as set fi flag Víctor Colombo 2022-05-10 23:58 ` Richard Henderson 2022-05-11 10:12 ` Rashmica Gupta
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.