* [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
@ 2011-01-28 15:50 ` christophe.lyon
2011-01-31 8:20 ` Aurelien Jarno
2011-01-28 15:51 ` [Qemu-devel] [PATCH 2/8] target-arm: Create and use neon_unarrow_sat* helpers christophe.lyon
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:50 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Handle corner cases where the addition of the rounding constant could
cause overflows.
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/neon_helper.c | 61 ++++++++++++++++++++++++++++++++++++++-------
target-arm/translate.c | 17 ++++++++++--
2 files changed, 65 insertions(+), 13 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index bf29bbe..5971275 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -540,6 +540,9 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
return val;
}
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator, which is really needed only when
+ * dealing with 32 bits input values. */
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
@@ -548,11 +551,12 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
} 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 = src1 >> (-tmp - 1); \
dest++; \
dest >>= 1; \
} else if (tmp < 0) { \
- dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
+ int64_t big_dest = ((int64_t)src1 + (1 << (-1 - tmp))); \
+ dest = big_dest >> -tmp; \
} else { \
dest = src1 << tmp; \
}} while (0)
@@ -561,6 +565,8 @@ NEON_VOP(rshl_s16, neon_s16, 2)
NEON_VOP(rshl_s32, neon_s32, 1)
#undef NEON_FN
+/* 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;
@@ -569,18 +575,37 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
val = 0;
} else if (shift < -64) {
val >>= 63;
- } else if (shift == -63) {
+ } else if (shift == -64) {
val >>= 63;
val++;
val >>= 1;
} else if (shift < 0) {
- val = (val + ((int64_t)1 << (-1 - shift))) >> -shift;
+ int64_t round = (int64_t)1 << (-1 - shift);
+ /* Reduce the range as long as the addition overflows. It's
+ * sufficient to check if (val+round) is < 0 and val > 0
+ * because round is > 0. */
+ while ((val > 0) && ((val + round) < 0) && round > 1) {
+ shift++;
+ round >>= 1;
+ val >>= 1;
+ }
+ if ((val > 0) && (val + round) < 0) {
+ /* If addition still overflows at this point, it means
+ * that round==1, thus shift==-1, and also that
+ * val==0x7FFFFFFFFFFFFFFF. */
+ val = 0x4000000000000000LL;
+ } else {
+ val = (val + round) >> -shift;
+ }
} else {
val <<= shift;
}
return val;
}
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator, which is really needed only when
+ * dealing with 32 bits input values. */
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
@@ -588,9 +613,10 @@ 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; \
+ uint64_t big_dest = ((uint64_t)src1 + (1 << (-1 - tmp))); \
+ dest = big_dest >> -tmp; \
} else { \
dest = src1 << tmp; \
}} while (0)
@@ -602,14 +628,29 @@ NEON_VOP(rshl_u32, neon_u32, 1)
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. */
val >>= 63;
- } if (shift < 0) {
- val = (val + ((uint64_t)1 << (-1 - shift))) >> -shift;
- val >>= -shift;
+ } else if (shift < 0) {
+ uint64_t round = (uint64_t)1 << (-1 - shift);
+ /* Reduce the range as long as the addition overflows. It's
+ * sufficient to check if (val+round) is < val
+ * because val and round are > 0. */
+ while (((val + round) < val) && round > 1) {
+ shift++;
+ round >>= 1;
+ val >>= 1;
+ }
+ if ((val + round) < val) {
+ /* If addition still overflows at this point, it means
+ * that round==1, thus shift==-1, and also that
+ * val==0x&FFFFFFFFFFFFFFF. */
+ val = 0x8000000000000000LL;
+ } else {
+ val = (val + round) >> -shift;
+ }
} else {
val <<= shift;
}
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 4cf2ecd..b14fa4b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4885,13 +4885,24 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
tcg_gen_shli_i64(cpu_V0, cpu_V0, shift);
if (size < 2 || !u) {
uint64_t imm64;
- if (size == 0) {
+ switch(size) {
+ case 0:
imm = (0xffu >> (8 - shift));
imm |= imm << 16;
- } else {
+ break;
+ case 1:
imm = 0xffff >> (16 - shift);
+ break;
+ case 2:
+ imm = 0xffffffff >> (32 - shift);
+ break;
+ }
+ if (size < 2) {
+ imm64 = imm | (((uint64_t)imm) << 32);
+ } else {
+ imm64 = imm;
}
- imm64 = imm | (((uint64_t)imm) << 32);
+ imm64 = ~imm64;
tcg_gen_andi_i64(cpu_V0, cpu_V0, imm64);
}
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-28 15:50 ` [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA christophe.lyon
@ 2011-01-31 8:20 ` Aurelien Jarno
2011-01-31 9:35 ` Christophe Lyon
0 siblings, 1 reply; 21+ messages in thread
From: Aurelien Jarno @ 2011-01-31 8:20 UTC (permalink / raw)
To: christophe.lyon; +Cc: qemu-devel
On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
> From: Christophe Lyon <christophe.lyon@st.com>
>
> Handle corner cases where the addition of the rounding constant could
> cause overflows.
After applying this patch, I get the following gcc warning:
CC translate.o
cc1: warnings being treated as errors
qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
make: *** [translate.o] Error 1
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
> ---
> target-arm/neon_helper.c | 61 ++++++++++++++++++++++++++++++++++++++-------
> target-arm/translate.c | 17 ++++++++++--
> 2 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index bf29bbe..5971275 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -540,6 +540,9 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
> return val;
> }
>
> +/* The addition of the rounding constant may overflow, so we use an
> + * intermediate 64 bits accumulator, which is really needed only when
> + * dealing with 32 bits input values. */
> #define NEON_FN(dest, src1, src2) do { \
> int8_t tmp; \
> tmp = (int8_t)src2; \
> @@ -548,11 +551,12 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
> } 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 = src1 >> (-tmp - 1); \
> dest++; \
> dest >>= 1; \
> } else if (tmp < 0) { \
> - dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
> + int64_t big_dest = ((int64_t)src1 + (1 << (-1 - tmp))); \
> + dest = big_dest >> -tmp; \
> } else { \
> dest = src1 << tmp; \
> }} while (0)
> @@ -561,6 +565,8 @@ NEON_VOP(rshl_s16, neon_s16, 2)
> NEON_VOP(rshl_s32, neon_s32, 1)
> #undef NEON_FN
>
> +/* 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;
> @@ -569,18 +575,37 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
> val = 0;
> } else if (shift < -64) {
> val >>= 63;
> - } else if (shift == -63) {
> + } else if (shift == -64) {
> val >>= 63;
> val++;
> val >>= 1;
> } else if (shift < 0) {
> - val = (val + ((int64_t)1 << (-1 - shift))) >> -shift;
> + int64_t round = (int64_t)1 << (-1 - shift);
> + /* Reduce the range as long as the addition overflows. It's
> + * sufficient to check if (val+round) is < 0 and val > 0
> + * because round is > 0. */
> + while ((val > 0) && ((val + round) < 0) && round > 1) {
> + shift++;
> + round >>= 1;
> + val >>= 1;
> + }
> + if ((val > 0) && (val + round) < 0) {
> + /* If addition still overflows at this point, it means
> + * that round==1, thus shift==-1, and also that
> + * val==0x7FFFFFFFFFFFFFFF. */
> + val = 0x4000000000000000LL;
> + } else {
> + val = (val + round) >> -shift;
> + }
> } else {
> val <<= shift;
> }
> return val;
> }
>
> +/* The addition of the rounding constant may overflow, so we use an
> + * intermediate 64 bits accumulator, which is really needed only when
> + * dealing with 32 bits input values. */
> #define NEON_FN(dest, src1, src2) do { \
> int8_t tmp; \
> tmp = (int8_t)src2; \
> @@ -588,9 +613,10 @@ 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; \
> + uint64_t big_dest = ((uint64_t)src1 + (1 << (-1 - tmp))); \
> + dest = big_dest >> -tmp; \
> } else { \
> dest = src1 << tmp; \
> }} while (0)
> @@ -602,14 +628,29 @@ NEON_VOP(rshl_u32, neon_u32, 1)
> 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. */
> val >>= 63;
> - } if (shift < 0) {
> - val = (val + ((uint64_t)1 << (-1 - shift))) >> -shift;
> - val >>= -shift;
> + } else if (shift < 0) {
> + uint64_t round = (uint64_t)1 << (-1 - shift);
> + /* Reduce the range as long as the addition overflows. It's
> + * sufficient to check if (val+round) is < val
> + * because val and round are > 0. */
> + while (((val + round) < val) && round > 1) {
> + shift++;
> + round >>= 1;
> + val >>= 1;
> + }
> + if ((val + round) < val) {
> + /* If addition still overflows at this point, it means
> + * that round==1, thus shift==-1, and also that
> + * val==0x&FFFFFFFFFFFFFFF. */
> + val = 0x8000000000000000LL;
> + } else {
> + val = (val + round) >> -shift;
> + }
> } else {
> val <<= shift;
> }
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 4cf2ecd..b14fa4b 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4885,13 +4885,24 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
> tcg_gen_shli_i64(cpu_V0, cpu_V0, shift);
> if (size < 2 || !u) {
> uint64_t imm64;
> - if (size == 0) {
> + switch(size) {
> + case 0:
> imm = (0xffu >> (8 - shift));
> imm |= imm << 16;
> - } else {
> + break;
> + case 1:
> imm = 0xffff >> (16 - shift);
> + break;
> + case 2:
> + imm = 0xffffffff >> (32 - shift);
> + break;
> + }
> + if (size < 2) {
> + imm64 = imm | (((uint64_t)imm) << 32);
> + } else {
> + imm64 = imm;
> }
> - imm64 = imm | (((uint64_t)imm) << 32);
> + imm64 = ~imm64;
> tcg_gen_andi_i64(cpu_V0, cpu_V0, imm64);
> }
> }
> --
> 1.7.2.3
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 8:20 ` Aurelien Jarno
@ 2011-01-31 9:35 ` Christophe Lyon
2011-01-31 9:44 ` Aurelien Jarno
0 siblings, 1 reply; 21+ messages in thread
From: Christophe Lyon @ 2011-01-31 9:35 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 31.01.2011 09:20, Aurelien Jarno wrote:
> On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
>> From: Christophe Lyon <christophe.lyon@st.com>
>>
>> Handle corner cases where the addition of the rounding constant could
>> cause overflows.
>
> After applying this patch, I get the following gcc warning:
> CC translate.o
> cc1: warnings being treated as errors
> qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
> qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
> make: *** [translate.o] Error 1
>
Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64).
Christophe.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 9:35 ` Christophe Lyon
@ 2011-01-31 9:44 ` Aurelien Jarno
2011-01-31 9:54 ` Alon Levy
2011-01-31 14:27 ` Christophe Lyon
0 siblings, 2 replies; 21+ messages in thread
From: Aurelien Jarno @ 2011-01-31 9:44 UTC (permalink / raw)
To: Christophe Lyon; +Cc: qemu-devel
On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote:
> On 31.01.2011 09:20, Aurelien Jarno wrote:
> > On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
> >> From: Christophe Lyon <christophe.lyon@st.com>
> >>
> >> Handle corner cases where the addition of the rounding constant could
> >> cause overflows.
> >
> > After applying this patch, I get the following gcc warning:
> > CC translate.o
> > cc1: warnings being treated as errors
> > qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
> > qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
> > make: *** [translate.o] Error 1
> >
>
> Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64).
>
I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0
(r169270). This is also on x86_64.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 9:44 ` Aurelien Jarno
@ 2011-01-31 9:54 ` Alon Levy
2011-01-31 14:27 ` Christophe Lyon
1 sibling, 0 replies; 21+ messages in thread
From: Alon Levy @ 2011-01-31 9:54 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Christophe Lyon, qemu-devel
On Mon, Jan 31, 2011 at 10:44:14AM +0100, Aurelien Jarno wrote:
> On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote:
> > On 31.01.2011 09:20, Aurelien Jarno wrote:
> > > On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
> > >> From: Christophe Lyon <christophe.lyon@st.com>
> > >>
> > >> Handle corner cases where the addition of the rounding constant could
> > >> cause overflows.
> > >
> > > After applying this patch, I get the following gcc warning:
> > > CC translate.o
> > > cc1: warnings being treated as errors
> > > qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
> > > qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
> > > make: *** [translate.o] Error 1
> > >
> >
> > Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64).
> >
>
> I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0
> (r169270). This is also on x86_64.
>
gcc 4.6.0 turned on warnings also on set but not used variables, there
are a few places that need fixes (pretty simple). This is the patch
I have applied:
commit 53bf1becf85a14c399698ae0961eb2b08d231986
Author: Alon Levy <alevy@redhat.com>
Date: Fri Jan 28 11:04:43 2011 +0200
(temp) gcc 4.6.0 corrections for build
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index e37e226..da1a2e9 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -708,6 +708,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int l2_size, i, j, l1_modified, l2_modified, nb_csectors, refcount;
int ret;
+ (void)l2_modified;
+ (void)l2_size;
+
l2_table = NULL;
l1_table = NULL;
l1_size2 = l1_size * sizeof(uint64_t);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7dfc357..a7019ee 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -892,7 +892,7 @@ static int kvm_get_xsave(CPUState *env)
#ifdef KVM_CAP_XSAVE
struct kvm_xsave* xsave;
int ret, i;
- uint16_t cwd, swd, twd, fop;
+ uint16_t cwd, swd, twd;
if (!kvm_has_xsave())
return kvm_get_fpu(env);
@@ -907,7 +907,6 @@ static int kvm_get_xsave(CPUState *env)
cwd = (uint16_t)xsave->region[0];
swd = (uint16_t)(xsave->region[0] >> 16);
twd = (uint16_t)xsave->region[1];
- fop = (uint16_t)(xsave->region[1] >> 16);
env->fpstt = (swd >> 11) & 7;
env->fpus = swd;
env->fpuc = cwd;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 5dd6a2c..b4d230b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -582,6 +582,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
nparam = gen_opparam_ptr++;
#ifdef TCG_TARGET_I386
call_type = (flags & TCG_CALL_TYPE_MASK);
+ (void)call_type;
#endif
if (ret != TCG_CALL_DUMMY_ARG) {
#if TCG_TARGET_REG_BITS < 64
diff --git a/ui/sdl.c b/ui/sdl.c
index f599d42..a1458ce 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -87,12 +87,6 @@ static void sdl_update(DisplayState *ds, int x, int y, int w, int h)
static void sdl_setdata(DisplayState *ds)
{
- SDL_Rect rec;
- rec.x = 0;
- rec.y = 0;
- rec.w = real_screen->w;
- rec.h = real_screen->h;
-
if (guest_screen != NULL) SDL_FreeSurface(guest_screen);
guest_screen = SDL_CreateRGBSurfaceFrom(ds_get_data(ds), ds_get_width(ds), ds_get_height(ds),
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 9:44 ` Aurelien Jarno
2011-01-31 9:54 ` Alon Levy
@ 2011-01-31 14:27 ` Christophe Lyon
2011-01-31 15:59 ` Aurelien Jarno
1 sibling, 1 reply; 21+ messages in thread
From: Christophe Lyon @ 2011-01-31 14:27 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 31.01.2011 10:44, Aurelien Jarno wrote:
> On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote:
>> On 31.01.2011 09:20, Aurelien Jarno wrote:
>>> On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
>>>> From: Christophe Lyon <christophe.lyon@st.com>
>>>>
>>>> Handle corner cases where the addition of the rounding constant could
>>>> cause overflows.
>>>
>>> After applying this patch, I get the following gcc warning:
>>> CC translate.o
>>> cc1: warnings being treated as errors
>>> qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
>>> qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
>>> make: *** [translate.o] Error 1
>>>
>>
>> Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64).
>>
>
> I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0
> (r169270). This is also on x86_64.
>
Well, I can't reproduce this error :-(
For the record, I configure with --target-list=arm-softmmu,arm-linux-user --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to GCC-4.5.1.
In verbose mode, I confirm that GCC is invoked with
-Werror -m64 -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fstack-protector-all -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits
I have tried from a freshly cloned qemu.git, and I have no error.
Can you send me your translate.c & translate.i (pre-processed by GCC's C-preprocesssor with -E option)
Christophe.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 14:27 ` Christophe Lyon
@ 2011-01-31 15:59 ` Aurelien Jarno
2011-01-31 16:46 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Aurelien Jarno @ 2011-01-31 15:59 UTC (permalink / raw)
To: Christophe Lyon; +Cc: qemu-devel
Christophe Lyon a écrit :
> On 31.01.2011 10:44, Aurelien Jarno wrote:
>> On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote:
>>> On 31.01.2011 09:20, Aurelien Jarno wrote:
>>>> On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.lyon@st.com wrote:
>>>>> From: Christophe Lyon <christophe.lyon@st.com>
>>>>>
>>>>> Handle corner cases where the addition of the rounding constant could
>>>>> cause overflows.
>>>> After applying this patch, I get the following gcc warning:
>>>> CC translate.o
>>>> cc1: warnings being treated as errors
>>>> qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
>>>> qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function
>>>> make: *** [translate.o] Error 1
>>>>
>>> Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64).
>>>
>> I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0
>> (r169270). This is also on x86_64.
>>
>
> Well, I can't reproduce this error :-(
> For the record, I configure with --target-list=arm-softmmu,arm-linux-user --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to GCC-4.5.1.
>
It seems the problems come from here, if I add --enable-debug, I am not
able to reproduce the problem anymore. I don't understand why though.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 15:59 ` Aurelien Jarno
@ 2011-01-31 16:46 ` Peter Maydell
2011-01-31 18:09 ` Christophe Lyon
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2011-01-31 16:46 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Christophe Lyon, qemu-devel
On 31 January 2011 15:59, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Christophe Lyon a écrit :
>> Well, I can't reproduce this error :-(
>> For the record, I configure with --target-list=arm-softmmu,arm-linux-user --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to GCC-4.5.1.
>>
>
> It seems the problems come from here, if I add --enable-debug, I am not
> able to reproduce the problem anymore. I don't understand why though.
--enable-debug turns off optimisation (ie does not pass -O2); a number
of gcc's warnings, including this one, are only done in the dataflow analysis
pass and so will not be generated unless you have optimisation enabled.
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
2011-01-31 16:46 ` Peter Maydell
@ 2011-01-31 18:09 ` Christophe Lyon
0 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2011-01-31 18:09 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 31.01.2011 17:46, Peter Maydell wrote:
> On 31 January 2011 15:59, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> It seems the problems come from here, if I add --enable-debug, I am not
>> able to reproduce the problem anymore. I don't understand why though.
>
> --enable-debug turns off optimisation (ie does not pass -O2); a number
> of gcc's warnings, including this one, are only done in the dataflow analysis
> pass and so will not be generated unless you have optimisation enabled.
>
Indeed.
I have just sent another patchset which addresses this problem.
Christophe.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 2/8] target-arm: Create and use neon_unarrow_sat* helpers
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
2011-01-28 15:50 ` [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 3/8] target-arm: VQRSHRN related changes christophe.lyon
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Fix VQMOVUN, improve VQSHRUN and VQRSHRUN.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/helpers.h | 3 ++
target-arm/neon_helper.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++
target-arm/translate.c | 43 ++++++++++++++++++++++---------
3 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/target-arm/helpers.h b/target-arm/helpers.h
index 8a2564e..4d0de00 100644
--- a/target-arm/helpers.h
+++ b/target-arm/helpers.h
@@ -299,10 +299,13 @@ DEF_HELPER_3(neon_qrdmulh_s32, i32, env, i32, i32)
DEF_HELPER_1(neon_narrow_u8, i32, i64)
DEF_HELPER_1(neon_narrow_u16, i32, i64)
+DEF_HELPER_2(neon_unarrow_sat8, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_u8, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_s8, i32, env, i64)
+DEF_HELPER_2(neon_unarrow_sat16, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_u16, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_s16, i32, env, i64)
+DEF_HELPER_2(neon_unarrow_sat32, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_u32, i32, env, i64)
DEF_HELPER_2(neon_narrow_sat_s32, i32, env, i64)
DEF_HELPER_1(neon_narrow_high_u8, i32, i64)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 5971275..71e3c74 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1094,6 +1094,33 @@ uint32_t HELPER(neon_narrow_round_high_u16)(uint64_t x)
return ((x >> 16) & 0xffff) | ((x >> 32) & 0xffff0000);
}
+uint32_t HELPER(neon_unarrow_sat8)(CPUState *env, uint64_t x)
+{
+ uint16_t s;
+ uint8_t d;
+ uint32_t res = 0;
+#define SAT8(n) \
+ s = x >> n; \
+ if (s & 0x8000) { \
+ SET_QC(); \
+ } else { \
+ if (s > 0xff) { \
+ d = 0xff; \
+ SET_QC(); \
+ } else { \
+ d = s; \
+ } \
+ res |= (uint32_t)d << (n / 2); \
+ }
+
+ SAT8(0);
+ SAT8(16);
+ SAT8(32);
+ SAT8(48);
+#undef SAT8
+ return res;
+}
+
uint32_t HELPER(neon_narrow_sat_u8)(CPUState *env, uint64_t x)
{
uint16_t s;
@@ -1140,6 +1167,29 @@ uint32_t HELPER(neon_narrow_sat_s8)(CPUState *env, uint64_t x)
return res;
}
+uint32_t HELPER(neon_unarrow_sat16)(CPUState *env, uint64_t x)
+{
+ uint32_t high;
+ uint32_t low;
+ low = x;
+ if (low & 0x80000000) {
+ low = 0;
+ SET_QC();
+ } else if (low > 0xffff) {
+ low = 0xffff;
+ SET_QC();
+ }
+ high = x >> 32;
+ if (high & 0x80000000) {
+ high = 0;
+ SET_QC();
+ } else if (high > 0xffff) {
+ high = 0xffff;
+ SET_QC();
+ }
+ return low | (high << 16);
+}
+
uint32_t HELPER(neon_narrow_sat_u16)(CPUState *env, uint64_t x)
{
uint32_t high;
@@ -1174,6 +1224,19 @@ uint32_t HELPER(neon_narrow_sat_s16)(CPUState *env, uint64_t x)
return (uint16_t)low | (high << 16);
}
+uint32_t HELPER(neon_unarrow_sat32)(CPUState *env, uint64_t x)
+{
+ if (x & 0x8000000000000000ull) {
+ SET_QC();
+ return 0;
+ }
+ if (x > 0xffffffffu) {
+ SET_QC();
+ return 0xffffffffu;
+ }
+ return x;
+}
+
uint32_t HELPER(neon_narrow_sat_u32)(CPUState *env, uint64_t x)
{
if (x > 0xffffffffu) {
diff --git a/target-arm/translate.c b/target-arm/translate.c
index b14fa4b..cda5a73 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4078,6 +4078,16 @@ static inline void gen_neon_narrow_satu(int size, TCGv dest, TCGv_i64 src)
}
}
+static inline void gen_neon_unarrow_sats(int size, TCGv dest, TCGv_i64 src)
+{
+ switch(size) {
+ case 0: gen_helper_neon_unarrow_sat8(dest, cpu_env, src); break;
+ case 1: gen_helper_neon_unarrow_sat16(dest, cpu_env, src); break;
+ case 2: gen_helper_neon_unarrow_sat32(dest, cpu_env, src); break;
+ default: abort();
+ }
+}
+
static inline void gen_neon_shift_narrow(int size, TCGv var, TCGv shift,
int q, int u)
{
@@ -4852,13 +4862,14 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
dead_tmp(tmp3);
}
tmp = new_tmp();
- if (op == 8 && !u) {
- gen_neon_narrow(size - 1, tmp, cpu_V0);
- } else {
- if (op == 8)
- gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
- else
- gen_neon_narrow_satu(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 { /* VQSHRN / VQRSHRN */
+ gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
}
neon_store_reg(rd, pass, tmp);
} /* for pass */
@@ -5485,12 +5496,18 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
for (pass = 0; pass < 2; pass++) {
neon_load_reg64(cpu_V0, rm + pass);
tmp = new_tmp();
- if (op == 36 && q == 0) {
- gen_neon_narrow(size, tmp, cpu_V0);
- } else if (q) {
- gen_neon_narrow_satu(size, tmp, cpu_V0);
- } else {
- gen_neon_narrow_sats(size, tmp, cpu_V0);
+ if (op == 36) {
+ if (q) { /* VQMOVUN */
+ gen_neon_unarrow_sats(size, tmp, cpu_V0);
+ } else { /* VMOVN */
+ gen_neon_narrow(size, tmp, cpu_V0);
+ }
+ } else { /* VQMOVN */
+ if (q) {
+ gen_neon_narrow_satu(size, tmp, cpu_V0);
+ } else {
+ gen_neon_narrow_sats(size, tmp, cpu_V0);
+ }
}
if (pass == 0) {
tmp2 = tmp;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 3/8] target-arm: VQRSHRN related changes
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
2011-01-28 15:50 ` [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 2/8] target-arm: Create and use neon_unarrow_sat* helpers christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 4/8] target-arm: fiddle decoding of 64 bit shift by imm and narrow christophe.lyon
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
More fixes for VQSHRN and VQSHRUN.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
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 cda5a73..3537698 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4108,8 +4108,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.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 4/8] target-arm: fiddle decoding of 64 bit shift by imm and narrow
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (2 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 3/8] target-arm: VQRSHRN related changes christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 5/8] target-arm: fix neon vqrshl instruction christophe.lyon
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Tweak decoding of the shift-by-imm and narrow 64 bit insns
(VSHRN, VRSHRN, VQSHRN, VQSHRUN, VQRSHRN, VQRSHRUN).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/translate.c | 28 ++++++++++++++++++----------
1 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3537698..452cb71 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4842,21 +4842,29 @@ 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 ((op == 8 && !u) || (op == 9 && u)) {
+ 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 ((op == 8 && !u) || (op == 9 && u)) {
+ 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,
+ (op == 8) ? !u : u);
tmp3 = neon_load_reg(rm + pass, 1);
- gen_neon_shift_narrow(size, tmp3, tmp2, q, u);
+ gen_neon_shift_narrow(size, tmp3, tmp2, q,
+ (op == 8) ? !u : u);
tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
dead_tmp(tmp);
dead_tmp(tmp3);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 5/8] target-arm: fix neon vqrshl instruction
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (3 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 4/8] target-arm: fiddle decoding of 64 bit shift by imm and narrow christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 6/8] target-arm: Fix Neon VQ(R)SHRN instructions christophe.lyon
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/neon_helper.c | 21 ++++++++++++++++++---
1 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 71e3c74..3337c52 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -825,9 +825,24 @@ 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
+uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t val, uint32_t shiftop)
+{
+ int8_t shift = (int8_t)shiftop;
+ if (shift < 0) {
+ val = ((uint64_t)val + (1 << (-1 - shift))) >> -shift;
+ } else {
+ uint32_t tmp = val;
+ val <<= shift;
+ if ((val >> shift) != tmp) {
+ SET_QC();
+ val = ~0;
+ }
+ }
+ return val;
+}
+
uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -853,7 +868,7 @@ 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)) - (src1 > 0 ? 1 : 0); \
} \
}} while (0)
NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
@@ -869,7 +884,7 @@ uint64_t HELPER(neon_qrshl_s64)(CPUState *env, uint64_t valop, uint64_t shiftop)
if (shift < 0) {
val = (val + (1 << (-1 - shift))) >> -shift;
} else {
- int64_t tmp = val;;
+ int64_t tmp = val;
val <<= shift;
if ((val >> shift) != tmp) {
SET_QC();
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 6/8] target-arm: Fix Neon VQ(R)SHRN instructions.
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (4 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 5/8] target-arm: fix neon vqrshl instruction christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 7/8] implement vsli.64, vsri.64 christophe.lyon
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Handle unsigned variant of VQ(R)SHRN instructions.
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/translate.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 452cb71..3b14b8f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4876,8 +4876,12 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
} else { /* VSHRN / VRSHRN */
gen_neon_narrow(size - 1, tmp, cpu_V0);
}
- } else { /* VQSHRN / VQRSHRN */
- gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
+ } else {
+ if (u) { /* VQSHRUN / VQRSHRUN */
+ 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.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 7/8] implement vsli.64, vsri.64
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (5 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 6/8] target-arm: Fix Neon VQ(R)SHRN instructions christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 15:51 ` [Qemu-devel] [PATCH 8/8] target-arm: Fix VQRSHL Neon instructions (signed/unsigned 64 bits and signed 32 bits variants) christophe.lyon
2011-01-28 16:03 ` [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI Peter Maydell
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/translate.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3b14b8f..984df08 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4711,7 +4711,16 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
tcg_gen_add_i64(cpu_V0, cpu_V0, cpu_V1);
} else if (op == 4 || (op == 5 && u)) {
/* Insert */
- cpu_abort(env, "VS[LR]I.64 not implemented");
+ neon_load_reg64(cpu_V1, rd + pass);
+ uint64_t mask;
+ if (op == 4) {
+ mask = 0xffffffffffffffffull >> -shift;
+ } else {
+ mask = 0xffffffffffffffffull << shift;
+ }
+ tcg_gen_andi_i64(cpu_V0, cpu_V0, mask);
+ tcg_gen_andi_i64(cpu_V1, cpu_V1, ~mask);
+ tcg_gen_or_i64(cpu_V0, cpu_V0, cpu_V1);
}
neon_store_reg64(cpu_V0, rd + pass);
} else { /* size < 3 */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 8/8] target-arm: Fix VQRSHL Neon instructions (signed/unsigned 64 bits and signed 32 bits variants).
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (6 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 7/8] implement vsli.64, vsri.64 christophe.lyon
@ 2011-01-28 15:51 ` christophe.lyon
2011-01-28 16:03 ` [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI Peter Maydell
8 siblings, 0 replies; 21+ messages in thread
From: christophe.lyon @ 2011-01-28 15:51 UTC (permalink / raw)
To: qemu-devel
From: Christophe Lyon <christophe.lyon@st.com>
The addition of the rounding constant could cause overflows.
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
target-arm/neon_helper.c | 50 ++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 3337c52..9faa348 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -847,7 +847,23 @@ 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;
+ uint64_t round = (uint64_t)1 << (-1 - shift);
+ /* Reduce the range as long as the addition overflows. It's
+ * sufficient to check if (val+round) is < val
+ * because val and round are > 0. */
+ while (((val + round) < val) && round > 1) {
+ shift++;
+ round >>= 1;
+ val >>= 1;
+ }
+ if ((val + round) < val) {
+ /* If addition still overflows at this point, it means
+ * that round==1, thus shift==-1, and also that
+ * val==0x&FFFFFFFFFFFFFFF. */
+ val = 0x8000000000000000LL;
+ } else {
+ val = (val + round) >> -shift;
+ }
} else { \
uint64_t tmp = val;
val <<= shift;
@@ -859,11 +875,15 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
return val;
}
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator, which is really needed only when
+ * dealing with 32 bits input values. */
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
if (tmp < 0) { \
- dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
+ int64_t big_dest = ((int64_t)src1 + (1 << (-1 - tmp))); \
+ dest = big_dest >> -tmp; \
} else { \
dest = src1 << tmp; \
if ((dest >> tmp) != src1) { \
@@ -876,19 +896,41 @@ NEON_VOP_ENV(qrshl_s16, neon_s16, 2)
NEON_VOP_ENV(qrshl_s32, neon_s32, 1)
#undef NEON_FN
+/* 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;
+ int64_t round = (int64_t)1 << (-1 - shift);
+ /* Reduce the range as long as the addition overflows. It's
+ * sufficient to check if (val+round) is < 0 and val > 0
+ * because round is > 0. */
+ while ((val > 0) && ((val + round) < 0) && round > 1) {
+ shift++;
+ round >>= 1;
+ val >>= 1;
+ }
+ if ((val > 0) && (val + round) < 0) {
+ /* If addition still overflows at this point, it means
+ * that round==1, thus shift==-1, and also that
+ * val==0x7FFFFFFFFFFFFFFF. */
+ val = 0x4000000000000000LL;
+ } else {
+ val = (val + round) >> -shift;
+ }
} else {
int64_t tmp = val;
val <<= shift;
if ((val >> shift) != tmp) {
SET_QC();
- val = tmp >> 31;
+ if (tmp < 0) {
+ val = INT64_MIN;
+ } else {
+ val = INT64_MAX;
+ }
}
}
return val;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI
2011-01-28 15:50 [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI christophe.lyon
` (7 preceding siblings ...)
2011-01-28 15:51 ` [Qemu-devel] [PATCH 8/8] target-arm: Fix VQRSHL Neon instructions (signed/unsigned 64 bits and signed 32 bits variants) christophe.lyon
@ 2011-01-28 16:03 ` Peter Maydell
8 siblings, 0 replies; 21+ messages in thread
From: Peter Maydell @ 2011-01-28 16:03 UTC (permalink / raw)
To: christophe.lyon; +Cc: qemu-devel
On 28 January 2011 15:50, <christophe.lyon@st.com> wrote:
> Peter Maydell (4):
> Create and use neon_unarrow_sat* helpers
> VQRSHRN related changes
> fiddle decoding of 64 bit shift by imm and narrow
> implement vsli.64, vsri.64
These aren't actually by me, you're just importing the
patches from that tree where I inadvertantly added
signed-off tags I didn't really intend to.
I'll probably review these next week.
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread