* [Qemu-devel] [PATCH 01/10] target-arm: Fix rounding constant addition for Neon shifts
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 02/10] target-arm: Fix signed VRSHL by large shift counts Peter Maydell
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
From: Christophe Lyon <christophe.lyon@st.com>
Handle cases where adding the rounding constant could overflow in Neon
shift instructions: VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN.
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
[peter.maydell@linaro.org: fix handling of large shifts in rshl_s32,
calculate signed saturated value as other functions do.]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/neon_helper.c | 139 ++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 127 insertions(+), 12 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index dc09968..cc63636 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -558,9 +558,28 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
}} while (0)
NEON_VOP(rshl_s8, neon_s8, 4)
NEON_VOP(rshl_s16, neon_s16, 2)
-NEON_VOP(rshl_s32, neon_s32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
+{
+ int32_t dest;
+ int32_t val = (int32_t)valop;
+ int8_t shift = (int8_t)shiftop;
+ if ((shift >= 32) || (shift <= -32)) {
+ dest = 0;
+ } else if (shift < 0) {
+ int64_t big_dest = ((int64_t)val + (1 << (-1 - shift)));
+ dest = big_dest >> -shift;
+ } else {
+ dest = val << shift;
+ }
+ return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -574,7 +593,16 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
val++;
val >>= 1;
} else if (shift < 0) {
- val = (val + ((int64_t)1 << (-1 - shift))) >> -shift;
+ val >>= (-shift - 1);
+ if (val == INT64_MAX) {
+ /* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+ val = 0x4000000000000000LL;
+ } else {
+ val++;
+ val >>= 1;
+ }
} else {
val <<= shift;
}
@@ -596,9 +624,29 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
}} while (0)
NEON_VOP(rshl_u8, neon_u8, 4)
NEON_VOP(rshl_u16, neon_u16, 2)
-NEON_VOP(rshl_u32, neon_u32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
+{
+ uint32_t dest;
+ int8_t shift = (int8_t)shiftop;
+ if (shift >= 32 || shift < -32) {
+ dest = 0;
+ } else if (shift == -32) {
+ dest = val >> 31;
+ } else if (shift < 0) {
+ uint64_t big_dest = ((uint64_t)val + (1 << (-1 - shift)));
+ dest = big_dest >> -shift;
+ } else {
+ dest = val << shift;
+ }
+ return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
@@ -607,9 +655,17 @@ uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
} else if (shift == -64) {
/* Rounding a 1-bit result just preserves that bit. */
val >>= 63;
- } if (shift < 0) {
- val = (val + ((uint64_t)1 << (-1 - shift))) >> -shift;
- val >>= -shift;
+ } else if (shift < 0) {
+ val >>= (-shift - 1);
+ if (val == UINT64_MAX) {
+ /* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+ val = 0x8000000000000000ULL;
+ } else {
+ val++;
+ val >>= 1;
+ }
} else {
val <<= shift;
}
@@ -784,14 +840,43 @@ uint64_t HELPER(neon_qshlu_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
}} while (0)
NEON_VOP_ENV(qrshl_u8, neon_u8, 4)
NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
-NEON_VOP_ENV(qrshl_u32, neon_u32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t val, uint32_t shiftop)
+{
+ uint32_t dest;
+ int8_t shift = (int8_t)shiftop;
+ if (shift < 0) {
+ uint64_t big_dest = ((uint64_t)val + (1 << (-1 - shift)));
+ dest = big_dest >> -shift;
+ } else {
+ dest = val << shift;
+ if ((dest >> shift) != val) {
+ SET_QC();
+ dest = ~0;
+ }
+ }
+ return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
if (shift < 0) {
- val = (val + (1 << (-1 - shift))) >> -shift;
+ val >>= (-shift - 1);
+ if (val == UINT64_MAX) {
+ /* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+ val = 0x8000000000000000ULL;
+ } else {
+ val++;
+ val >>= 1;
+ }
} else { \
uint64_t tmp = val;
val <<= shift;
@@ -817,22 +902,52 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
}} while (0)
NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
NEON_VOP_ENV(qrshl_s16, neon_s16, 2)
-NEON_VOP_ENV(qrshl_s32, neon_s32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t valop, uint32_t shiftop)
+{
+ int32_t dest;
+ int32_t val = (int32_t)valop;
+ int8_t shift = (int8_t)shiftop;
+ if (shift < 0) {
+ int64_t big_dest = ((int64_t)val + (1 << (-1 - shift)));
+ dest = big_dest >> -shift;
+ } else {
+ dest = val << shift;
+ if ((dest >> shift) != val) {
+ SET_QC();
+ dest = (val >> 31) ^ ~SIGNBIT;
+ }
+ }
+ return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_qrshl_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
int64_t val = valop;
if (shift < 0) {
- val = (val + (1 << (-1 - shift))) >> -shift;
+ val >>= (-shift - 1);
+ if (val == INT64_MAX) {
+ /* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+ val = 0x4000000000000000ULL;
+ } else {
+ val++;
+ val >>= 1;
+ }
} else {
- int64_t tmp = val;;
+ int64_t tmp = val;
val <<= shift;
if ((val >> shift) != tmp) {
SET_QC();
- val = tmp >> 31;
+ val = (tmp >> 63) ^ ~SIGNBIT64;
}
}
return val;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 02/10] target-arm: Fix signed VRSHL by large shift counts
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 01/10] target-arm: Fix rounding constant addition for Neon shifts Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 03/10] target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width Peter Maydell
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
Correctly handle VRSHL of signed values by a shift count of the
width of the data type or larger, which must be special-cased in the
rshl_s* helper functions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/neon_helper.c | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index cc63636..f692640 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -543,14 +543,9 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
- if (tmp >= (ssize_t)sizeof(src1) * 8) { \
+ if ((tmp >= (ssize_t)sizeof(src1) * 8) \
+ || (tmp <= -(ssize_t)sizeof(src1) * 8)) { \
dest = 0; \
- } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
- dest = src1 >> (sizeof(src1) * 8 - 1); \
- } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
- dest = src1 >> (tmp - 1); \
- dest++; \
- dest >>= 1; \
} else if (tmp < 0) { \
dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
} else { \
@@ -584,14 +579,8 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
int64_t val = valop;
- if (shift >= 64) {
+ if ((shift >= 64) || (shift <= -64)) {
val = 0;
- } else if (shift < -64) {
- val >>= 63;
- } else if (shift == -63) {
- val >>= 63;
- val++;
- val >>= 1;
} else if (shift < 0) {
val >>= (-shift - 1);
if (val == INT64_MAX) {
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 03/10] target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 01/10] target-arm: Fix rounding constant addition for Neon shifts Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 02/10] target-arm: Fix signed VRSHL by large shift counts Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 04/10] target-arm: fix unsigned 64 bit right shifts Peter Maydell
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
From: Christophe Lyon <christophe.lyon@st.com>
Fix handling of unsigned VRSHL.s8 and .s16 right shifts by the type
width.
Signed-off-by: Christophe Lyon <christophe.lyon@st.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 f692640..2930b5e 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -605,7 +605,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
tmp < -(ssize_t)sizeof(src1) * 8) { \
dest = 0; \
} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
- dest = src1 >> (tmp - 1); \
+ dest = src1 >> (-tmp - 1); \
} else if (tmp < 0) { \
dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
} else { \
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 04/10] target-arm: fix unsigned 64 bit right shifts.
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (2 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 03/10] target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 05/10] target-arm: Fix saturated values for Neon " Peter Maydell
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
From: Christophe Lyon <christophe.lyon@st.com>
Fix range of shift amounts which always give 0 as result.
Signed-off-by: Christophe Lyon <christophe.lyon@st.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 2930b5e..a8885fa 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -639,7 +639,7 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
- if (shift >= 64 || shift < 64) {
+ if (shift >= 64 || shift < -64) {
val = 0;
} else if (shift == -64) {
/* Rounding a 1-bit result just preserves that bit. */
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 05/10] target-arm: Fix saturated values for Neon right shifts
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (3 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 04/10] target-arm: fix unsigned 64 bit right shifts Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 06/10] target-arm: fix Neon VQSHRN and VSHRN Peter Maydell
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
Fix value returned by signed 8 and 16 bit qrshl helpers
when the result has saturated.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/neon_helper.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index a8885fa..7859f0b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -886,7 +886,10 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
dest = src1 << tmp; \
if ((dest >> tmp) != src1) { \
SET_QC(); \
- dest = src1 >> 31; \
+ dest = (uint32_t)(1 << (sizeof(src1) * 8 - 1)); \
+ if (src1 > 0) { \
+ dest--; \
+ } \
} \
}} while (0)
NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 06/10] target-arm: fix Neon VQSHRN and VSHRN.
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (4 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 05/10] target-arm: Fix saturated values for Neon " Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 07/10] target-arm: fix decoding of Neon 64 bit shifts Peter Maydell
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
From: Christophe Lyon <christophe.lyon@st.com>
Call the normal shift helpers instead of the rounding ones.
Signed-off-by: Christophe Lyon <christophe.lyon@st.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 362d1d0..bd26b1b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4095,8 +4095,8 @@ static inline void gen_neon_shift_narrow(int size, TCGv var, TCGv shift,
} else {
if (u) {
switch (size) {
- case 1: gen_helper_neon_rshl_u16(var, var, shift); break;
- case 2: gen_helper_neon_rshl_u32(var, var, shift); break;
+ case 1: gen_helper_neon_shl_u16(var, var, shift); break;
+ case 2: gen_helper_neon_shl_u32(var, var, shift); break;
default: abort();
}
} else {
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 07/10] target-arm: fix decoding of Neon 64 bit shifts.
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (5 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 06/10] target-arm: fix Neon VQSHRN and VSHRN Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 08/10] target-arm: Fix signed VQRSHL by large shift counts Peter Maydell
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
From: Christophe Lyon <christophe.lyon@st.com>
Fix decoding of 64 bits variants of VSHRN, VRSHRN, VQSHRN, VQSHRUN,
VQRSHRN, VQRSHRUN, taking into account whether inputs are unsigned
or not.
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 45 ++++++++++++++++++++++++++++++---------------
1 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index bd26b1b..a02b20f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4815,6 +4815,8 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
} else if (op < 10) {
/* Shift by immediate and narrow:
VSHRN, VRSHRN, VQSHRN, VQRSHRN. */
+ int input_unsigned = (op == 8) ? !u : u;
+
shift = shift - (1 << (size + 3));
size++;
switch (size) {
@@ -4841,33 +4843,46 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
if (size == 3) {
neon_load_reg64(cpu_V0, rm + pass);
if (q) {
- if (u)
- gen_helper_neon_rshl_u64(cpu_V0, cpu_V0, tmp64);
- else
- gen_helper_neon_rshl_s64(cpu_V0, cpu_V0, tmp64);
+ if (input_unsigned) {
+ gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
+ tmp64);
+ } else {
+ gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
+ tmp64);
+ }
} else {
- if (u)
- gen_helper_neon_shl_u64(cpu_V0, cpu_V0, tmp64);
- else
- gen_helper_neon_shl_s64(cpu_V0, cpu_V0, tmp64);
+ if (input_unsigned) {
+ gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
+ tmp64);
+ } else {
+ gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
+ tmp64);
+ }
}
} else {
tmp = neon_load_reg(rm + pass, 0);
- gen_neon_shift_narrow(size, tmp, tmp2, q, u);
+ gen_neon_shift_narrow(size, tmp, tmp2, q,
+ input_unsigned);
tmp3 = neon_load_reg(rm + pass, 1);
- gen_neon_shift_narrow(size, tmp3, tmp2, q, u);
+ gen_neon_shift_narrow(size, tmp3, tmp2, q,
+ input_unsigned);
tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
dead_tmp(tmp);
dead_tmp(tmp3);
}
tmp = new_tmp();
- if (op == 8 && !u) {
- gen_neon_narrow(size - 1, tmp, cpu_V0);
+ if (op == 8) {
+ if (u) { /* VQSHRUN / VQRSHRUN */
+ gen_neon_unarrow_sats(size - 1, tmp, cpu_V0);
+ } else { /* VSHRN / VRSHRN */
+ gen_neon_narrow(size - 1, tmp, cpu_V0);
+ }
} else {
- if (op == 8)
- gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
- else
+ if (u) { /* VQSHRN / VQRSHRN */
gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
+ } else { /* VQSHRN / VQRSHRN */
+ gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
+ }
}
neon_store_reg(rd, pass, tmp);
} /* for pass */
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 08/10] target-arm: Fix signed VQRSHL by large shift counts
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (6 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 07/10] target-arm: fix decoding of Neon 64 bit shifts Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 09/10] target-arm: Fix unsigned " Peter Maydell
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
Handle the case of signed VQRSHL by a shift count of the width of the
data type or larger, which must be special cased in the qrshl_s*
helper functions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/neon_helper.c | 34 +++++++++++++++++++++++++++++++---
1 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 7859f0b..f8f6db6 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -880,7 +880,19 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
- if (tmp < 0) { \
+ if (tmp >= (ssize_t)sizeof(src1) * 8) { \
+ if (src1) { \
+ SET_QC(); \
+ dest = (1 << (sizeof(src1) * 8 - 1)); \
+ if (src1 > 0) { \
+ dest--; \
+ } \
+ } else { \
+ dest = 0; \
+ } \
+ } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
+ dest = 0; \
+ } else if (tmp < 0) { \
dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
} else { \
dest = src1 << tmp; \
@@ -903,7 +915,16 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t valop, uint32_t shiftop)
int32_t dest;
int32_t val = (int32_t)valop;
int8_t shift = (int8_t)shiftop;
- if (shift < 0) {
+ if (shift >= 32) {
+ if (val) {
+ SET_QC();
+ dest = (val >> 31) ^ ~SIGNBIT;
+ } else {
+ dest = 0;
+ }
+ } else if (shift <= -32) {
+ dest = 0;
+ } else if (shift < 0) {
int64_t big_dest = ((int64_t)val + (1 << (-1 - shift)));
dest = big_dest >> -shift;
} else {
@@ -923,7 +944,14 @@ uint64_t HELPER(neon_qrshl_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
int8_t shift = (uint8_t)shiftop;
int64_t val = valop;
- if (shift < 0) {
+ if (shift >= 64) {
+ if (val) {
+ SET_QC();
+ val = (val >> 63) ^ ~SIGNBIT64;
+ }
+ } else if (shift <= -64) {
+ val = 0;
+ } else if (shift < 0) {
val >>= (-shift - 1);
if (val == INT64_MAX) {
/* In this case, it means that the rounding constant is 1,
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 09/10] target-arm: Fix unsigned VQRSHL by large shift counts
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (7 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 08/10] target-arm: Fix signed VQRSHL by large shift counts Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-15 13:44 ` [Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap Peter Maydell
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
Correctly handle VQRSHL of unsigned values by a shift count of the
width of the data type or larger, which must be special-cased in the
qrshl_u* helper functions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/neon_helper.c | 37 ++++++++++++++++++++++++++++++++++---
1 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f8f6db6..c813fa0 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -818,7 +818,18 @@ uint64_t HELPER(neon_qshlu_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
- if (tmp < 0) { \
+ if (tmp >= (ssize_t)sizeof(src1) * 8) { \
+ if (src1) { \
+ SET_QC(); \
+ dest = ~0; \
+ } else { \
+ dest = 0; \
+ } \
+ } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
+ dest = 0; \
+ } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
+ dest = src1 >> (sizeof(src1) * 8 - 1); \
+ } else if (tmp < 0) { \
dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
} else { \
dest = src1 << tmp; \
@@ -837,7 +848,18 @@ uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t val, uint32_t shiftop)
{
uint32_t dest;
int8_t shift = (int8_t)shiftop;
- if (shift < 0) {
+ if (shift >= 32) {
+ if (val) {
+ SET_QC();
+ dest = ~0;
+ } else {
+ dest = 0;
+ }
+ } else if (shift < -32) {
+ dest = 0;
+ } else if (shift == -32) {
+ dest = val >> 31;
+ } else if (shift < 0) {
uint64_t big_dest = ((uint64_t)val + (1 << (-1 - shift)));
dest = big_dest >> -shift;
} else {
@@ -855,7 +877,16 @@ uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t val, uint32_t shiftop)
uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
- if (shift < 0) {
+ if (shift >= 64) {
+ if (val) {
+ SET_QC();
+ val = ~0;
+ }
+ } else if (shift < -64) {
+ val = 0;
+ } else if (shift == -64) {
+ val >>= 63;
+ } else if (shift < 0) {
val >>= (-shift - 1);
if (val == UINT64_MAX) {
/* In this case, it means that the rounding constant is 1,
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (8 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 09/10] target-arm: Fix unsigned " Peter Maydell
@ 2011-02-15 13:44 ` Peter Maydell
2011-02-20 16:52 ` Aurelien Jarno
2011-02-15 16:35 ` [Qemu-devel] Re: [PATCH 00/10] Fix Neon shift instructions Christophe Lyon
2011-02-20 16:52 ` [Qemu-devel] " Aurelien Jarno
11 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2011-02-15 13:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Lyon, patches
For Neon shifts by immediate and narrow, correctly handle the case
where the source registers and the destination registers overlap
(the second pass should use the original register contents, not the
results of the first pass).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 38 +++++++++++++++++++++++++++-----------
1 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index a02b20f..4d5d305 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4839,31 +4839,47 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
abort();
}
+ if (size == 3) {
+ neon_load_reg64(cpu_V0, rm);
+ neon_load_reg64(cpu_V1, rm + 1);
+ } else {
+ tmp4 = neon_load_reg(rm + 1, 0);
+ tmp5 = neon_load_reg(rm + 1, 1);
+ }
for (pass = 0; pass < 2; pass++) {
if (size == 3) {
- neon_load_reg64(cpu_V0, rm + pass);
+ TCGv_i64 in;
+ if (pass == 0) {
+ in = cpu_V0;
+ } else {
+ in = cpu_V1;
+ }
if (q) {
if (input_unsigned) {
- gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
- tmp64);
+ gen_helper_neon_rshl_u64(cpu_V0, in, tmp64);
} else {
- gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
- tmp64);
+ gen_helper_neon_rshl_s64(cpu_V0, in, tmp64);
}
} else {
if (input_unsigned) {
- gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
- tmp64);
+ gen_helper_neon_shl_u64(cpu_V0, in, tmp64);
} else {
- gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
- tmp64);
+ gen_helper_neon_shl_s64(cpu_V0, in, tmp64);
}
}
} else {
- tmp = neon_load_reg(rm + pass, 0);
+ if (pass == 0) {
+ tmp = neon_load_reg(rm, 0);
+ } else {
+ tmp = tmp4;
+ }
gen_neon_shift_narrow(size, tmp, tmp2, q,
input_unsigned);
- tmp3 = neon_load_reg(rm + pass, 1);
+ if (pass == 0) {
+ tmp3 = neon_load_reg(rm, 1);
+ } else {
+ tmp3 = tmp5;
+ }
gen_neon_shift_narrow(size, tmp3, tmp2, q,
input_unsigned);
tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap
2011-02-15 13:44 ` [Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap Peter Maydell
@ 2011-02-20 16:52 ` Aurelien Jarno
0 siblings, 0 replies; 14+ messages in thread
From: Aurelien Jarno @ 2011-02-20 16:52 UTC (permalink / raw)
To: Peter Maydell; +Cc: Christophe Lyon, qemu-devel, patches
On Tue, Feb 15, 2011 at 01:44:50PM +0000, Peter Maydell wrote:
> For Neon shifts by immediate and narrow, correctly handle the case
> where the source registers and the destination registers overlap
> (the second pass should use the original register contents, not the
> results of the first pass).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target-arm/translate.c | 38 +++++++++++++++++++++++++++-----------
> 1 files changed, 27 insertions(+), 11 deletions(-)
That looks correct, but it makes GCC (tested 4.3 to 4.6) complaining:
| cc1: warnings being treated as errors
| qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
| qemu/target-arm/translate.c:4185: error: ‘tmp4’ may be used uninitialized in this function
| qemu/target-arm/translate.c:4185: error: ‘tmp5’ may be used uninitialized in this function
For a quick look, it seems to be a GCC issue, but we have no other
choices than workarouding it.
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index a02b20f..4d5d305 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4839,31 +4839,47 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
> abort();
> }
>
> + if (size == 3) {
> + neon_load_reg64(cpu_V0, rm);
> + neon_load_reg64(cpu_V1, rm + 1);
> + } else {
> + tmp4 = neon_load_reg(rm + 1, 0);
> + tmp5 = neon_load_reg(rm + 1, 1);
> + }
> for (pass = 0; pass < 2; pass++) {
> if (size == 3) {
> - neon_load_reg64(cpu_V0, rm + pass);
> + TCGv_i64 in;
> + if (pass == 0) {
> + in = cpu_V0;
> + } else {
> + in = cpu_V1;
> + }
> if (q) {
> if (input_unsigned) {
> - gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
> - tmp64);
> + gen_helper_neon_rshl_u64(cpu_V0, in, tmp64);
> } else {
> - gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
> - tmp64);
> + gen_helper_neon_rshl_s64(cpu_V0, in, tmp64);
> }
> } else {
> if (input_unsigned) {
> - gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
> - tmp64);
> + gen_helper_neon_shl_u64(cpu_V0, in, tmp64);
> } else {
> - gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
> - tmp64);
> + gen_helper_neon_shl_s64(cpu_V0, in, tmp64);
> }
> }
> } else {
> - tmp = neon_load_reg(rm + pass, 0);
> + if (pass == 0) {
> + tmp = neon_load_reg(rm, 0);
> + } else {
> + tmp = tmp4;
> + }
> gen_neon_shift_narrow(size, tmp, tmp2, q,
> input_unsigned);
> - tmp3 = neon_load_reg(rm + pass, 1);
> + if (pass == 0) {
> + tmp3 = neon_load_reg(rm, 1);
> + } else {
> + tmp3 = tmp5;
> + }
> gen_neon_shift_narrow(size, tmp3, tmp2, q,
> input_unsigned);
> tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
> --
> 1.7.1
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [PATCH 00/10] Fix Neon shift instructions
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (9 preceding siblings ...)
2011-02-15 13:44 ` [Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap Peter Maydell
@ 2011-02-15 16:35 ` Christophe Lyon
2011-02-20 16:52 ` [Qemu-devel] " Aurelien Jarno
11 siblings, 0 replies; 14+ messages in thread
From: Christophe Lyon @ 2011-02-15 16:35 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel@nongnu.org
On 15.02.2011 14:44, Peter Maydell wrote:
> This patch series fixes bugs in the Neon shift instructions
> VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN,
> VRSHL, and VQRSHL. It is based on the v3 patchset Christophe
> sent recently, with some fixes for minor nits in those patches,
> plus some patches from me which fix problems with shifts by
> large shift counts and an issue with overlapping source and
> destination registers.
>
> With this patchset qemu passes random instruction sequence
> testing for all these instruction patterns.
>
FWIW, I confirm that with this patchset, qemu passes all my tests on shift instruction.
Peter, thank you for adapting my patches and transforming my patch #4 into your patch #5.
Christophe.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions
2011-02-15 13:44 [Qemu-devel] [PATCH 00/10] Fix Neon shift instructions Peter Maydell
` (10 preceding siblings ...)
2011-02-15 16:35 ` [Qemu-devel] Re: [PATCH 00/10] Fix Neon shift instructions Christophe Lyon
@ 2011-02-20 16:52 ` Aurelien Jarno
11 siblings, 0 replies; 14+ messages in thread
From: Aurelien Jarno @ 2011-02-20 16:52 UTC (permalink / raw)
To: Peter Maydell; +Cc: Christophe Lyon, qemu-devel, patches
On Tue, Feb 15, 2011 at 01:44:40PM +0000, Peter Maydell wrote:
> This patch series fixes bugs in the Neon shift instructions
> VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN,
> VRSHL, and VQRSHL. It is based on the v3 patchset Christophe
> sent recently, with some fixes for minor nits in those patches,
> plus some patches from me which fix problems with shifts by
> large shift counts and an issue with overlapping source and
> destination registers.
>
> With this patchset qemu passes random instruction sequence
> testing for all these instruction patterns.
>
> Christophe Lyon (5):
> target-arm: Fix rounding constant addition for Neon shifts
> target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width
> target-arm: fix unsigned 64 bit right shifts.
> target-arm: fix Neon VQSHRN and VSHRN.
> target-arm: fix decoding of Neon 64 bit shifts.
>
> Peter Maydell (5):
> target-arm: Fix signed VRSHL by large shift counts
> target-arm: Fix saturated values for Neon right shifts
> target-arm: Fix signed VQRSHL by large shift counts
> target-arm: Fix unsigned VQRSHL by large shift counts
> target-arm: Fix shift by immediate and narrow where src,dest overlap
>
> target-arm/neon_helper.c | 232 +++++++++++++++++++++++++++++++++++++++-------
> target-arm/translate.c | 71 ++++++++++----
> 2 files changed, 250 insertions(+), 53 deletions(-)
>
Thanks, I applied patches 1 to 9. See my comment for the patch 10.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 14+ messages in thread