[Qemu-devel] [PATCH 0/4] target/arm sve fixes

[Qemu-devel] [PATCH 0/4] target/arm sve fixes

[Richard Henderson]

These four patches are minor, reported by Laurent this week.

If there happens to be an -rc4 release, it would be nice if
they were included.  But if not, no biggie.  I suspect that
other minor issues will be found past these four, so I would
hope everyone who cares about sve is just as happy working
from a branch anyway.


r~


Richard Henderson (4):
  target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw
  target/arm: Fix typo in do_sat_addsub_64
  target/arm: Reorganize SVE WHILE
  target/arm: Fix typo in helper_sve_movz_d

 target/arm/sve_helper.c    | 19 ++++++--------
 target/arm/translate-sve.c | 51 ++++++++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 30 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/4] target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw

[Richard Henderson]

The normal vector element is sign-extended before
comparing with the wide vector element.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/sve_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 54795c9194..9bd0694d55 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -2436,13 +2436,13 @@ uint32_t HELPER(NAME)(void *vd, void *vn, void *vm, void *vg, uint32_t desc) \
 #define DO_CMP_PPZW_S(NAME, TYPE, TYPEW, OP) \
     DO_CMP_PPZW(NAME, TYPE, TYPEW, OP, H1_4, 0x1111111111111111ull)
 
-DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, uint8_t,  uint64_t, ==)
-DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, uint16_t, uint64_t, ==)
-DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, uint32_t, uint64_t, ==)
+DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, int8_t,  uint64_t, ==)
+DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, int16_t, uint64_t, ==)
+DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, int32_t, uint64_t, ==)
 
-DO_CMP_PPZW_B(sve_cmpne_ppzw_b, uint8_t,  uint64_t, !=)
-DO_CMP_PPZW_H(sve_cmpne_ppzw_h, uint16_t, uint64_t, !=)
-DO_CMP_PPZW_S(sve_cmpne_ppzw_s, uint32_t, uint64_t, !=)
+DO_CMP_PPZW_B(sve_cmpne_ppzw_b, int8_t,  uint64_t, !=)
+DO_CMP_PPZW_H(sve_cmpne_ppzw_h, int16_t, uint64_t, !=)
+DO_CMP_PPZW_S(sve_cmpne_ppzw_s, int32_t, uint64_t, !=)
 
 DO_CMP_PPZW_B(sve_cmpgt_ppzw_b, int8_t,   int64_t, >)
 DO_CMP_PPZW_H(sve_cmpgt_ppzw_h, int16_t,  int64_t, >)
-- 
2.17.1

[Qemu-devel] [PATCH 2/4] target/arm: Fix typo in do_sat_addsub_64

[Richard Henderson]

Used the wrong temporary in the computation of subtractive overflow.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-sve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 374051cd20..9dd4c38bab 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -1625,7 +1625,7 @@ static void do_sat_addsub_64(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
             /* Detect signed overflow for subtraction.  */
             tcg_gen_xor_i64(t0, reg, val);
             tcg_gen_sub_i64(t1, reg, val);
-            tcg_gen_xor_i64(reg, reg, t0);
+            tcg_gen_xor_i64(reg, reg, t1);
             tcg_gen_and_i64(t0, t0, reg);
 
             /* Bound the result.  */
-- 
2.17.1

[Qemu-devel] [PATCH 3/4] target/arm: Reorganize SVE WHILE

[Richard Henderson]

The pseudocode for this operation is an increment + compare loop,
so comparing <= the maximum integer produces an all-true predicate.

Rather than bound in both the inline code and the helper, pass the
helper the number of predicate bits to set instead of the number
of predicate elements to set.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/sve_helper.c    |  5 ----
 target/arm/translate-sve.c | 49 +++++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 9bd0694d55..87594a8adb 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -2846,11 +2846,6 @@ uint32_t HELPER(sve_while)(void *vd, uint32_t count, uint32_t pred_desc)
         return flags;
     }
 
-    /* Scale from predicate element count to bits.  */
-    count <<= esz;
-    /* Bound to the bits in the predicate.  */
-    count = MIN(count, oprsz * 8);
-
     /* Set all of the requested bits.  */
     for (i = 0; i < count / 64; ++i) {
         d->p[i] = esz_mask;
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 9dd4c38bab..89efc80ee7 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -3173,19 +3173,19 @@ static bool trans_CTERM(DisasContext *s, arg_CTERM *a, uint32_t insn)
 
 static bool trans_WHILE(DisasContext *s, arg_WHILE *a, uint32_t insn)
 {
-    if (!sve_access_check(s)) {
-        return true;
-    }
-
-    TCGv_i64 op0 = read_cpu_reg(s, a->rn, 1);
-    TCGv_i64 op1 = read_cpu_reg(s, a->rm, 1);
-    TCGv_i64 t0 = tcg_temp_new_i64();
-    TCGv_i64 t1 = tcg_temp_new_i64();
+    TCGv_i64 op0, op1, t0, t1, tmax;
     TCGv_i32 t2, t3;
     TCGv_ptr ptr;
     unsigned desc, vsz = vec_full_reg_size(s);
     TCGCond cond;
 
+    if (!sve_access_check(s)) {
+        return true;
+    }
+
+    op0 = read_cpu_reg(s, a->rn, 1);
+    op1 = read_cpu_reg(s, a->rm, 1);
+
     if (!a->sf) {
         if (a->u) {
             tcg_gen_ext32u_i64(op0, op0);
@@ -3198,32 +3198,47 @@ static bool trans_WHILE(DisasContext *s, arg_WHILE *a, uint32_t insn)
 
     /* For the helper, compress the different conditions into a computation
      * of how many iterations for which the condition is true.
-     *
-     * This is slightly complicated by 0 <= UINT64_MAX, which is nominally
-     * 2**64 iterations, overflowing to 0.  Of course, predicate registers
-     * aren't that large, so any value >= predicate size is sufficient.
      */
+    t0 = tcg_temp_new_i64();
+    t1 = tcg_temp_new_i64();
     tcg_gen_sub_i64(t0, op1, op0);
 
-    /* t0 = MIN(op1 - op0, vsz).  */
-    tcg_gen_movi_i64(t1, vsz);
-    tcg_gen_umin_i64(t0, t0, t1);
+    tmax = tcg_const_i64(vsz >> a->esz);
     if (a->eq) {
         /* Equality means one more iteration.  */
         tcg_gen_addi_i64(t0, t0, 1);
+
+        /* If op1 is max (un)signed integer (and the only time the addition
+         * above could overflow), then we produce an all-true predicate by
+         * setting the count to the vector length.  This is because the
+         * pseudocode is described as an increment + compare loop, and the
+         * max integer would always compare true.
+         */
+        tcg_gen_movi_i64(t1, (a->sf
+                              ? (a->u ? UINT64_MAX : INT64_MAX)
+                              : (a->u ? UINT32_MAX : INT32_MAX)));
+        tcg_gen_movcond_i64(TCG_COND_EQ, t0, op1, t1, tmax, t0);
     }
 
-    /* t0 = (condition true ? t0 : 0).  */
+    /* Bound to the maximum.  */
+    tcg_gen_umin_i64(t0, t0, tmax);
+    tcg_temp_free_i64(tmax);
+
+    /* Set the count to zero if the condition is false.  */
     cond = (a->u
             ? (a->eq ? TCG_COND_LEU : TCG_COND_LTU)
             : (a->eq ? TCG_COND_LE : TCG_COND_LT));
     tcg_gen_movi_i64(t1, 0);
     tcg_gen_movcond_i64(cond, t0, op0, op1, t0, t1);
+    tcg_temp_free_i64(t1);
 
+    /* Since we're bounded, pass as a 32-bit type.  */
     t2 = tcg_temp_new_i32();
     tcg_gen_extrl_i64_i32(t2, t0);
     tcg_temp_free_i64(t0);
-    tcg_temp_free_i64(t1);
+
+    /* Scale elements to bits.  */
+    tcg_gen_shli_i32(t2, t2, a->esz);
 
     desc = (vsz / 8) - 2;
     desc = deposit32(desc, SIMD_DATA_SHIFT, 2, a->esz);
-- 
2.17.1

[Qemu-devel] [PATCH 4/4] target/arm: Fix typo in helper_sve_movz_d

[Richard Henderson]

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/sve_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 87594a8adb..c3cbec9cf5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -1042,7 +1042,7 @@ void HELPER(sve_movz_d)(void *vd, void *vn, void *vg, uint32_t desc)
     uint64_t *d = vd, *n = vn;
     uint8_t *pg = vg;
     for (i = 0; i < opr_sz; i += 1) {
-        d[i] = n[1] & -(uint64_t)(pg[H1(i)] & 1);
+        d[i] = n[i] & -(uint64_t)(pg[H1(i)] & 1);
     }
 }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/4] target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw

[Laurent Desnogues]

On Wed, Aug 1, 2018 at 2:31 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> The normal vector element is sign-extended before
> comparing with the wide vector element.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>


Laurent

> ---
>  target/arm/sve_helper.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 54795c9194..9bd0694d55 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2436,13 +2436,13 @@ uint32_t HELPER(NAME)(void *vd, void *vn, void *vm, void *vg, uint32_t desc) \
>  #define DO_CMP_PPZW_S(NAME, TYPE, TYPEW, OP) \
>      DO_CMP_PPZW(NAME, TYPE, TYPEW, OP, H1_4, 0x1111111111111111ull)
>
> -DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, uint8_t,  uint64_t, ==)
> -DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, uint16_t, uint64_t, ==)
> -DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, uint32_t, uint64_t, ==)
> +DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, int8_t,  uint64_t, ==)
> +DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, int16_t, uint64_t, ==)
> +DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, int32_t, uint64_t, ==)
>
> -DO_CMP_PPZW_B(sve_cmpne_ppzw_b, uint8_t,  uint64_t, !=)
> -DO_CMP_PPZW_H(sve_cmpne_ppzw_h, uint16_t, uint64_t, !=)
> -DO_CMP_PPZW_S(sve_cmpne_ppzw_s, uint32_t, uint64_t, !=)
> +DO_CMP_PPZW_B(sve_cmpne_ppzw_b, int8_t,  uint64_t, !=)
> +DO_CMP_PPZW_H(sve_cmpne_ppzw_h, int16_t, uint64_t, !=)
> +DO_CMP_PPZW_S(sve_cmpne_ppzw_s, int32_t, uint64_t, !=)
>
>  DO_CMP_PPZW_B(sve_cmpgt_ppzw_b, int8_t,   int64_t, >)
>  DO_CMP_PPZW_H(sve_cmpgt_ppzw_h, int16_t,  int64_t, >)
> --
> 2.17.1
>

Re: [Qemu-devel] [PATCH 4/4] target/arm: Fix typo in helper_sve_movz_d

[Laurent Desnogues]

On Wed, Aug 1, 2018 at 2:31 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>


Laurent

> ---
>  target/arm/sve_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 87594a8adb..c3cbec9cf5 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1042,7 +1042,7 @@ void HELPER(sve_movz_d)(void *vd, void *vn, void *vg, uint32_t desc)
>      uint64_t *d = vd, *n = vn;
>      uint8_t *pg = vg;
>      for (i = 0; i < opr_sz; i += 1) {
> -        d[i] = n[1] & -(uint64_t)(pg[H1(i)] & 1);
> +        d[i] = n[i] & -(uint64_t)(pg[H1(i)] & 1);
>      }
>  }
>
> --
> 2.17.1
>

Re: [Qemu-devel] [PATCH 3/4] target/arm: Reorganize SVE WHILE

[Laurent Desnogues]

On Wed, Aug 1, 2018 at 2:31 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> The pseudocode for this operation is an increment + compare loop,
> so comparing <= the maximum integer produces an all-true predicate.
>
> Rather than bound in both the inline code and the helper, pass the
> helper the number of predicate bits to set instead of the number
> of predicate elements to set.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>


Laurent

> ---
>  target/arm/sve_helper.c    |  5 ----
>  target/arm/translate-sve.c | 49 +++++++++++++++++++++++++-------------
>  2 files changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 9bd0694d55..87594a8adb 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2846,11 +2846,6 @@ uint32_t HELPER(sve_while)(void *vd, uint32_t count, uint32_t pred_desc)
>          return flags;
>      }
>
> -    /* Scale from predicate element count to bits.  */
> -    count <<= esz;
> -    /* Bound to the bits in the predicate.  */
> -    count = MIN(count, oprsz * 8);
> -
>      /* Set all of the requested bits.  */
>      for (i = 0; i < count / 64; ++i) {
>          d->p[i] = esz_mask;
> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index 9dd4c38bab..89efc80ee7 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -3173,19 +3173,19 @@ static bool trans_CTERM(DisasContext *s, arg_CTERM *a, uint32_t insn)
>
>  static bool trans_WHILE(DisasContext *s, arg_WHILE *a, uint32_t insn)
>  {
> -    if (!sve_access_check(s)) {
> -        return true;
> -    }
> -
> -    TCGv_i64 op0 = read_cpu_reg(s, a->rn, 1);
> -    TCGv_i64 op1 = read_cpu_reg(s, a->rm, 1);
> -    TCGv_i64 t0 = tcg_temp_new_i64();
> -    TCGv_i64 t1 = tcg_temp_new_i64();
> +    TCGv_i64 op0, op1, t0, t1, tmax;
>      TCGv_i32 t2, t3;
>      TCGv_ptr ptr;
>      unsigned desc, vsz = vec_full_reg_size(s);
>      TCGCond cond;
>
> +    if (!sve_access_check(s)) {
> +        return true;
> +    }
> +
> +    op0 = read_cpu_reg(s, a->rn, 1);
> +    op1 = read_cpu_reg(s, a->rm, 1);
> +
>      if (!a->sf) {
>          if (a->u) {
>              tcg_gen_ext32u_i64(op0, op0);
> @@ -3198,32 +3198,47 @@ static bool trans_WHILE(DisasContext *s, arg_WHILE *a, uint32_t insn)
>
>      /* For the helper, compress the different conditions into a computation
>       * of how many iterations for which the condition is true.
> -     *
> -     * This is slightly complicated by 0 <= UINT64_MAX, which is nominally
> -     * 2**64 iterations, overflowing to 0.  Of course, predicate registers
> -     * aren't that large, so any value >= predicate size is sufficient.
>       */
> +    t0 = tcg_temp_new_i64();
> +    t1 = tcg_temp_new_i64();
>      tcg_gen_sub_i64(t0, op1, op0);
>
> -    /* t0 = MIN(op1 - op0, vsz).  */
> -    tcg_gen_movi_i64(t1, vsz);
> -    tcg_gen_umin_i64(t0, t0, t1);
> +    tmax = tcg_const_i64(vsz >> a->esz);
>      if (a->eq) {
>          /* Equality means one more iteration.  */
>          tcg_gen_addi_i64(t0, t0, 1);
> +
> +        /* If op1 is max (un)signed integer (and the only time the addition
> +         * above could overflow), then we produce an all-true predicate by
> +         * setting the count to the vector length.  This is because the
> +         * pseudocode is described as an increment + compare loop, and the
> +         * max integer would always compare true.
> +         */
> +        tcg_gen_movi_i64(t1, (a->sf
> +                              ? (a->u ? UINT64_MAX : INT64_MAX)
> +                              : (a->u ? UINT32_MAX : INT32_MAX)));
> +        tcg_gen_movcond_i64(TCG_COND_EQ, t0, op1, t1, tmax, t0);
>      }
>
> -    /* t0 = (condition true ? t0 : 0).  */
> +    /* Bound to the maximum.  */
> +    tcg_gen_umin_i64(t0, t0, tmax);
> +    tcg_temp_free_i64(tmax);
> +
> +    /* Set the count to zero if the condition is false.  */
>      cond = (a->u
>              ? (a->eq ? TCG_COND_LEU : TCG_COND_LTU)
>              : (a->eq ? TCG_COND_LE : TCG_COND_LT));
>      tcg_gen_movi_i64(t1, 0);
>      tcg_gen_movcond_i64(cond, t0, op0, op1, t0, t1);
> +    tcg_temp_free_i64(t1);
>
> +    /* Since we're bounded, pass as a 32-bit type.  */
>      t2 = tcg_temp_new_i32();
>      tcg_gen_extrl_i64_i32(t2, t0);
>      tcg_temp_free_i64(t0);
> -    tcg_temp_free_i64(t1);
> +
> +    /* Scale elements to bits.  */
> +    tcg_gen_shli_i32(t2, t2, a->esz);
>
>      desc = (vsz / 8) - 2;
>      desc = deposit32(desc, SIMD_DATA_SHIFT, 2, a->esz);
> --
> 2.17.1
>

Re: [Qemu-devel] [PATCH 2/4] target/arm: Fix typo in do_sat_addsub_64

[Laurent Desnogues]

On Wed, Aug 1, 2018 at 2:31 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Used the wrong temporary in the computation of subtractive overflow.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>


Laurent

> ---
>  target/arm/translate-sve.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index 374051cd20..9dd4c38bab 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -1625,7 +1625,7 @@ static void do_sat_addsub_64(TCGv_i64 reg, TCGv_i64 val, bool u, bool d)
>              /* Detect signed overflow for subtraction.  */
>              tcg_gen_xor_i64(t0, reg, val);
>              tcg_gen_sub_i64(t1, reg, val);
> -            tcg_gen_xor_i64(reg, reg, t0);
> +            tcg_gen_xor_i64(reg, reg, t1);
>              tcg_gen_and_i64(t0, t0, reg);
>
>              /* Bound the result.  */
> --
> 2.17.1
>

Re: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

[Laurent Desnogues]

On Wed, Aug 1, 2018 at 2:31 PM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> These four patches are minor, reported by Laurent this week.

This fixes the reported issues, thanks!

> If there happens to be an -rc4 release, it would be nice if
> they were included.  But if not, no biggie.  I suspect that
> other minor issues will be found past these four, so I would
> hope everyone who cares about sve is just as happy working
> from a branch anyway.

I indeed still have some things to look at.


Laurent

>
> r~
>
>
> Richard Henderson (4):
>   target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw
>   target/arm: Fix typo in do_sat_addsub_64
>   target/arm: Reorganize SVE WHILE
>   target/arm: Fix typo in helper_sve_movz_d
>
>  target/arm/sve_helper.c    | 19 ++++++--------
>  target/arm/translate-sve.c | 51 ++++++++++++++++++++++++--------------
>  2 files changed, 40 insertions(+), 30 deletions(-)
>
> --
> 2.17.1
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/4] target/arm: Fix typo in helper_sve_movz_d

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/sve_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 87594a8adb..c3cbec9cf5 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1042,7 +1042,7 @@ void HELPER(sve_movz_d)(void *vd, void *vn, void *vg, uint32_t desc)
>      uint64_t *d = vd, *n = vn;
>      uint8_t *pg = vg;
>      for (i = 0; i < opr_sz; i += 1) {
> -        d[i] = n[1] & -(uint64_t)(pg[H1(i)] & 1);
> +        d[i] = n[i] & -(uint64_t)(pg[H1(i)] & 1);
>      }
>  }


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/4] target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> The normal vector element is sign-extended before
> comparing with the wide vector element.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/sve_helper.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 54795c9194..9bd0694d55 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2436,13 +2436,13 @@ uint32_t HELPER(NAME)(void *vd, void *vn, void *vm, void *vg, uint32_t desc) \
>  #define DO_CMP_PPZW_S(NAME, TYPE, TYPEW, OP) \
>      DO_CMP_PPZW(NAME, TYPE, TYPEW, OP, H1_4, 0x1111111111111111ull)
>
> -DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, uint8_t,  uint64_t, ==)
> -DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, uint16_t, uint64_t, ==)
> -DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, uint32_t, uint64_t, ==)
> +DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, int8_t,  uint64_t, ==)
> +DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, int16_t, uint64_t, ==)
> +DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, int32_t, uint64_t, ==)
>
> -DO_CMP_PPZW_B(sve_cmpne_ppzw_b, uint8_t,  uint64_t, !=)
> -DO_CMP_PPZW_H(sve_cmpne_ppzw_h, uint16_t, uint64_t, !=)
> -DO_CMP_PPZW_S(sve_cmpne_ppzw_s, uint32_t, uint64_t, !=)
> +DO_CMP_PPZW_B(sve_cmpne_ppzw_b, int8_t,  uint64_t, !=)
> +DO_CMP_PPZW_H(sve_cmpne_ppzw_h, int16_t, uint64_t, !=)
> +DO_CMP_PPZW_S(sve_cmpne_ppzw_s, int32_t, uint64_t, !=)
>
>  DO_CMP_PPZW_B(sve_cmpgt_ppzw_b, int8_t,   int64_t, >)
>  DO_CMP_PPZW_H(sve_cmpgt_ppzw_h, int16_t,  int64_t, >)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> These four patches are minor, reported by Laurent this week.
>
> If there happens to be an -rc4 release, it would be nice if
> they were included.  But if not, no biggie.  I suspect that
> other minor issues will be found past these four, so I would
> hope everyone who cares about sve is just as happy working
> from a branch anyway.

All the RISU tests are still passing so:

Tested-by: Alex Bennée <alex.bennee@linaro.org>

>
>
> r~
>
>
> Richard Henderson (4):
>   target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw
>   target/arm: Fix typo in do_sat_addsub_64
>   target/arm: Reorganize SVE WHILE
>   target/arm: Fix typo in helper_sve_movz_d
>
>  target/arm/sve_helper.c    | 19 ++++++--------
>  target/arm/translate-sve.c | 51 ++++++++++++++++++++++++--------------
>  2 files changed, 40 insertions(+), 30 deletions(-)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180801123111.3595-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
316eefdbc8 target/arm: Fix typo in helper_sve_movz_d
4b115c0c0d target/arm: Reorganize SVE WHILE
b5f496f5e7 target/arm: Fix typo in do_sat_addsub_64
795c7bee8d target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw

=== OUTPUT BEGIN ===
Checking PATCH 1/4: target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw...
ERROR: spaces required around that '==' (ctx:WxB)
#31: FILE: target/arm/sve_helper.c:2439:
+DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, int8_t,  uint64_t, ==)
                                                    ^

ERROR: spaces required around that '==' (ctx:WxB)
#32: FILE: target/arm/sve_helper.c:2440:
+DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, int16_t, uint64_t, ==)
                                                    ^

ERROR: spaces required around that '==' (ctx:WxB)
#33: FILE: target/arm/sve_helper.c:2441:
+DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, int32_t, uint64_t, ==)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#38: FILE: target/arm/sve_helper.c:2443:
+DO_CMP_PPZW_B(sve_cmpne_ppzw_b, int8_t,  uint64_t, !=)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#39: FILE: target/arm/sve_helper.c:2444:
+DO_CMP_PPZW_H(sve_cmpne_ppzw_h, int16_t, uint64_t, !=)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#40: FILE: target/arm/sve_helper.c:2445:
+DO_CMP_PPZW_S(sve_cmpne_ppzw_s, int32_t, uint64_t, !=)
                                                    ^

total: 6 errors, 0 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/4: target/arm: Fix typo in do_sat_addsub_64...
Checking PATCH 3/4: target/arm: Reorganize SVE WHILE...
Checking PATCH 4/4: target/arm: Fix typo in helper_sve_movz_d...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/4] target/arm: Fix typo in helper_sve_movz_d

[Philippe Mathieu-Daudé]

On 08/01/2018 09:31 AM, Richard Henderson wrote:
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/sve_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 87594a8adb..c3cbec9cf5 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -1042,7 +1042,7 @@ void HELPER(sve_movz_d)(void *vd, void *vn, void *vg, uint32_t desc)
>      uint64_t *d = vd, *n = vn;
>      uint8_t *pg = vg;
>      for (i = 0; i < opr_sz; i += 1) {
> -        d[i] = n[1] & -(uint64_t)(pg[H1(i)] & 1);
> +        d[i] = n[i] & -(uint64_t)(pg[H1(i)] & 1);

:)))

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      }
>  }
>  
> 

Re: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180801123111.3595-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e4c5ee9260 target/arm: Fix typo in helper_sve_movz_d
9a37ac5908 target/arm: Reorganize SVE WHILE
c529b75fec target/arm: Fix typo in do_sat_addsub_64
fb72755faa target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw

=== OUTPUT BEGIN ===
Checking PATCH 1/4: target/arm: Fix sign of sve_cmpeq_ppzw/sve_cmpne_ppzw...
ERROR: spaces required around that '==' (ctx:WxB)
#31: FILE: target/arm/sve_helper.c:2439:
+DO_CMP_PPZW_B(sve_cmpeq_ppzw_b, int8_t,  uint64_t, ==)
                                                    ^

ERROR: spaces required around that '==' (ctx:WxB)
#32: FILE: target/arm/sve_helper.c:2440:
+DO_CMP_PPZW_H(sve_cmpeq_ppzw_h, int16_t, uint64_t, ==)
                                                    ^

ERROR: spaces required around that '==' (ctx:WxB)
#33: FILE: target/arm/sve_helper.c:2441:
+DO_CMP_PPZW_S(sve_cmpeq_ppzw_s, int32_t, uint64_t, ==)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#38: FILE: target/arm/sve_helper.c:2443:
+DO_CMP_PPZW_B(sve_cmpne_ppzw_b, int8_t,  uint64_t, !=)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#39: FILE: target/arm/sve_helper.c:2444:
+DO_CMP_PPZW_H(sve_cmpne_ppzw_h, int16_t, uint64_t, !=)
                                                    ^

ERROR: spaces required around that '!=' (ctx:WxB)
#40: FILE: target/arm/sve_helper.c:2445:
+DO_CMP_PPZW_S(sve_cmpne_ppzw_s, int32_t, uint64_t, !=)
                                                    ^

total: 6 errors, 0 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/4: target/arm: Fix typo in do_sat_addsub_64...
Checking PATCH 3/4: target/arm: Reorganize SVE WHILE...
Checking PATCH 4/4: target/arm: Fix typo in helper_sve_movz_d...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/4] target/arm sve fixes

[Peter Maydell]

On 1 August 2018 at 13:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
> These four patches are minor, reported by Laurent this week.
>
> If there happens to be an -rc4 release, it would be nice if
> they were included.  But if not, no biggie.  I suspect that
> other minor issues will be found past these four, so I would
> hope everyone who cares about sve is just as happy working
> from a branch anyway.

Applied to target-arm.for-3.1, thanks.

(I currently expect there will be an -rc4, but I only
want to put in fixes for regressions from 2.12.)

-- PMM