target-ppc: Bug: VSX Convert to Integer Should Truncate
PATCH 1/1 target-ppc: Bug: VSX Convert to Integer Should Truncate
The various VSX Convert to Integer instructions should truncate the mantissa. This fix forces the softfloat rounding mode to "round to zero" prior to performing the conversion. After the conversion is completed, the internal rounding mode is restored from the PowerPC FPSCR bits.
---
This bug was discovered when running wget, which does this:
double maxtime;
struct timeval tmout;
...
tmout.tv_usec = 1000000 * (maxtime - (long) maxtime);The newest PowerPC 64-bit gcc's are now using xscvdpsxds to perform the cast of the double to long. A timeout of 0.95 was erroneously rounding up to 1 and hence computing a negative timeout value.
It would be great if we could still get this into 2.0.
target-ppc/fpu_helper.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
target-ppc/fpu_helper.c +4 -0
@@ -2558,10 +2558,14 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ xt.tfld = rnan; \ }else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode (float_round_to_zero, &env->fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, &env->fp_status); \ if (env->fp_status.float_exception_flags & float_flag_invalid) { \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0); \ } \ +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode (env); \ } \ } \ \
Signature · 1 line
1.7.1
Quoted 5 lines · Tom Musta
target-ppc/fpu_helper.c +0
-0
target-ppc/fpu_helper.c void helper_##op(CPUPPCState *env, uint32_t opcode) \ · line 2566
Quoted 9 lines
target-ppc/fpu_helper.c +2
-0@@ -2558,10 +2558,14 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ xt.tfld = rnan; \ }else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode (float_round_to_zero, &env->fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, &env->fp_status); \ if (env->fp_status.float_exception_flags & float_flag_invalid) { \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0); \ } \
If we raise a CPU exception here (via helper_raise_exception_err()) we'll longjmp out of here and never restore the rounding mode. So the restoring of the rounding mode needs to happen before we check for exceptions here.
target-ppc/fpu_helper.c void helper_##op(CPUPPCState *env, uint32_t opcode) \ · line 2571
Quoted 5 lines
target-ppc/fpu_helper.c +2
-0+/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode (env); \ } \ } \ \
thanks
-- PMM
PATCH v2 1/1 target-ppc: Bug: VSX Convert to Integer Should Truncate
The various VSX Convert to Integer instructions should truncate the mantissa. This fix forces the softfloat rounding mode to "round to zero" prior to performing the conversion. After the conversion is completed, the internal rounding mode is restored from the PowerPC FPSCR bits.
---
V2: Restored rounding mode prior to checking exceptions per Peter Maydell's review.
This bug was discovered when running wget, which does this:
double maxtime;
struct timeval tmout;
...
tmout.tv_usec = 1000000 * (maxtime - (long) maxtime);The newest PowerPC 64-bit gcc's are now using xscvdpsxds to perform the cast of the double to long. A timeout of 0.95 was erroneously rounding up to 1 and hence computing a negative timeout value.
It would be great if we could still get this into 2.0.
target-ppc/fpu_helper.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
target-ppc/fpu_helper.c +4 -0
@@ -2568,7 +2568,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ xt.tfld = rnan; \ }else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode (float_round_to_zero, &env->fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, &env->fp_status); \ +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode (env); \if (env->fp_status.float_exception_flags & float_flag_invalid) { \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ } \
Signature · 1 line
1.7.1
Quoted 44 lines · Tom Musta
The various VSX Convert to Integer instructions should truncate the mantissa. This fix forces the softfloat rounding mode to "round to zero" prior to performing the conversion. After the conversion is completed, the internal rounding mode is restored from the PowerPC FPSCR bits.
Signed-off-by Tom Musta <[email protected]>---
V2: Restored rounding mode prior to checking exceptions per Peter Maydell's review.This bug was discovered when running wget, which does this:
double maxtime; struct timeval tmout; ... tmout.tv_usec = 1000000 * (maxtime - (long) maxtime);The newest PowerPC 64-bit gcc's are now using xscvdpsxds to perform the cast of the double to long. A timeout of 0.95 was erroneously rounding up to 1 and hence computing a negative timeout value.
It would be great if we could still get this into 2.0.
target-ppc/fpu_helper.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)target-ppc/fpu_helper.c +4
-1@@ -2568,7 +2568,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ xt.tfld = rnan; \ }else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode (float_round_to_zero, &env->fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, &env->fp_status); \ +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode (env); \if (env->fp_status.float_exception_flags & float_flag_invalid) { \fload_invalid_op_excp (env,POWERPC_EXCP_FP_VXCVI ,0 ); \ } \ --
Quoted 1 line
1.7.1
Looking at this a little more closely, why aren't we just using the _round_to_zero versions of the float to int conversion softfloat functions? (This is how we implement fctiwz vs fctiw, for instance.)
thanks
-- PMM
Quoted 9 lines · Peter Maydell
Looking at this a little more closely, why aren't we just using the
_round_to_zeroversions of the float to int conversion softfloat functions? (This is how we implement fctiwz vs fctiw, for instance.)thanks
-- PMM
Fair question, Peter. I will recode, retest and resubmit. Also, I have found a rather pervasive problem with the VSX helper code when running on LE hosts and mixing load/store styles. The fix touches this same code, so I will re-publish this one as part of a series.
Quoted 4 lines · Peter Maydell
Looking at this a little more closely, why aren't we just using the
_round_to_zeroversions of the float to int conversion softfloat functions? (This is how we implement fctiwz vs fctiw, for instance.)
Unfortunately, the softfloat list of these functions is not complete. Specifically, float32_to_uint64_round_to_zero is missing. I can add it but this means a bigger patch. Thoughts?
Quoted 10 lines · Tom Musta
Quoted 4 lines · Peter Maydell
Looking at this a little more closely, why aren't we just using the
_round_to_zeroversions of the float to int conversion softfloat functions? (This is how we implement fctiwz vs fctiw, for instance.)Unfortunately, the softfloat list of these functions is not complete. Specifically,
float32_to_uint64_round_to_zerois missing. I can add it but this means a bigger patch. Thoughts?
Please do.
r~