| Message ID | SN6PR05MB5837B5EF0D11E0A787EC30179DA5A@SN6PR05MB5837.namprd05.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,RESEND,PING] target/ppc: Fix bugs in VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros | expand |
What is the status of integrating the fixes to the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros in target/ppc/fpu_helper.c? The bug that is currently there with the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros is that float_invalid_cvt is incorrectly called if the current source value is a non-NaN value and a previous NaN source value from the same source vector was encountered. The link to a patch that fixes the above bug in the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros can be found at https://patchew.org/QEMU/SN6PR05MB5837B5EF0D11E0A787EC30179DA5A@SN6PR05MB5837.namprd05.prod.outlook.com/. A description of this bug can be found at https://gitlab.com/qemu-project/qemu/-/issues/1941. Here is a link to a test program that will detect the bugs that are currently there with the emulation of the xvcvspsxws, xvcvspuxws, xvcvspsxds, xvcvspuxds, xvcvdpsxws, xvcvdpuxws, xvcvdpsxds, and xvcvdpuxds instructions if the source vector contains at least one NaN value: https://gitlab.com/qemu-project/qemu/uploads/fcbb97896ff2f4ab435affae94467f4d/vsx_f2i_nan_test_102523.c
Hello John, On 11/9/23 23:18, John Platts wrote: > What is the status of integrating the fixes to the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros in target/ppc/fpu_helper.c? You sent a couple of times the same patch : https://patchwork.ozlabs.org/project/qemu-ppc/list/?submitter=87426 I suppose v3 is the one to consider. > > The bug that is currently there with the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros is that float_invalid_cvt is incorrectly called if the current source value is a non-NaN value and a previous NaN source value from the same source vector was encountered. > > The link to a patch that fixes the above bug in the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros can be found at https://patchew.org/QEMU/SN6PR05MB5837B5EF0D11E0A787EC30179DA5A@SN6PR05MB5837.namprd05.prod.outlook.com/. > > A description of this bug can be found at https://gitlab.com/qemu-project/qemu/-/issues/1941. > > Here is a link to a test program that will detect the bugs that are currently there with the emulation of the xvcvspsxws, xvcvspuxws, xvcvspsxds, xvcvspuxds, xvcvdpsxws, xvcvdpuxws, xvcvdpsxds, and xvcvdpuxds instructions if the source vector contains at least one NaN value: > https://gitlab.com/qemu-project/qemu/uploads/fcbb97896ff2f4ab435affae94467f4d/vsx_f2i_nan_test_102523.c If you have time, could you please respin a v4 and include the test case under tests/tcg/ppc64 ? Please add a Fixes: tag with the commit id also. The plan is to merge fixes, such as this one, in the coming weeks, and I will send one PPC PR before the end of the 8.2 cycle. Thanks, C.
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index 03150a0f10..d6c8583416 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -2880,19 +2880,21 @@ uint64_t helper_XSCVSPDPN(uint64_t xb) #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 = 0; \ ppc_vsr_t t = { }; \ int i, flags; \ \ - helper_reset_fpstatus(env); \ - \ for (i = 0; i < nels; i++) { \ + helper_reset_fpstatus(env); \ t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status); \ flags = env->fp_status.float_exception_flags; \ + all_flags |= flags; \ if (unlikely(flags & float_flag_invalid)) { \ t.tfld = float_invalid_cvt(env, flags, t.tfld, rnan, 0, GETPC());\ } \ } \ \ + env->fp_status.float_exception_flags = all_flags; \ *xt = t; \ do_float_check_status(env, sfi, GETPC()); \ } @@ -2945,15 +2947,16 @@ VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000000000000000ULL); #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 = 0; \ ppc_vsr_t t = { }; \ int i, flags; \ \ - helper_reset_fpstatus(env); \ - \ for (i = 0; i < nels; i++) { \ + helper_reset_fpstatus(env); \ t.VsrW(2 * i) = stp##_to_##ttp##_round_to_zero(xb->VsrD(i), \ &env->fp_status); \ flags = env->fp_status.float_exception_flags; \ + all_flags |= flags; \ if (unlikely(flags & float_flag_invalid)) { \ t.VsrW(2 * i) = float_invalid_cvt(env, flags, t.VsrW(2 * i), \ rnan, 0, GETPC()); \ @@ -2961,6 +2964,7 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \ t.VsrW(2 * i + 1) = t.VsrW(2 * i); \ } \ \ + env->fp_status.float_exception_flags = all_flags; \ *xt = t; \ do_float_check_status(env, sfi, GETPC()); \ }
The patch below fixes a bug in the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros in target/ppc/fpu_helper.c where a non-NaN floating point value from the source vector is incorrectly converted to 0, 0x80000000, or 0x8000000000000000 instead of the expected value if a preceding source floating point value from the same source vector was a NaN. The bug in the VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 macros in target/ppc/fpu_helper.c was introduced with commit c3f24257. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1941 Signed-off-by: John Platts <john_platts@hotmail.com> --- target/ppc/fpu_helper.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)