Re: [Qemu-devel] [PATCH 7/7] target-mips: Add IEEE 754-2008 features support

[Maciej W. Rozycki]

On Tue, 9 Dec 2014, Maciej W. Rozycki wrote:

> Index: qemu-git-trunk/target-mips/op_helper.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/op_helper.c       2014-12-08 
> 23:22:12.000000000 +0000
> +++ qemu-git-trunk/target-mips/op_helper.c    2014-12-08 23:25:02.558929097 
> +0000
> @@ -2274,8 +2274,12 @@ void mips_cpu_unassigned_access(CPUState
>  
>  #define FLOAT_TWO32 make_float32(1 << 30)
>  #define FLOAT_TWO64 make_float64(1ULL << 62)
> -#define FP_TO_INT32_OVERFLOW 0x7fffffff
> -#define FP_TO_INT64_OVERFLOW 0x7fffffffffffffffULL
> +#define FLOAT_INAN32(env)                                               \
> +    (((env)->active_fpu.fcr31 & (1 << FCR31_NAN2008))                   \
> +     ? 0x00000000 : 0x7fffffff)
> +#define FLOAT_INAN64(env)                                               \
> +    (((env)->active_fpu.fcr31 & (1 << FCR31_NAN2008))                   \
> +     ? 0x0000000000000000ULL : 0x7fffffffffffffffULL)
>  
>  /* convert MIPS rounding mode in FCR31 to IEEE library */
>  unsigned int ieee_rm[] = {
[...]
> @@ -2481,7 +2482,7 @@ uint64_t helper_float_cvtl_d(CPUMIPSStat
>      dt2 = float64_to_int64(fdt0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2494,7 +2495,7 @@ uint64_t helper_float_cvtl_s(CPUMIPSStat
>      dt2 = float32_to_int64(fst0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2520,14 +2521,14 @@ uint64_t helper_float_cvtpw_ps(CPUMIPSSt
>      wt2 = float32_to_int32(fdt0 & 0XFFFFFFFF, &env->active_fpu.fp_status);
>      excp = get_float_exception_flags(&env->active_fpu.fp_status);
>      if (excp & (float_flag_overflow | float_flag_invalid)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>  
>      set_float_exception_flags(0, &env->active_fpu.fp_status);
>      wth2 = float32_to_int32(fdt0 >> 32, &env->active_fpu.fp_status);
>      excph = get_float_exception_flags(&env->active_fpu.fp_status);
>      if (excph & (float_flag_overflow | float_flag_invalid)) {
> -        wth2 = FP_TO_INT32_OVERFLOW;
> +        wth2 = FLOAT_INAN32(env);
>      }
>  
>      set_float_exception_flags(excp | excph, &env->active_fpu.fp_status);
> @@ -2588,7 +2589,7 @@ uint32_t helper_float_cvtw_s(CPUMIPSStat
>      wt2 = float32_to_int32(fst0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2601,7 +2602,7 @@ uint32_t helper_float_cvtw_d(CPUMIPSStat
>      wt2 = float64_to_int32(fdt0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2616,7 +2617,7 @@ uint64_t helper_float_roundl_d(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2631,7 +2632,7 @@ uint64_t helper_float_roundl_s(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2646,7 +2647,7 @@ uint32_t helper_float_roundw_d(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2661,7 +2662,7 @@ uint32_t helper_float_roundw_s(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2674,7 +2675,7 @@ uint64_t helper_float_truncl_d(CPUMIPSSt
>      dt2 = float64_to_int64_round_to_zero(fdt0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2687,7 +2688,7 @@ uint64_t helper_float_truncl_s(CPUMIPSSt
>      dt2 = float32_to_int64_round_to_zero(fst0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2700,7 +2701,7 @@ uint32_t helper_float_truncw_d(CPUMIPSSt
>      wt2 = float64_to_int32_round_to_zero(fdt0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2713,7 +2714,7 @@ uint32_t helper_float_truncw_s(CPUMIPSSt
>      wt2 = float32_to_int32_round_to_zero(fst0, &env->active_fpu.fp_status);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2728,7 +2729,7 @@ uint64_t helper_float_ceill_d(CPUMIPSSta
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2743,7 +2744,7 @@ uint64_t helper_float_ceill_s(CPUMIPSSta
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2758,7 +2759,7 @@ uint32_t helper_float_ceilw_d(CPUMIPSSta
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2773,7 +2774,7 @@ uint32_t helper_float_ceilw_s(CPUMIPSSta
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2788,7 +2789,7 @@ uint64_t helper_float_floorl_d(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2803,7 +2804,7 @@ uint64_t helper_float_floorl_s(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        dt2 = FP_TO_INT64_OVERFLOW;
> +        dt2 = FLOAT_INAN64(env);
>      }
>      update_fcr31(env, GETPC());
>      return dt2;
> @@ -2818,7 +2819,7 @@ uint32_t helper_float_floorw_d(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;
> @@ -2833,23 +2834,23 @@ uint32_t helper_float_floorw_s(CPUMIPSSt
>      restore_rounding_mode(env);
>      if (get_float_exception_flags(&env->active_fpu.fp_status)
>          & (float_flag_invalid | float_flag_overflow)) {
> -        wt2 = FP_TO_INT32_OVERFLOW;
> +        wt2 = FLOAT_INAN32(env);
>      }
>      update_fcr31(env, GETPC());
>      return wt2;

 I have realised these all need a further update to follow the 
architecture specification correctly in the 2008-NaN mode.  This is 
because in the legacy-NaN mode a single value of `maxint' is returned for 
all invalid operations, however in the 2008-NaN mode distinct values are 
returned depending of the nature of the invalidity as follows [1][2]:

* `minint' -- for negative operand overflows,

* 0 -- for NaN operands,

* `maxint' -- for positive operand overflows.

(note that the values required here were changed between revisions 3.50 
and 5.00 of the architecture specifications).  It should be 
straightforward to implement as `float64_to_int64', etc. already return 
the correct values for overflows, so it's only NaN operands that need a 
fixup similar to the above, in the 2008-NaN mode.

 Additionally the reference to the `float_flag_overflow' flag can and IMO 
should be removed here, it's never set for conversions to a fixed-point 
integer format.

 References:

[1] "MIPS Architecture For Programmers, Volume I-A: Introduction to the
    MIPS32 Architecture", MIPS Technologies, Inc., Document Number:
    MD00082, Revision 5.00, December 14, 2012, Table 5.3 "Value Supplied 
    When a New Quiet NaN Is Created", p. 72

[2] "MIPS Architecture For Programmers, Volume I-A: Introduction to the
    MIPS64 Architecture", MIPS Technologies, Inc., Document Number:
    MD00083, Revision 5.00, December 14, 2012, Table 5.3 "Value Supplied 
    When a New Quiet NaN Is Created", p. 74

  Maciej