Patchwork target-mips: fix rndrashift_short_acc and code for EXTR_ instructions

login
register
mail settings
Submitter Petar Jovanovic
Date March 15, 2013, 5:56 p.m.
Message ID <1363370179-118188-1-git-send-email-petar.jovanovic@rt-rk.com>
Download mbox | patch
Permalink /patch/228129/
State New
Headers show

Comments

Petar Jovanovic - March 15, 2013, 5:56 p.m.
From: Petar Jovanovic <petar.jovanovic@imgtec.com>

Fix for rndrashift_short_acc to set correct value to higher 64 bits.
This change also corrects conditions when bit 23 of the DSPControl register
is set.

The existing test files have been extended with several examples that
trigger the issues. One bug/example in the test file for EXTR_RS_W has been
found and reported by Klaus Peichl.

Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
 target-mips/dsp_helper.c              |   23 +++++++----------
 tests/tcg/mips/mips32-dsp/extr_r_w.c  |   23 +++++++++++++++++
 tests/tcg/mips/mips32-dsp/extr_rs_w.c |   46 +++++++++++++++++++++++++++++++++
 tests/tcg/mips/mips32-dsp/extr_w.c    |   23 +++++++++++++++++
 4 files changed, 101 insertions(+), 14 deletions(-)
Aurelien Jarno - March 17, 2013, 10:10 a.m.
On Fri, Mar 15, 2013 at 06:56:19PM +0100, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
> 
> Fix for rndrashift_short_acc to set correct value to higher 64 bits.
> This change also corrects conditions when bit 23 of the DSPControl register
> is set.
> 
> The existing test files have been extended with several examples that
> trigger the issues. One bug/example in the test file for EXTR_RS_W has been
> found and reported by Klaus Peichl.
> 
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
>  target-mips/dsp_helper.c              |   23 +++++++----------
>  tests/tcg/mips/mips32-dsp/extr_r_w.c  |   23 +++++++++++++++++
>  tests/tcg/mips/mips32-dsp/extr_rs_w.c |   46 +++++++++++++++++++++++++++++++++
>  tests/tcg/mips/mips32-dsp/extr_w.c    |   23 +++++++++++++++++
>  4 files changed, 101 insertions(+), 14 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 472be35..c7df595 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -517,13 +517,8 @@ static inline void mipsdsp_rndrashift_short_acc(int64_t *p,
>  
>      acc = ((int64_t)env->active_tc.HI[ac] << 32) |
>            ((int64_t)env->active_tc.LO[ac] & 0xFFFFFFFF);
> -    if (shift == 0) {
> -        p[0] = acc << 1;
> -        p[1] = (acc >> 63) & 0x01;
> -    } else {
> -        p[0] = acc >> (shift - 1);
> -        p[1] = 0;
> -    }
> +    p[0] = (shift == 0) ? (acc << 1) : (acc >> (shift - 1));
> +    p[1] = (acc >> 63) & 0x01;
>  }
>  
>  /* 128 bits long. p[0] is LO, p[1] is HI */
> @@ -3161,8 +3156,8 @@ target_ulong helper_extr_w(target_ulong ac, target_ulong shift,
>          tempDL[1] += 1;
>      }
>  
> -    if ((!(tempDL[1] == 0 && (tempDL[0] & MIPSDSP_LHI) == 0x00)) &&
> -        (!(tempDL[1] == 1 && (tempDL[0] & MIPSDSP_LHI) == MIPSDSP_LHI))) {
> +    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
> +        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
>          set_DSPControl_overflow_flag(1, 23, env);
>      }
>  
> @@ -3187,8 +3182,8 @@ target_ulong helper_extr_r_w(target_ulong ac, target_ulong shift,
>          tempDL[1] += 1;
>      }
>  
> -    if ((tempDL[1] != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
> -        (tempDL[1] != 1 && (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
> +    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
> +        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
>          set_DSPControl_overflow_flag(1, 23, env);
>      }
>  
> @@ -3214,9 +3209,9 @@ target_ulong helper_extr_rs_w(target_ulong ac, target_ulong shift,
>      }
>      tempI = tempDL[0] >> 1;
>  
> -    if ((tempDL[1] != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
> -        (tempDL[1] != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
> -        temp64 = tempDL[1];
> +    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
> +        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
> +        temp64 = tempDL[1] & 0x01;
>          if (temp64 == 0) {
>              tempI = 0x7FFFFFFF;
>          } else {
> diff --git a/tests/tcg/mips/mips32-dsp/extr_r_w.c b/tests/tcg/mips/mips32-dsp/extr_r_w.c
> index 02e0224..489c193 100644
> --- a/tests/tcg/mips/mips32-dsp/extr_r_w.c
> +++ b/tests/tcg/mips/mips32-dsp/extr_r_w.c
> @@ -67,5 +67,28 @@ int main()
>      assert(dsp == 0);
>      assert(result == rt);
>  
> +    /* Clear dspcontrol */
> +    dsp = 0;
> +    __asm
> +        ("wrdsp %0\n\t"
> +         :
> +         : "r"(dsp)
> +        );
> +
> +    ach = 0xFFFFFFFF;
> +    acl = 0xFFFFFFFF;
> +    result = 0;
> +    __asm
> +        ("mthi %2, $ac1\n\t"
> +         "mtlo %3, $ac1\n\t"
> +         "extr_r.w %0, $ac1, 0x1F\n\t"
> +         "rddsp %1\n\t"
> +         : "=r"(rt), "=r"(dsp)
> +         : "r"(ach), "r"(acl)
> +         );
> +    dsp = (dsp >> 23) & 0x01;
> +    assert(dsp == 0);
> +    assert(result == rt);
> +
>      return 0;
>  }
> diff --git a/tests/tcg/mips/mips32-dsp/extr_rs_w.c b/tests/tcg/mips/mips32-dsp/extr_rs_w.c
> index c3a22ee..f9d2ed6 100644
> --- a/tests/tcg/mips/mips32-dsp/extr_rs_w.c
> +++ b/tests/tcg/mips/mips32-dsp/extr_rs_w.c
> @@ -67,5 +67,51 @@ int main()
>      assert(dsp == 0);
>      assert(result == rt);
>  
> +    /* Clear dspcontrol */
> +    dsp = 0;
> +    __asm
> +        ("wrdsp %0\n\t"
> +         :
> +         : "r"(dsp)
> +        );
> +
> +    ach = 0x80000000;
> +    acl = 0x00000000;
> +    result = 0x80000000;
> +    __asm
> +        ("mthi %2, $ac1\n\t"
> +         "mtlo %3, $ac1\n\t"
> +         "extr_rs.w %0, $ac1, 0x1F\n\t"
> +         "rddsp %1\n\t"
> +         : "=r"(rt), "=r"(dsp)
> +         : "r"(ach), "r"(acl)
> +        );
> +    dsp = (dsp >> 23) & 0x01;
> +    assert(dsp == 1);
> +    assert(result == rt);
> +
> +    /* Clear dspcontrol */
> +    dsp = 0;
> +    __asm
> +        ("wrdsp %0\n\t"
> +         :
> +         : "r"(dsp)
> +        );
> +
> +    ach = 0xFFFFFFFF;
> +    acl = 0xFFFFFFFF;
> +    result = 0;
> +    __asm
> +        ("mthi %2, $ac1\n\t"
> +         "mtlo %3, $ac1\n\t"
> +         "extr_rs.w %0, $ac1, 0x1F\n\t"
> +         "rddsp %1\n\t"
> +         : "=r"(rt), "=r"(dsp)
> +         : "r"(ach), "r"(acl)
> +         );
> +    dsp = (dsp >> 23) & 0x01;
> +    assert(dsp == 0);
> +    assert(result == rt);
> +
>      return 0;
>  }
> diff --git a/tests/tcg/mips/mips32-dsp/extr_w.c b/tests/tcg/mips/mips32-dsp/extr_w.c
> index bd6b0b9..cf92614 100644
> --- a/tests/tcg/mips/mips32-dsp/extr_w.c
> +++ b/tests/tcg/mips/mips32-dsp/extr_w.c
> @@ -67,5 +67,28 @@ int main()
>      assert(dsp == 0);
>      assert(result == rt);
>  
> +    /* Clear dspcontrol */
> +    dsp = 0;
> +    __asm
> +        ("wrdsp %0\n\t"
> +         :
> +         : "r"(dsp)
> +        );
> +
> +    ach = 0xFFFFFFFF;
> +    acl = 0xFFFFFFFF;
> +    result = 0xFFFFFFFF;
> +    __asm
> +        ("mthi %2, $ac1\n\t"
> +         "mtlo %3, $ac1\n\t"
> +         "extr.w %0, $ac1, 0x1F\n\t"
> +         "rddsp %1\n\t"
> +         : "=r"(rt), "=r"(dsp)
> +         : "r"(ach), "r"(acl)
> +         );
> +    dsp = (dsp >> 23) & 0x01;
> +    assert(dsp == 0);
> +    assert(result == rt);
> +
>      return 0;
>  }

Thanks, applied.

Patch

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 472be35..c7df595 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -517,13 +517,8 @@  static inline void mipsdsp_rndrashift_short_acc(int64_t *p,
 
     acc = ((int64_t)env->active_tc.HI[ac] << 32) |
           ((int64_t)env->active_tc.LO[ac] & 0xFFFFFFFF);
-    if (shift == 0) {
-        p[0] = acc << 1;
-        p[1] = (acc >> 63) & 0x01;
-    } else {
-        p[0] = acc >> (shift - 1);
-        p[1] = 0;
-    }
+    p[0] = (shift == 0) ? (acc << 1) : (acc >> (shift - 1));
+    p[1] = (acc >> 63) & 0x01;
 }
 
 /* 128 bits long. p[0] is LO, p[1] is HI */
@@ -3161,8 +3156,8 @@  target_ulong helper_extr_w(target_ulong ac, target_ulong shift,
         tempDL[1] += 1;
     }
 
-    if ((!(tempDL[1] == 0 && (tempDL[0] & MIPSDSP_LHI) == 0x00)) &&
-        (!(tempDL[1] == 1 && (tempDL[0] & MIPSDSP_LHI) == MIPSDSP_LHI))) {
+    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
+        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
         set_DSPControl_overflow_flag(1, 23, env);
     }
 
@@ -3187,8 +3182,8 @@  target_ulong helper_extr_r_w(target_ulong ac, target_ulong shift,
         tempDL[1] += 1;
     }
 
-    if ((tempDL[1] != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
-        (tempDL[1] != 1 && (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
+    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
+        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
         set_DSPControl_overflow_flag(1, 23, env);
     }
 
@@ -3214,9 +3209,9 @@  target_ulong helper_extr_rs_w(target_ulong ac, target_ulong shift,
     }
     tempI = tempDL[0] >> 1;
 
-    if ((tempDL[1] != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
-        (tempDL[1] != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
-        temp64 = tempDL[1];
+    if (((tempDL[1] & 0x01) != 0 || (tempDL[0] & MIPSDSP_LHI) != 0) &&
+        ((tempDL[1] & 0x01) != 1 || (tempDL[0] & MIPSDSP_LHI) != MIPSDSP_LHI)) {
+        temp64 = tempDL[1] & 0x01;
         if (temp64 == 0) {
             tempI = 0x7FFFFFFF;
         } else {
diff --git a/tests/tcg/mips/mips32-dsp/extr_r_w.c b/tests/tcg/mips/mips32-dsp/extr_r_w.c
index 02e0224..489c193 100644
--- a/tests/tcg/mips/mips32-dsp/extr_r_w.c
+++ b/tests/tcg/mips/mips32-dsp/extr_r_w.c
@@ -67,5 +67,28 @@  int main()
     assert(dsp == 0);
     assert(result == rt);
 
+    /* Clear dspcontrol */
+    dsp = 0;
+    __asm
+        ("wrdsp %0\n\t"
+         :
+         : "r"(dsp)
+        );
+
+    ach = 0xFFFFFFFF;
+    acl = 0xFFFFFFFF;
+    result = 0;
+    __asm
+        ("mthi %2, $ac1\n\t"
+         "mtlo %3, $ac1\n\t"
+         "extr_r.w %0, $ac1, 0x1F\n\t"
+         "rddsp %1\n\t"
+         : "=r"(rt), "=r"(dsp)
+         : "r"(ach), "r"(acl)
+         );
+    dsp = (dsp >> 23) & 0x01;
+    assert(dsp == 0);
+    assert(result == rt);
+
     return 0;
 }
diff --git a/tests/tcg/mips/mips32-dsp/extr_rs_w.c b/tests/tcg/mips/mips32-dsp/extr_rs_w.c
index c3a22ee..f9d2ed6 100644
--- a/tests/tcg/mips/mips32-dsp/extr_rs_w.c
+++ b/tests/tcg/mips/mips32-dsp/extr_rs_w.c
@@ -67,5 +67,51 @@  int main()
     assert(dsp == 0);
     assert(result == rt);
 
+    /* Clear dspcontrol */
+    dsp = 0;
+    __asm
+        ("wrdsp %0\n\t"
+         :
+         : "r"(dsp)
+        );
+
+    ach = 0x80000000;
+    acl = 0x00000000;
+    result = 0x80000000;
+    __asm
+        ("mthi %2, $ac1\n\t"
+         "mtlo %3, $ac1\n\t"
+         "extr_rs.w %0, $ac1, 0x1F\n\t"
+         "rddsp %1\n\t"
+         : "=r"(rt), "=r"(dsp)
+         : "r"(ach), "r"(acl)
+        );
+    dsp = (dsp >> 23) & 0x01;
+    assert(dsp == 1);
+    assert(result == rt);
+
+    /* Clear dspcontrol */
+    dsp = 0;
+    __asm
+        ("wrdsp %0\n\t"
+         :
+         : "r"(dsp)
+        );
+
+    ach = 0xFFFFFFFF;
+    acl = 0xFFFFFFFF;
+    result = 0;
+    __asm
+        ("mthi %2, $ac1\n\t"
+         "mtlo %3, $ac1\n\t"
+         "extr_rs.w %0, $ac1, 0x1F\n\t"
+         "rddsp %1\n\t"
+         : "=r"(rt), "=r"(dsp)
+         : "r"(ach), "r"(acl)
+         );
+    dsp = (dsp >> 23) & 0x01;
+    assert(dsp == 0);
+    assert(result == rt);
+
     return 0;
 }
diff --git a/tests/tcg/mips/mips32-dsp/extr_w.c b/tests/tcg/mips/mips32-dsp/extr_w.c
index bd6b0b9..cf92614 100644
--- a/tests/tcg/mips/mips32-dsp/extr_w.c
+++ b/tests/tcg/mips/mips32-dsp/extr_w.c
@@ -67,5 +67,28 @@  int main()
     assert(dsp == 0);
     assert(result == rt);
 
+    /* Clear dspcontrol */
+    dsp = 0;
+    __asm
+        ("wrdsp %0\n\t"
+         :
+         : "r"(dsp)
+        );
+
+    ach = 0xFFFFFFFF;
+    acl = 0xFFFFFFFF;
+    result = 0xFFFFFFFF;
+    __asm
+        ("mthi %2, $ac1\n\t"
+         "mtlo %3, $ac1\n\t"
+         "extr.w %0, $ac1, 0x1F\n\t"
+         "rddsp %1\n\t"
+         : "=r"(rt), "=r"(dsp)
+         : "r"(ach), "r"(acl)
+         );
+    dsp = (dsp >> 23) & 0x01;
+    assert(dsp == 0);
+    assert(result == rt);
+
     return 0;
 }