Message ID | 20210517142739.38597-2-david@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | s390x/tcg: Implement Vector enhancements facility and switch to z14 | expand |
On 5/17/21 7:27 AM, David Hildenbrand wrote: > @@ -634,6 +664,9 @@ uint64_t HELPER(clfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) > > s390_restore_bfp_rounding_mode(env, old_mode); > handle_exceptions(env, xxc_from_m34(m34), GETPC()); > + if (float128_is_any_nan(make_float128(h, l))) { > + return 0; > + } I wonder if handle_exceptions should return s390_exc. Then you can test exc = handle_exceptions(...); if (unlikely(exc & S390_IEEE_MASK_INVALID)) { ret = 0; } return ret; > +++ b/target/s390x/vec_fpu_helper.c > @@ -326,6 +326,9 @@ void HELPER(gvec_vcdlg64s)(void *v1, const void *v2, CPUS390XState *env, > > static uint64_t vcgd64(uint64_t a, float_status *s) > { > + if (float64_is_any_nan(a)) { > + return INT64_MIN; > + } > return float64_to_int64(a, s); > } > > @@ -349,6 +352,9 @@ void HELPER(gvec_vcgd64s)(void *v1, const void *v2, CPUS390XState *env, > > static uint64_t vclgd64(uint64_t a, float_status *s) > { > + if (float64_is_any_nan(a)) { > + return 0; > + } > return float64_to_uint64(a, s); > } You do still need to raise invalid, as far as I can see. r~
On 01.06.21 23:27, Richard Henderson wrote: > On 5/17/21 7:27 AM, David Hildenbrand wrote: >> @@ -634,6 +664,9 @@ uint64_t HELPER(clfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) >> >> s390_restore_bfp_rounding_mode(env, old_mode); >> handle_exceptions(env, xxc_from_m34(m34), GETPC()); >> + if (float128_is_any_nan(make_float128(h, l))) { >> + return 0; >> + } > > I wonder if handle_exceptions should return s390_exc. > Then you can test > > exc = handle_exceptions(...); > if (unlikely(exc & S390_IEEE_MASK_INVALID)) { > ret = 0; > } > return ret; > I'll give it a thought if that makes things easier. > > >> +++ b/target/s390x/vec_fpu_helper.c >> @@ -326,6 +326,9 @@ void HELPER(gvec_vcdlg64s)(void *v1, const void *v2, CPUS390XState *env, >> >> static uint64_t vcgd64(uint64_t a, float_status *s) >> { >> + if (float64_is_any_nan(a)) { >> + return INT64_MIN; >> + } >> return float64_to_int64(a, s); >> } >> >> @@ -349,6 +352,9 @@ void HELPER(gvec_vcgd64s)(void *v1, const void *v2, CPUS390XState *env, >> >> static uint64_t vclgd64(uint64_t a, float_status *s) >> { >> + if (float64_is_any_nan(a)) { >> + return 0; >> + } >> return float64_to_uint64(a, s); >> } > > You do still need to raise invalid, as far as I can see. Good point, so maybe uint64_t ret = float64_to_uint64(a, s); /* Note: check after converting to properly raise exceptions. */ if (float64_is_any_nan(a)) { ret = 0; } return ret; to minimize manual handling?
On 02.06.21 11:50, David Hildenbrand wrote: > On 01.06.21 23:27, Richard Henderson wrote: >> On 5/17/21 7:27 AM, David Hildenbrand wrote: >>> @@ -634,6 +664,9 @@ uint64_t HELPER(clfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) >>> >>> s390_restore_bfp_rounding_mode(env, old_mode); >>> handle_exceptions(env, xxc_from_m34(m34), GETPC()); >>> + if (float128_is_any_nan(make_float128(h, l))) { >>> + return 0; >>> + } >> >> I wonder if handle_exceptions should return s390_exc. >> Then you can test >> >> exc = handle_exceptions(...); >> if (unlikely(exc & S390_IEEE_MASK_INVALID)) { >> ret = 0; >> } >> return ret; >> > > I'll give it a thought if that makes things easier. Looking at partsN(float_to_uint), we also raise float_flag_invalid in other cases, for example with float_class_inf. So IIUC, your proposal would also set "ret = 0" in case our input is +inf, which would be wrong.
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c index f155bc048c..13af158748 100644 --- a/target/s390x/fpu_helper.c +++ b/target/s390x/fpu_helper.c @@ -509,6 +509,9 @@ uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float32_is_any_nan(v2)) { + return INT64_MIN; + } return ret; } @@ -520,6 +523,9 @@ uint64_t HELPER(cgdb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float64_is_any_nan(v2)) { + return INT64_MIN; + } return ret; } @@ -532,6 +538,9 @@ uint64_t HELPER(cgxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float128_is_any_nan(v2)) { + return INT64_MIN; + } return ret; } @@ -543,6 +552,9 @@ uint64_t HELPER(cfeb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float32_is_any_nan(v2)) { + return INT32_MIN; + } return ret; } @@ -554,6 +566,9 @@ uint64_t HELPER(cfdb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float64_is_any_nan(v2)) { + return INT32_MIN; + } return ret; } @@ -566,6 +581,9 @@ uint64_t HELPER(cfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float128_is_any_nan(v2)) { + return INT32_MIN; + } return ret; } @@ -573,12 +591,12 @@ uint64_t HELPER(cfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) uint64_t HELPER(clgeb)(CPUS390XState *env, uint64_t v2, uint32_t m34) { int old_mode = s390_swap_bfp_rounding_mode(env, round_from_m34(m34)); - uint64_t ret; - - v2 = float32_to_float64(v2, &env->fpu_status); - ret = float64_to_uint64(v2, &env->fpu_status); + uint64_t ret = float32_to_uint64(v2, &env->fpu_status); s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float32_is_any_nan(v2)) { + return 0; + } return ret; } @@ -590,6 +608,9 @@ uint64_t HELPER(clgdb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float64_is_any_nan(v2)) { + return 0; + } return ret; } @@ -601,6 +622,9 @@ uint64_t HELPER(clgxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float128_is_any_nan(make_float128(h, l))) { + return 0; + } return ret; } @@ -612,6 +636,9 @@ uint64_t HELPER(clfeb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float32_is_any_nan(v2)) { + return 0; + } return ret; } @@ -623,6 +650,9 @@ uint64_t HELPER(clfdb)(CPUS390XState *env, uint64_t v2, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float64_is_any_nan(v2)) { + return 0; + } return ret; } @@ -634,6 +664,9 @@ uint64_t HELPER(clfxb)(CPUS390XState *env, uint64_t h, uint64_t l, uint32_t m34) s390_restore_bfp_rounding_mode(env, old_mode); handle_exceptions(env, xxc_from_m34(m34), GETPC()); + if (float128_is_any_nan(make_float128(h, l))) { + return 0; + } return ret; } diff --git a/target/s390x/vec_fpu_helper.c b/target/s390x/vec_fpu_helper.c index c1564e819b..d485837930 100644 --- a/target/s390x/vec_fpu_helper.c +++ b/target/s390x/vec_fpu_helper.c @@ -326,6 +326,9 @@ void HELPER(gvec_vcdlg64s)(void *v1, const void *v2, CPUS390XState *env, static uint64_t vcgd64(uint64_t a, float_status *s) { + if (float64_is_any_nan(a)) { + return INT64_MIN; + } return float64_to_int64(a, s); } @@ -349,6 +352,9 @@ void HELPER(gvec_vcgd64s)(void *v1, const void *v2, CPUS390XState *env, static uint64_t vclgd64(uint64_t a, float_status *s) { + if (float64_is_any_nan(a)) { + return 0; + } return float64_to_uint64(a, s); }
In case we encounter a NaN, we have to return the smallest possible number, corresponding to either 0 or the maximum negative number. This seems to differ from IEEE handling as implemented in softfloat, whereby we return the biggest possible number. While at it, use float32_to_uint64() in the CLGEB handler. Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/fpu_helper.c | 41 +++++++++++++++++++++++++++++++---- target/s390x/vec_fpu_helper.c | 6 +++++ 2 files changed, 43 insertions(+), 4 deletions(-)