Re: [Qemu-devel] [PATCH v2 1/5] target-tricore: Added FTOUZ instruction

[Richard Henderson]

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 <address@hidden>
> ---
> 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~