[Qemu-devel] [PATCH 0/5] ARM: fix VQSHL (register)

[Qemu-devel] [PATCH 0/5] ARM: fix VQSHL (register)

[Peter Maydell]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

This patchset fixes bugs in the decode and implementation
of the ARM Neon VQSHL (register) instruction form. It is
a split out version of those parts of the maemo-qemu tree
commit 03a2445a which deal with this instruction form;
the remainder which address VQSHL (immediate) I'll send
in a separate patchset.

The 'Fix VQSHL of signed 64 bit values by shift counts >= 64'
patch is a bug fix which does not appear in the maemo-qemu
patch. 'Correct result in saturating cases for VQSHL of s8/16/32'
does appear, but I refactored it a little to be clearer;
these two therefore have me as the author.

These patches have been tested by running random instruction
sequences and comparing against A8 hardware.


Juha Riihimäki (3):
  ARM: Fix arguments passed to VQSHL helpers
  ARM: Fix VQSHL of signed 64 bit values
  ARM: remove pointless else clause in VQSHL of u64

Peter Maydell (2):
  ARM: Fix VQSHL of signed 64 bit values by shift counts >= 64
  ARM: Correct result in saturating cases for VQSHL of s8/16/32

 target-arm/neon_helper.c |   21 ++++++++++++++-------
 target-arm/translate.c   |    4 ++--
 2 files changed, 16 insertions(+), 9 deletions(-)

[Qemu-devel] [PATCH 1/5] ARM: Fix arguments passed to VQSHL helpers

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Correct the arguments passed when generating neon qshl_{u,s}64()
helpers so that we use the correct registers.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 99464ab..b5af1c6 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4235,9 +4235,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                 case 9: /* VQSHL */
                     if (u) {
                         gen_helper_neon_qshl_u64(cpu_V0, cpu_env,
-                                                 cpu_V0, cpu_V0);
+                                                 cpu_V1, cpu_V0);
                     } else {
-                        gen_helper_neon_qshl_s64(cpu_V1, cpu_env,
+                        gen_helper_neon_qshl_s64(cpu_V0, cpu_env,
                                                  cpu_V1, cpu_V0);
                     }
                     break;
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/5] ARM: Fix VQSHL of signed 64 bit values

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Add a missing '-' which meant that we were misinterpreting the shift
argument for VQSHL of 64 bit signed values and treating almost every
shift value as if it were an extremely large right shift.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/neon_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 5e6452b..d29b884 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -610,7 +610,7 @@ uint64_t HELPER(neon_qshl_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
             SET_QC();
             val = (val >> 63) & ~SIGNBIT64;
         }
-    } else if (shift <= 64) {
+    } else if (shift <= -64) {
         val >>= 63;
     } else if (shift < 0) {
         val >>= -shift;
-- 
1.6.3.3

[Qemu-devel] [PATCH 3/5] ARM: Fix VQSHL of signed 64 bit values by shift counts >= 64

[Peter Maydell]

VQSHL of a signed 64 bit non-zero value by a shift count >= 64 should
saturate; return the correct value in this case.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/neon_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index d29b884..2dc3d96 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -608,7 +608,7 @@ uint64_t HELPER(neon_qshl_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
     if (shift >= 64) {
         if (val) {
             SET_QC();
-            val = (val >> 63) & ~SIGNBIT64;
+            val = (val >> 63) ^ ~SIGNBIT64;
         }
     } else if (shift <= -64) {
         val >>= 63;
-- 
1.6.3.3

[Qemu-devel] [PATCH 4/5] ARM: remove pointless else clause in VQSHL of u64

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Remove a pointless else clause in the neon_qshl_u64 helper.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/neon_helper.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 2dc3d96..48b9f5b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -560,8 +560,6 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
         if (val) {
             val = ~(uint64_t)0;
             SET_QC();
-        } else {
-            val = 0;
         }
     } else if (shift <= -64) {
         val = 0;
-- 
1.6.3.3

[Qemu-devel] [PATCH 5/5] ARM: Correct result in saturating cases for VQSHL of s8/16/32

[Peter Maydell]

Where VQSHL of a signed 8/16/32 bit value saturated, the result
value was not being calculated correctly (it should be either
the minimum or maximum value for the size of the signed type).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/neon_helper.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 48b9f5b..dae063e 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -580,9 +580,15 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
     int8_t tmp; \
     tmp = (int8_t)src2; \
     if (tmp >= (ssize_t)sizeof(src1) * 8) { \
-        if (src1) \
+        if (src1) { \
             SET_QC(); \
-        dest = src1 >> 31; \
+            dest = (uint32_t)(1 << (sizeof(src1) * 8 - 1)); \
+            if (src1 > 0) { \
+                dest--; \
+            } \
+        } else { \
+            dest = src1; \
+        } \
     } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> 31; \
     } else if (tmp < 0) { \
@@ -591,7 +597,10 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
         dest = src1 << tmp; \
         if ((dest >> tmp) != src1) { \
             SET_QC(); \
-            dest = src2 >> 31; \
+            dest = (uint32_t)(1 << (sizeof(src1) * 8 - 1)); \
+            if (src1 > 0) { \
+                dest--; \
+            } \
         } \
     }} while (0)
 NEON_VOP_ENV(qshl_s8, neon_s8, 4)
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 0/5] ARM: fix VQSHL (register)

[Aurelien Jarno]

On Tue, Dec 07, 2010 at 02:13:40PM +0000, Peter Maydell wrote:
> This patchset fixes bugs in the decode and implementation
> of the ARM Neon VQSHL (register) instruction form. It is
> a split out version of those parts of the maemo-qemu tree
> commit 03a2445a which deal with this instruction form;
> the remainder which address VQSHL (immediate) I'll send
> in a separate patchset.
> 
> The 'Fix VQSHL of signed 64 bit values by shift counts >= 64'
> patch is a bug fix which does not appear in the maemo-qemu
> patch. 'Correct result in saturating cases for VQSHL of s8/16/32'
> does appear, but I refactored it a little to be clearer;
> these two therefore have me as the author.
> 
> These patches have been tested by running random instruction
> sequences and comparing against A8 hardware.
> 

Thanks, applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net