Patchwork target-mips: Fix helper and tests for dot/cross-dot product instructions

login
register
mail settings
Submitter Petar Jovanovic
Date Jan. 2, 2013, 4:08 a.m.
Message ID <1357099728-4916-1-git-send-email-petar.jovanovic@rt-rk.com>
Download mbox | patch
Permalink /patch/208968/
State New
Headers show

Comments

Petar Jovanovic - Jan. 2, 2013, 4:08 a.m.
From: Petar Jovanovic <petarj@mips.com>

Helper function for dpa_w_ph, dpax_w_ph, dps_w_ph and dpsx_w_ph incorrectly
defines halfword vector elements as unsigned values. This results in wrong
output which is not triggered in the tests as they also follow this logic.

Signed-off-by: Petar Jovanovic <petarj@mips.com>
---
 target-mips/dsp_helper.c                |    2 +-
 tests/tcg/mips/mips32-dspr2/dpa_w_ph.c  |    4 ++--
 tests/tcg/mips/mips32-dspr2/dpax_w_ph.c |   17 +++++++++++++++++
 tests/tcg/mips/mips32-dspr2/dps_w_ph.c  |   17 +++++++++++++++++
 tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c |    4 ++--
 5 files changed, 39 insertions(+), 5 deletions(-)
Eric Johnson - Jan. 8, 2013, 5:43 a.m.
On 01/01/2013 08:08 PM, Petar Jovanovic wrote:
> From: Petar Jovanovic<petarj@mips.com>
>
> Helper function for dpa_w_ph, dpax_w_ph, dps_w_ph and dpsx_w_ph incorrectly
> defines halfword vector elements as unsigned values. This results in wrong
> output which is not triggered in the tests as they also follow this logic.
>
> Signed-off-by: Petar Jovanovic<petarj@mips.com>
> ---
>   target-mips/dsp_helper.c                |    2 +-
>   tests/tcg/mips/mips32-dspr2/dpa_w_ph.c  |    4 ++--
>   tests/tcg/mips/mips32-dspr2/dpax_w_ph.c |   17 +++++++++++++++++
>   tests/tcg/mips/mips32-dspr2/dps_w_ph.c  |   17 +++++++++++++++++
>   tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c |    4 ++--
>   5 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index a33e2bf..4870e3d 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -2473,7 +2473,7 @@ DP_OB(dpsu_h_obr, 0, 24, 16, 8, 0, 24, 16, 8, 0);
>   void helper_##name(uint32_t ac, target_ulong rs, target_ulong rt,              \
>                      CPUMIPSState *env)                                          \
>   {                                                                              \
> -    uint16_t rsB, rsA, rtB, rtA;                                               \
> +    int16_t rsB, rsA, rtB, rtA;                                                \
>       int32_t  tempA, tempB;                                                     \
>       int64_t  acc;                                                              \
>                                                                                  \
> diff --git a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> index 1cfbdb0..fae49f1 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> @@ -26,8 +26,8 @@ int main()
>       ach = 6, acl = 7;
>       rs     = 0xFFFF00FF;
>       rt     = 0xFFFF0002;
> -    resulth = 0x05;
> -    resultl = 0xfffe0206;
> +    resulth = 0x06;
> +    resultl = 0x206;
>       __asm
>           ("mthi  %0, $ac1\n\t"
>            "mtlo  %1, $ac1\n\t"
> diff --git a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> index f756997..514797c 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> @@ -23,5 +23,22 @@ int main()
>       assert(ach == resulth);
>       assert(acl == resultl);
>
> +    ach = 6, acl = 7;
> +    rs     = 0xFFFF00FF;
> +    rt     = 0xFFFF0002;
> +    resulth = 0x05;
> +    resultl = 0xFFFFFF06;
> +    __asm
> +        ("mthi  %0, $ac1\n\t"
> +         "mtlo  %1, $ac1\n\t"
> +         "dpax.w.ph $ac1, %2, %3\n\t"
> +         "mfhi  %0, $ac1\n\t"
> +         "mflo  %1, $ac1\n\t"
> +         : "+r"(ach), "+r"(acl)
> +         : "r"(rs), "r"(rt)
> +        );
> +    assert(ach == resulth);
> +    assert(acl == resultl);
> +
>       return 0;
>   }
> diff --git a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> index 8303643..f51f9b9 100644
> --- a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> @@ -23,5 +23,22 @@ int main()
>       assert(ach == resulth);
>       assert(acl == resultl);
>
> +    ach = 6, acl = 7;
> +    rs     = 0xFFFF00FF;
> +    rt     = 0xFFFF0002;
> +    resulth = 0x05;
> +    resultl = 0xFFFFFE08;
> +    __asm
> +        ("mthi  %0, $ac1\n\t"
> +         "mtlo  %1, $ac1\n\t"
> +         "dps.w.ph $ac1, %2, %3\n\t"
> +         "mfhi  %0, $ac1\n\t"
> +         "mflo  %1, $ac1\n\t"
> +         : "+r"(ach), "+r"(acl)
> +         : "r"(rs), "r"(rt)
> +        );
> +    assert(ach == resulth);
> +    assert(acl == resultl);
> +
>       return 0;
>   }
> diff --git a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> index 6db59a4..bb49a40 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> @@ -9,8 +9,8 @@ int main()
>
>       rs      = 0xBC0123AD;
>       rt      = 0x01643721;
> -    resulth = 0x04;
> -    resultl = 0xD751F050;
> +    resulth = 0x05;
> +    resultl = 0xE72F050;
>       __asm
>           ("mthi  %0, $ac1\n\t"
>            "mtlo  %1, $ac1\n\t"

Reviewed-by: Eric Johnson <ericj@mips.com>
Aurelien Jarno - Jan. 8, 2013, 3:45 p.m.
On Wed, Jan 02, 2013 at 05:08:48AM +0100, Petar Jovanovic wrote:
> From: Petar Jovanovic <petarj@mips.com>
> 
> Helper function for dpa_w_ph, dpax_w_ph, dps_w_ph and dpsx_w_ph incorrectly
> defines halfword vector elements as unsigned values. This results in wrong
> output which is not triggered in the tests as they also follow this logic.
> 
> Signed-off-by: Petar Jovanovic <petarj@mips.com>
> ---
>  target-mips/dsp_helper.c                |    2 +-
>  tests/tcg/mips/mips32-dspr2/dpa_w_ph.c  |    4 ++--
>  tests/tcg/mips/mips32-dspr2/dpax_w_ph.c |   17 +++++++++++++++++
>  tests/tcg/mips/mips32-dspr2/dps_w_ph.c  |   17 +++++++++++++++++
>  tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c |    4 ++--
>  5 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index a33e2bf..4870e3d 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -2473,7 +2473,7 @@ DP_OB(dpsu_h_obr, 0, 24, 16, 8, 0, 24, 16, 8, 0);
>  void helper_##name(uint32_t ac, target_ulong rs, target_ulong rt,              \
>                     CPUMIPSState *env)                                          \
>  {                                                                              \
> -    uint16_t rsB, rsA, rtB, rtA;                                               \
> +    int16_t rsB, rsA, rtB, rtA;                                                \
>      int32_t  tempA, tempB;                                                     \
>      int64_t  acc;                                                              \
>                                                                                 \
> diff --git a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> index 1cfbdb0..fae49f1 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
> @@ -26,8 +26,8 @@ int main()
>      ach = 6, acl = 7;
>      rs     = 0xFFFF00FF;
>      rt     = 0xFFFF0002;
> -    resulth = 0x05;
> -    resultl = 0xfffe0206;
> +    resulth = 0x06;
> +    resultl = 0x206;
>      __asm
>          ("mthi  %0, $ac1\n\t"
>           "mtlo  %1, $ac1\n\t"
> diff --git a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> index f756997..514797c 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
> @@ -23,5 +23,22 @@ int main()
>      assert(ach == resulth);
>      assert(acl == resultl);
>  
> +    ach = 6, acl = 7;
> +    rs     = 0xFFFF00FF;
> +    rt     = 0xFFFF0002;
> +    resulth = 0x05;
> +    resultl = 0xFFFFFF06;
> +    __asm
> +        ("mthi  %0, $ac1\n\t"
> +         "mtlo  %1, $ac1\n\t"
> +         "dpax.w.ph $ac1, %2, %3\n\t"
> +         "mfhi  %0, $ac1\n\t"
> +         "mflo  %1, $ac1\n\t"
> +         : "+r"(ach), "+r"(acl)
> +         : "r"(rs), "r"(rt)
> +        );
> +    assert(ach == resulth);
> +    assert(acl == resultl);
> +
>      return 0;
>  }
> diff --git a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> index 8303643..f51f9b9 100644
> --- a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
> @@ -23,5 +23,22 @@ int main()
>      assert(ach == resulth);
>      assert(acl == resultl);
>  
> +    ach = 6, acl = 7;
> +    rs     = 0xFFFF00FF;
> +    rt     = 0xFFFF0002;
> +    resulth = 0x05;
> +    resultl = 0xFFFFFE08;
> +    __asm
> +        ("mthi  %0, $ac1\n\t"
> +         "mtlo  %1, $ac1\n\t"
> +         "dps.w.ph $ac1, %2, %3\n\t"
> +         "mfhi  %0, $ac1\n\t"
> +         "mflo  %1, $ac1\n\t"
> +         : "+r"(ach), "+r"(acl)
> +         : "r"(rs), "r"(rt)
> +        );
> +    assert(ach == resulth);
> +    assert(acl == resultl);
> +
>      return 0;
>  }
> diff --git a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> index 6db59a4..bb49a40 100644
> --- a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> +++ b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
> @@ -9,8 +9,8 @@ int main()
>  
>      rs      = 0xBC0123AD;
>      rt      = 0x01643721;
> -    resulth = 0x04;
> -    resultl = 0xD751F050;
> +    resulth = 0x05;
> +    resultl = 0xE72F050;
>      __asm
>          ("mthi  %0, $ac1\n\t"
>           "mtlo  %1, $ac1\n\t"

Thanks, applied.

Patch

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index a33e2bf..4870e3d 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -2473,7 +2473,7 @@  DP_OB(dpsu_h_obr, 0, 24, 16, 8, 0, 24, 16, 8, 0);
 void helper_##name(uint32_t ac, target_ulong rs, target_ulong rt,              \
                    CPUMIPSState *env)                                          \
 {                                                                              \
-    uint16_t rsB, rsA, rtB, rtA;                                               \
+    int16_t rsB, rsA, rtB, rtA;                                                \
     int32_t  tempA, tempB;                                                     \
     int64_t  acc;                                                              \
                                                                                \
diff --git a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
index 1cfbdb0..fae49f1 100644
--- a/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dpa_w_ph.c
@@ -26,8 +26,8 @@  int main()
     ach = 6, acl = 7;
     rs     = 0xFFFF00FF;
     rt     = 0xFFFF0002;
-    resulth = 0x05;
-    resultl = 0xfffe0206;
+    resulth = 0x06;
+    resultl = 0x206;
     __asm
         ("mthi  %0, $ac1\n\t"
          "mtlo  %1, $ac1\n\t"
diff --git a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
index f756997..514797c 100644
--- a/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dpax_w_ph.c
@@ -23,5 +23,22 @@  int main()
     assert(ach == resulth);
     assert(acl == resultl);
 
+    ach = 6, acl = 7;
+    rs     = 0xFFFF00FF;
+    rt     = 0xFFFF0002;
+    resulth = 0x05;
+    resultl = 0xFFFFFF06;
+    __asm
+        ("mthi  %0, $ac1\n\t"
+         "mtlo  %1, $ac1\n\t"
+         "dpax.w.ph $ac1, %2, %3\n\t"
+         "mfhi  %0, $ac1\n\t"
+         "mflo  %1, $ac1\n\t"
+         : "+r"(ach), "+r"(acl)
+         : "r"(rs), "r"(rt)
+        );
+    assert(ach == resulth);
+    assert(acl == resultl);
+
     return 0;
 }
diff --git a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
index 8303643..f51f9b9 100644
--- a/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dps_w_ph.c
@@ -23,5 +23,22 @@  int main()
     assert(ach == resulth);
     assert(acl == resultl);
 
+    ach = 6, acl = 7;
+    rs     = 0xFFFF00FF;
+    rt     = 0xFFFF0002;
+    resulth = 0x05;
+    resultl = 0xFFFFFE08;
+    __asm
+        ("mthi  %0, $ac1\n\t"
+         "mtlo  %1, $ac1\n\t"
+         "dps.w.ph $ac1, %2, %3\n\t"
+         "mfhi  %0, $ac1\n\t"
+         "mflo  %1, $ac1\n\t"
+         : "+r"(ach), "+r"(acl)
+         : "r"(rs), "r"(rt)
+        );
+    assert(ach == resulth);
+    assert(acl == resultl);
+
     return 0;
 }
diff --git a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
index 6db59a4..bb49a40 100644
--- a/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dpsx_w_ph.c
@@ -9,8 +9,8 @@  int main()
 
     rs      = 0xBC0123AD;
     rt      = 0x01643721;
-    resulth = 0x04;
-    resultl = 0xD751F050;
+    resulth = 0x05;
+    resultl = 0xE72F050;
     __asm
         ("mthi  %0, $ac1\n\t"
          "mtlo  %1, $ac1\n\t"