[Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Aurelien Jarno]

This patchset should fix the bug #1701821 reported by Bruno Haible,
which makes the gnulib testsuite to fail for single precision libm
tests.

Aurelien Jarno (2):
  target/sh4: do not check for PR bit for fabs instruction
  target/sh4: do not use a helper to implement fneg

 target/sh4/helper.h    |  3 ---
 target/sh4/op_helper.c | 15 ---------------
 target/sh4/translate.c | 20 +++++---------------
 3 files changed, 5 insertions(+), 33 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH 1/2] target/sh4: do not check for PR bit for fabs instruction

[Aurelien Jarno]

The SH4 manual is not fully clear about that, but real hardware do not
check for the PR bit, which allows to select between single or double
precision, for the fabs instruction. This is probably what is meant by
"Same operation is performed regardless of precision."

Remove the check, and at the same time use a TCG instruction instead of
a helper to clear one bit.

LP: https://bugs.launchpad.net/qemu/+bug/1701821
Reported-by: Bruno Haible <bruno@clisp.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.h    |  2 --
 target/sh4/op_helper.c | 10 ----------
 target/sh4/translate.c | 15 +++------------
 3 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index dce859caea..f715224822 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -16,8 +16,6 @@ DEF_HELPER_3(macw, void, env, i32, i32)
 
 DEF_HELPER_2(ld_fpscr, void, env, i32)
 
-DEF_HELPER_FLAGS_1(fabs_FT, TCG_CALL_NO_RWG_SE, f32, f32)
-DEF_HELPER_FLAGS_1(fabs_DT, TCG_CALL_NO_RWG_SE, f64, f64)
 DEF_HELPER_FLAGS_3(fadd_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fadd_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fcnvsd_FT_DT, TCG_CALL_NO_WG, f64, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 528a40ac1d..5e3a3ba68c 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -252,16 +252,6 @@ static void update_fpscr(CPUSH4State *env, uintptr_t retaddr)
     }
 }
 
-float32 helper_fabs_FT(float32 t0)
-{
-    return float32_abs(t0);
-}
-
-float64 helper_fabs_DT(float64 t0)
-{
-    return float64_abs(t0);
-}
-
 float32 helper_fadd_FT(CPUSH4State *env, float32 t0, float32 t1)
 {
     set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 4f20537ef8..7c40945908 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1695,19 +1695,10 @@ static void _decode_opc(DisasContext * ctx)
 	    gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
 	}
 	return;
-    case 0xf05d: /* fabs FRn/DRn */
+    case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
 	CHECK_FPU_ENABLED
-        if (ctx->tbflags & FPSCR_PR) {
-	    if (ctx->opcode & 0x0100)
-		break; /* illegal instruction */
-	    TCGv_i64 fp = tcg_temp_new_i64();
-	    gen_load_fpr64(fp, DREG(B11_8));
-	    gen_helper_fabs_DT(fp, fp);
-	    gen_store_fpr64(fp, DREG(B11_8));
-	    tcg_temp_free_i64(fp);
-	} else {
-	    gen_helper_fabs_FT(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-	}
+        tcg_gen_andi_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+                         0x7fffffff);
 	return;
     case 0xf06d: /* fsqrt FRn */
 	CHECK_FPU_ENABLED
-- 
2.11.0

[Qemu-devel] [PATCH 2/2] target/sh4: do not use a helper to implement fneg

[Aurelien Jarno]

There is no need to use a helper to flip one bit, just use a TCG xor
instruction instead.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.h    | 1 -
 target/sh4/op_helper.c | 5 -----
 target/sh4/translate.c | 5 ++---
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index f715224822..d2398922dd 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -32,7 +32,6 @@ DEF_HELPER_FLAGS_2(float_DT, TCG_CALL_NO_WG, f64, env, i32)
 DEF_HELPER_FLAGS_4(fmac_FT, TCG_CALL_NO_WG, f32, env, f32, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
-DEF_HELPER_FLAGS_1(fneg_T, TCG_CALL_NO_RWG_SE, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fsqrt_FT, TCG_CALL_NO_WG, f32, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 5e3a3ba68c..d561141301 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -396,11 +396,6 @@ float64 helper_fmul_DT(CPUSH4State *env, float64 t0, float64 t1)
     return t0;
 }
 
-float32 helper_fneg_T(float32 t0)
-{
-    return float32_chs(t0);
-}
-
 float32 helper_fsqrt_FT(CPUSH4State *env, float32 t0)
 {
     set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 7c40945908..8098228c51 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1691,9 +1691,8 @@ static void _decode_opc(DisasContext * ctx)
 	return;
     case 0xf04d: /* fneg FRn/DRn - FPSCR: Nothing */
 	CHECK_FPU_ENABLED
-	{
-	    gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-	}
+        tcg_gen_xori_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+                         0x80000000);
 	return;
     case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
 	CHECK_FPU_ENABLED
-- 
2.11.0

Re: [Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Bruno Haible]

Hi Aurelien,

> This patchset should fix the bug #1701821 reported by Bruno Haible,
> which makes the gnulib testsuite to fail for single precision libm
> tests.
> 
> Aurelien Jarno (2):
>   target/sh4: do not check for PR bit for fabs instruction
>   target/sh4: do not use a helper to implement fneg

Thanks. It fixes most of the reported issues indeed. The following tests
still fail, though:

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-floor2
../../gltests/test-floor2.c:130: assertion 'correct_result_p (x, reference)' failed
qemu: uncaught target signal 6 (Aborted) - core dumped

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-round2
reference implementations disagree: round(nan(nan)) = nan(nan) or 0(0x0p+0)?

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-roundf2
reference implementations disagree: roundf(nan(nan)) = nan(nan) or 0(0x0p+0)?

But these could also be glibc bugs in the respective functions; I cannot tell.

Bruno

Re: [Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Richard Henderson]

On 07/02/2017 09:32 AM, Aurelien Jarno wrote:
> This patchset should fix the bug #1701821 reported by Bruno Haible,
> which makes the gnulib testsuite to fail for single precision libm
> tests.
> 
> Aurelien Jarno (2):
>    target/sh4: do not check for PR bit for fabs instruction
>    target/sh4: do not use a helper to implement fneg
> 
>   target/sh4/helper.h    |  3 ---
>   target/sh4/op_helper.c | 15 ---------------
>   target/sh4/translate.c | 20 +++++---------------
>   3 files changed, 5 insertions(+), 33 deletions(-)
> 

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~