[Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI

[Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI

[christophe.lyon]

From: Christophe Lyon <christophe.lyon@st.com>

This patchset combines fixes from the Meego tree (Peter Maydell, Juha
Riihimäki) and my own fixes such that ARM Neon instructions VQMOVUN
VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI now pass all my
tests.

Christophe Lyon (3):
  Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL,
    VRSRA.
  target-arm: Fix Neon VQ(R)SHRN instructions.
  target-arm: Fix VQRSHL Neon instructions (signed/unsigned 64 bits and
        signed 32 bits variants).

Juha Riihimäki (1):
  target-arm: fix neon vqrshl instruction

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

 target-arm/helpers.h     |    3 +
 target-arm/neon_helper.c |  195 ++++++++++++++++++++++++++++++++++++++++++----
 target-arm/translate.c   |  103 ++++++++++++++++++-------
 3 files changed, 257 insertions(+), 44 deletions(-)

-- 
1.7.2.3

[Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[christophe.lyon]

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

[Qemu-devel] [PATCH 2/8] target-arm: Create and use neon_unarrow_sat* helpers

[christophe.lyon]

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

[Qemu-devel] [PATCH 3/8] target-arm: VQRSHRN related changes

[christophe.lyon]

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

[Qemu-devel] [PATCH 4/8] target-arm: fiddle decoding of 64 bit shift by imm and narrow

[christophe.lyon]

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

[Qemu-devel] [PATCH 5/8] target-arm: fix neon vqrshl instruction

[christophe.lyon]

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

[Qemu-devel] [PATCH 6/8] target-arm: Fix Neon VQ(R)SHRN instructions.

[christophe.lyon]

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

[Qemu-devel] [PATCH 7/8] implement vsli.64, vsri.64

[christophe.lyon]

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

[Qemu-devel] [PATCH 8/8] target-arm: Fix VQRSHL Neon instructions (signed/unsigned 64 bits and signed 32 bits variants).

[christophe.lyon]

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

Re: [Qemu-devel] [PATCH 0/8] target-arm: Fix Neon instructions VQMOVUN VQRSHL VQRSHRN VQRSHRUN VQSHRN VQSHRUN VSLI VSRI

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Aurelien Jarno]

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Christophe Lyon]

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.

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Aurelien Jarno]

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Alon Levy]

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
> 

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Christophe Lyon]

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.

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Aurelien Jarno]

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.

[Christophe Lyon]

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.