[Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Aurelien Jarno]

cvt.s.d and cvt.d.s are FP operations and thus need to convert input
sNaN into corresponding qNaN. Explicitely use the floatXX_maybe_silence_nan
functions for that as the floatXX_to_floatXX functions do not do that.

Cc: Leon Alrae <leon.alrae@imgtec.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-mips/op_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index d2c98c9..20e79be 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2545,6 +2545,7 @@ uint64_t helper_float_cvtd_s(CPUMIPSState *env, uint32_t fst0)
     uint64_t fdt2;
 
     fdt2 = float32_to_float64(fst0, &env->active_fpu.fp_status);
+    fdt2 = float64_maybe_silence_nan(fdt2);
     update_fcr31(env, GETPC());
     return fdt2;
 }
@@ -2634,6 +2635,7 @@ uint32_t helper_float_cvts_d(CPUMIPSState *env, uint64_t fdt0)
     uint32_t fst2;
 
     fst2 = float64_to_float32(fdt0, &env->active_fpu.fp_status);
+    fst2 = float32_maybe_silence_nan(fst2);
     update_fcr31(env, GETPC());
     return fst2;
 }
-- 
2.6.2

Re: [Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Leon Alrae]

On 06/12/15 16:11, Aurelien Jarno wrote:
> cvt.s.d and cvt.d.s are FP operations and thus need to convert input
> sNaN into corresponding qNaN. Explicitely use the floatXX_maybe_silence_nan
> functions for that as the floatXX_to_floatXX functions do not do that.
> 
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/op_helper.c | 2 ++
>  1 file changed, 2 insertions(+)

Thanks, applied to target-mips queue.

Regards,
Leon

Re: [Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Maciej W. Rozycki]

On Sun, 6 Dec 2015, Aurelien Jarno wrote:

> cvt.s.d and cvt.d.s are FP operations and thus need to convert input
> sNaN into corresponding qNaN. Explicitely use the floatXX_maybe_silence_nan
> functions for that as the floatXX_to_floatXX functions do not do that.
> 
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/op_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index d2c98c9..20e79be 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -2545,6 +2545,7 @@ uint64_t helper_float_cvtd_s(CPUMIPSState *env, uint32_t fst0)
>      uint64_t fdt2;
>  
>      fdt2 = float32_to_float64(fst0, &env->active_fpu.fp_status);
> +    fdt2 = float64_maybe_silence_nan(fdt2);
>      update_fcr31(env, GETPC());
>      return fdt2;
>  }
> @@ -2634,6 +2635,7 @@ uint32_t helper_float_cvts_d(CPUMIPSState *env, uint64_t fdt0)
>      uint32_t fst2;
>  
>      fst2 = float64_to_float32(fdt0, &env->active_fpu.fp_status);
> +    fst2 = float32_maybe_silence_nan(fst2);
>      update_fcr31(env, GETPC());
>      return fst2;
>  }

 FYI, I posted a more general fix to this a while ago, however the review 
regrettably went nowhere.  See the archive of discussion starting at: 
<http://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg00969.html> 
for details, including the justification and further design consideration.

  Maciej

Re: [Qemu-devel] [PATCH] target-mips: silence NaNs for cvt.s.d and cvt.d.s

[Peter Maydell]

On 24 January 2016 at 03:42, Maciej W. Rozycki <macro@linux-mips.org> wrote:
>
>  FYI, I posted a more general fix to this a while ago, however the review
> regrettably went nowhere.  See the archive of discussion starting at:
> <http://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg00969.html>
> for details, including the justification and further design consideration.

The relevant review is in this bit of the thread
http://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg00897.html
(which you can't trivially get to from the original top level post
since the mailing list archive regrettably splits archives by month
and doesn't track threads between months).

ARM does the same thing that this patch does for MIPS (silence
NaNs in the calling function after the conversion happens), but as
I mention in that email this is broken and really should be dealt
with via target-specific hooks in the conversion routines.

thanks
-- PMM