| Message ID | 20161107144445.14069-2-kbastian@mail.uni-paderborn.de |
|---|---|
| State | New |
| Headers | show |
On 11/07/2016 03:44 PM, Bastian Koppelmann wrote: > Converts a 32-bit floating point number to an unsigned int. The > result is rounded towards zero. > > Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > --- > v1 -> v2: > - ftouz: Correctly convert the result from uint32 to f32 > > target-tricore/fpu_helper.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > target-tricore/helper.h | 1 + > target-tricore/translate.c | 3 +++ > 3 files changed, 47 insertions(+) > > diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c > index 98fe947..f717b53 100644 > --- a/target-tricore/fpu_helper.c > +++ b/target-tricore/fpu_helper.c > @@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env, uint32_t arg) > } > return (uint32_t)f_result; > } > + > +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg) > +{ > + float32 f_arg = make_float32(arg); > + uint32_t result; > + int32_t flags; > + result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status); > + > + flags = f_get_excp_flags(env); > + if (flags) { > + if (float32_is_any_nan(f_arg)) { > + flags |= float_flag_invalid; > + result = 0; > + /* f_real(D[a]) < 0.0 */ > + } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) { > + flags |= float_flag_invalid; > + result = 0; > + /* f_real(D[a]) > 2^32 -1 */ > + } else if (float32_lt_quiet(0x4f800000, f_arg, &env->fp_status)) { > + flags |= float_flag_invalid; > + result = 0xffffffff; > + } else { > + flags &= ~float_flag_invalid; > + } I don't understand this bit. Surely float_flag_invalid is already set by softfloat. And the bounding is done as well. What's up? > + /* once invalid flag has been set, we cannot set inexact anymore > + since each FPU operation can only assert ONE flag. (see > + TriCore ISA Manual Vol. 1 (11-9)) */ > + if (!(flags & float_flag_invalid)) { > + if (!float32_eq(f_arg, uint32_to_float32(result, &env->fp_status), > + &env->fp_status)) { > + flags |= float_flag_inexact; > + } else { > + flags &= ~float_flag_inexact; > + } > + } else { > + flags &= ~float_flag_inexact; > + } And inexact is already set as well. The best I can think is meant is /* Only one flag can be asserted. */ if (flags & float_flag_invalid) { flags &= ~float_flag_inexact; } > + f_update_psw_flags(env, flags); > + } else { > + env->FPU_FS = 0; > + } > + return result; > +} r~
On 11/08/2016 12:36 PM, Richard Henderson wrote: > On 11/07/2016 03:44 PM, Bastian Koppelmann wrote: >> Converts a 32-bit floating point number to an unsigned int. The >> result is rounded towards zero. >> >> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> >> --- >> v1 -> v2: >> - ftouz: Correctly convert the result from uint32 to f32 >> >> target-tricore/fpu_helper.c | 43 >> +++++++++++++++++++++++++++++++++++++++++++ >> target-tricore/helper.h | 1 + >> target-tricore/translate.c | 3 +++ >> 3 files changed, 47 insertions(+) >> >> diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c >> index 98fe947..f717b53 100644 >> --- a/target-tricore/fpu_helper.c >> +++ b/target-tricore/fpu_helper.c >> @@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env, >> uint32_t arg) >> } >> return (uint32_t)f_result; >> } >> + >> +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg) >> +{ >> + float32 f_arg = make_float32(arg); >> + uint32_t result; >> + int32_t flags; >> + result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status); >> + >> + flags = f_get_excp_flags(env); >> + if (flags) { >> + if (float32_is_any_nan(f_arg)) { >> + flags |= float_flag_invalid; >> + result = 0; >> + /* f_real(D[a]) < 0.0 */ >> + } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) { >> + flags |= float_flag_invalid; >> + result = 0; >> + /* f_real(D[a]) > 2^32 -1 */ >> + } else if (float32_lt_quiet(0x4f800000, f_arg, >> &env->fp_status)) { >> + flags |= float_flag_invalid; >> + result = 0xffffffff; >> + } else { >> + flags &= ~float_flag_invalid; >> + } > > I don't understand this bit. Surely float_flag_invalid is already set > by softfloat. And the bounding is done as well. What's up? > Consider 0x836d4e86 as an input which is clearly negative, however float_flag_invalid is not set. The hardware on the other hand does set it. Cheers, Bastian
On 11/08/2016 02:37 PM, Bastian Koppelmann wrote: > Consider 0x836d4e86 as an input which is clearly negative, however > float_flag_invalid is not set. The hardware on the other hand does set it. Hmm. This is -0x1.da9d0cp-121. Softfloat claims that we should round to zero first, and only if the result is still < 0, raise invalid. Which does sound plausible as a common behaviour. Does your hardware raise invalid for true -0.0, i.e. 0x80000000? r~
On 11/08/2016 04:06 PM, Richard Henderson wrote: > On 11/08/2016 02:37 PM, Bastian Koppelmann wrote: >> Consider 0x836d4e86 as an input which is clearly negative, however >> float_flag_invalid is not set. The hardware on the other hand does set >> it. > > Hmm. This is -0x1.da9d0cp-121. Softfloat claims that we should round > to zero first, and only if the result is still < 0, raise invalid. > Which does sound plausible as a common behaviour. TriCore does it the other way round. > > Does your hardware raise invalid for true -0.0, i.e. 0x80000000? No, -0.0 does not raise invalid. Cheers, Bastian
On 11/08/2016 04:12 PM, Bastian Koppelmann wrote: > On 11/08/2016 04:06 PM, Richard Henderson wrote: >> On 11/08/2016 02:37 PM, Bastian Koppelmann wrote: >>> Consider 0x836d4e86 as an input which is clearly negative, however >>> float_flag_invalid is not set. The hardware on the other hand does set >>> it. >> >> Hmm. This is -0x1.da9d0cp-121. Softfloat claims that we should round >> to zero first, and only if the result is still < 0, raise invalid. >> Which does sound plausible as a common behaviour. > > TriCore does it the other way round. > >> >> Does your hardware raise invalid for true -0.0, i.e. 0x80000000? > > No, -0.0 does not raise invalid. Then I suppose the check should look like flags = f_get_excp_flags(env); if (flags & float_flag_invalid) { flags &= ~float_flag_inexact; } else if (float32_lt_quiet(f_arg, 0, &env->status)) { flags = float_flag_invalid; } if (flags) { f_update_psw_flags(env, flags); } Note that the 0.0 that you use is truncated to 0 by C for the uint32_t argument. r~
On 11/08/2016 04:25 PM, Richard Henderson wrote: > On 11/08/2016 04:12 PM, Bastian Koppelmann wrote: >> On 11/08/2016 04:06 PM, Richard Henderson wrote: >>> On 11/08/2016 02:37 PM, Bastian Koppelmann wrote: >>>> Consider 0x836d4e86 as an input which is clearly negative, however >>>> float_flag_invalid is not set. The hardware on the other hand does set >>>> it. >>> >>> Hmm. This is -0x1.da9d0cp-121. Softfloat claims that we should round >>> to zero first, and only if the result is still < 0, raise invalid. >>> Which does sound plausible as a common behaviour. >> >> TriCore does it the other way round. >> >>> >>> Does your hardware raise invalid for true -0.0, i.e. 0x80000000? >> >> No, -0.0 does not raise invalid. > > Then I suppose the check should look like > > flags = f_get_excp_flags(env); > if (flags & float_flag_invalid) { > flags &= ~float_flag_inexact; > } else if (float32_lt_quiet(f_arg, 0, &env->status)) { > flags = float_flag_invalid; > } > if (flags) { > f_update_psw_flags(env, flags); > } > > Note that the 0.0 that you use is truncated to 0 by C for the uint32_t > argument. Not quite -- it does not catch that an input NaN results in 0 as opposed to -1 returned by softfloat. But otherwise thanks for the review. This resulted in much nicer code. Cheers, Bastian
diff --git a/target-tricore/fpu_helper.c b/target-tricore/fpu_helper.c index 98fe947..f717b53 100644 --- a/target-tricore/fpu_helper.c +++ b/target-tricore/fpu_helper.c @@ -215,3 +215,46 @@ uint32_t helper_itof(CPUTriCoreState *env, uint32_t arg) } return (uint32_t)f_result; } + +uint32_t helper_ftouz(CPUTriCoreState *env, uint32_t arg) +{ + float32 f_arg = make_float32(arg); + uint32_t result; + int32_t flags; + result = float32_to_uint32_round_to_zero(f_arg, &env->fp_status); + + flags = f_get_excp_flags(env); + if (flags) { + if (float32_is_any_nan(f_arg)) { + flags |= float_flag_invalid; + result = 0; + /* f_real(D[a]) < 0.0 */ + } else if (float32_lt_quiet(f_arg, 0.0, &env->fp_status)) { + flags |= float_flag_invalid; + result = 0; + /* f_real(D[a]) > 2^32 -1 */ + } else if (float32_lt_quiet(0x4f800000, f_arg, &env->fp_status)) { + flags |= float_flag_invalid; + result = 0xffffffff; + } else { + flags &= ~float_flag_invalid; + } + /* once invalid flag has been set, we cannot set inexact anymore + since each FPU operation can only assert ONE flag. (see + TriCore ISA Manual Vol. 1 (11-9)) */ + if (!(flags & float_flag_invalid)) { + if (!float32_eq(f_arg, uint32_to_float32(result, &env->fp_status), + &env->fp_status)) { + flags |= float_flag_inexact; + } else { + flags &= ~float_flag_inexact; + } + } else { + flags &= ~float_flag_inexact; + } + f_update_psw_flags(env, flags); + } else { + env->FPU_FS = 0; + } + return result; +} diff --git a/target-tricore/helper.h b/target-tricore/helper.h index 9333e16..467c880 100644 --- a/target-tricore/helper.h +++ b/target-tricore/helper.h @@ -112,6 +112,7 @@ DEF_HELPER_3(fdiv, i32, env, i32, i32) DEF_HELPER_3(fcmp, i32, env, i32, i32) DEF_HELPER_2(ftoi, i32, env, i32) DEF_HELPER_2(itof, i32, env, i32) +DEF_HELPER_2(ftouz, i32, env, i32) /* dvinit */ DEF_HELPER_3(dvinit_b_13, i64, env, i32, i32) DEF_HELPER_3(dvinit_b_131, i64, env, i32, i32) diff --git a/target-tricore/translate.c b/target-tricore/translate.c index 9a50df9..27c6d31 100644 --- a/target-tricore/translate.c +++ b/target-tricore/translate.c @@ -6698,6 +6698,9 @@ static void decode_rr_divide(CPUTriCoreState *env, DisasContext *ctx) case OPC2_32_RR_ITOF: gen_helper_itof(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]); break; + case OPC2_32_RR_FTOUZ: + gen_helper_ftouz(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]); + break; default: generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC); }
Converts a 32-bit floating point number to an unsigned int. The result is rounded towards zero. Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> --- v1 -> v2: - ftouz: Correctly convert the result from uint32 to f32 target-tricore/fpu_helper.c | 43 +++++++++++++++++++++++++++++++++++++++++++ target-tricore/helper.h | 1 + target-tricore/translate.c | 3 +++ 3 files changed, 47 insertions(+)