[PATCH v2 07/13] target/riscv: Properly check SEW in amo_op

Richard Henderson posted 13 patches 1 month, 2 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Laurent Vivier <laurent@vivier.eu>, Palmer Dabbelt <palmer@dabbelt.com>

[PATCH v2 07/13] target/riscv: Properly check SEW in amo_op

Posted by Richard Henderson 1 month, 2 weeks ago
We're currently assuming SEW <= 3, and the "else" from
the SEW == 3 must be less.  Use a switch and explicitly
bound both SEW and SEQ for all cases.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 26 +++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index bbc5c93ef1..91fca4a2d1 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -704,18 +704,20 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
         gen_helper_exit_atomic(cpu_env);
         s->base.is_jmp = DISAS_NORETURN;
         return true;
-    } else {
-        if (s->sew == 3) {
-            if (!is_32bit(s)) {
-                fn = fnsd[seq];
-            } else {
-                /* Check done in amo_check(). */
-                g_assert_not_reached();
-            }
-        } else {
-            assert(seq < ARRAY_SIZE(fnsw));
-            fn = fnsw[seq];
-        }
+    }
+
+    switch (s->sew) {
+    case 0 ... 2:
+        assert(seq < ARRAY_SIZE(fnsw));
+        fn = fnsw[seq];
+        break;
+    case 3:
+        /* XLEN check done in amo_check(). */
+        assert(seq < ARRAY_SIZE(fnsd));
+        fn = fnsd[seq];
+        break;
+    default:
+        g_assert_not_reached();
     }
 
     data = FIELD_DP32(data, VDATA, MLEN, s->mlen);
-- 
2.25.1


Re: [PATCH v2 07/13] target/riscv: Properly check SEW in amo_op

Posted by Alistair Francis 1 month, 1 week ago
On Thu, Oct 14, 2021 at 6:55 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We're currently assuming SEW <= 3, and the "else" from
> the SEW == 3 must be less.  Use a switch and explicitly
> bound both SEW and SEQ for all cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 26 +++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index bbc5c93ef1..91fca4a2d1 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -704,18 +704,20 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
>          gen_helper_exit_atomic(cpu_env);
>          s->base.is_jmp = DISAS_NORETURN;
>          return true;
> -    } else {
> -        if (s->sew == 3) {
> -            if (!is_32bit(s)) {
> -                fn = fnsd[seq];
> -            } else {
> -                /* Check done in amo_check(). */
> -                g_assert_not_reached();
> -            }
> -        } else {
> -            assert(seq < ARRAY_SIZE(fnsw));
> -            fn = fnsw[seq];
> -        }
> +    }
> +
> +    switch (s->sew) {
> +    case 0 ... 2:
> +        assert(seq < ARRAY_SIZE(fnsw));
> +        fn = fnsw[seq];
> +        break;
> +    case 3:
> +        /* XLEN check done in amo_check(). */
> +        assert(seq < ARRAY_SIZE(fnsd));
> +        fn = fnsd[seq];
> +        break;
> +    default:
> +        g_assert_not_reached();
>      }
>
>      data = FIELD_DP32(data, VDATA, MLEN, s->mlen);
> --
> 2.25.1
>
>

Re: [PATCH v2 07/13] target/riscv: Properly check SEW in amo_op

Posted by LIU Zhiwei 1 month, 1 week ago
On 2021/10/14 上午4:50, Richard Henderson wrote:
> We're currently assuming SEW <= 3, and the "else" from
> the SEW == 3 must be less.  Use a switch and explicitly
> bound both SEW and SEQ for all cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: LIU Zhiwei<zhiwei_liu@c-sky.com>

Zhiwei
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 26 +++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index bbc5c93ef1..91fca4a2d1 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -704,18 +704,20 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
>           gen_helper_exit_atomic(cpu_env);
>           s->base.is_jmp = DISAS_NORETURN;
>           return true;
> -    } else {
> -        if (s->sew == 3) {
> -            if (!is_32bit(s)) {
> -                fn = fnsd[seq];
> -            } else {
> -                /* Check done in amo_check(). */
> -                g_assert_not_reached();
> -            }
> -        } else {
> -            assert(seq < ARRAY_SIZE(fnsw));
> -            fn = fnsw[seq];
> -        }
> +    }
> +
> +    switch (s->sew) {
> +    case 0 ... 2:
> +        assert(seq < ARRAY_SIZE(fnsw));
> +        fn = fnsw[seq];
> +        break;
> +    case 3:
> +        /* XLEN check done in amo_check(). */
> +        assert(seq < ARRAY_SIZE(fnsd));
> +        fn = fnsd[seq];
> +        break;
> +    default:
> +        g_assert_not_reached();
>       }
>   
>       data = FIELD_DP32(data, VDATA, MLEN, s->mlen);