diff mbox series

riscv: do not set the rounding mode via `gen_set_rm`

Message ID 20221229172734.119600-1-abdulras@google.com
State New
Headers show
Series riscv: do not set the rounding mode via `gen_set_rm` | expand

Commit Message

Saleem Abdulrasool Dec. 29, 2022, 5:27 p.m. UTC
From: Saleem Abdulrasool <compnerd@compnerd.org>

Setting the rounding mode via the `gen_set_rm` call would alter the
state of the disassembler, resetting the `TransOp` in the assembler
context.  When we subsequently set the rounding mode to the desired
value, we would trigger an assertion in `decode_save_opc`.  Instead
we can set the rounding mode via the `gen_helper_set_rounding_mode`
which will still trigger the exception in the case of an invalid RM
without altering the CPU disassembler state.

Signed-off-by: Saleem Abdulrasool <compnerd@compnerd.org>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 69 +++++++++++++------------
 1 file changed, 36 insertions(+), 33 deletions(-)

Comments

Bin Meng Jan. 8, 2023, 3:52 a.m. UTC | #1
On Fri, Dec 30, 2022 at 2:20 AM Saleem Abdulrasool <abdulras@google.com> wrote:
>
> From: Saleem Abdulrasool <compnerd@compnerd.org>
>
> Setting the rounding mode via the `gen_set_rm` call would alter the
> state of the disassembler, resetting the `TransOp` in the assembler
> context.  When we subsequently set the rounding mode to the desired
> value, we would trigger an assertion in `decode_save_opc`.  Instead
> we can set the rounding mode via the `gen_helper_set_rounding_mode`
> which will still trigger the exception in the case of an invalid RM
> without altering the CPU disassembler state.
>
> Signed-off-by: Saleem Abdulrasool <compnerd@compnerd.org>
> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 69 +++++++++++++------------
>  1 file changed, 36 insertions(+), 33 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 4dea4413ae..73f6fab1c5 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -2679,8 +2679,10 @@ static bool do_opfv(DisasContext *s, arg_rmr *a,
>                      int rm)
>  {
>      if (checkfn(s, a)) {
> +        // the helper will raise an exception if the rounding mode is invalid

nits: use /* */

>          if (rm != RISCV_FRM_DYN) {
> -            gen_set_rm(s, RISCV_FRM_DYN);
> +            gen_helper_set_rounding_mode(cpu_env,
> +                                         tcg_constant_i32(RISCV_FRM_DYN));
>          }
>
>          uint32_t data = 0;
> @@ -3001,38 +3003,39 @@ static bool opffv_narrow_check(DisasContext *s, arg_rmr *a)
>             require_scale_zve64f(s);
>  }
>
> -#define GEN_OPFV_NARROW_TRANS(NAME, CHECK, HELPER, FRM)            \
> -static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
> -{                                                                  \
> -    if (CHECK(s, a)) {                                             \
> -        if (FRM != RISCV_FRM_DYN) {                                \
> -            gen_set_rm(s, RISCV_FRM_DYN);                          \
> -        }                                                          \
> -                                                                   \
> -        uint32_t data = 0;                                         \
> -        static gen_helper_gvec_3_ptr * const fns[2] = {            \
> -            gen_helper_##HELPER##_h,                               \
> -            gen_helper_##HELPER##_w,                               \
> -        };                                                         \
> -        TCGLabel *over = gen_new_label();                          \
> -        gen_set_rm(s, FRM);                                        \
> -        tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);          \
> -        tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
> -                                                                   \
> -        data = FIELD_DP32(data, VDATA, VM, a->vm);                 \
> -        data = FIELD_DP32(data, VDATA, LMUL, s->lmul);             \
> -        data = FIELD_DP32(data, VDATA, VTA, s->vta);               \
> -        data = FIELD_DP32(data, VDATA, VMA, s->vma);               \
> -        tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),     \
> -                           vreg_ofs(s, a->rs2), cpu_env,           \
> -                           s->cfg_ptr->vlen / 8,                   \
> -                           s->cfg_ptr->vlen / 8, data,             \
> -                           fns[s->sew - 1]);                       \
> -        mark_vs_dirty(s);                                          \
> -        gen_set_label(over);                                       \
> -        return true;                                               \
> -    }                                                              \
> -    return false;                                                  \
> +#define GEN_OPFV_NARROW_TRANS(NAME, CHECK, HELPER, FRM)                     \
> +static bool trans_##NAME(DisasContext *s, arg_rmr *a)                       \
> +{                                                                           \
> +    if (CHECK(s, a)) {                                                      \
> +        if (FRM != RISCV_FRM_DYN) {                                         \
> +            gen_helper_set_rounding_mode(cpu_env,                           \
> +                                         tcg_constant_i32(RISCV_FRM_DYN));  \
> +        }                                                                   \
> +                                                                            \
> +        uint32_t data = 0;                                                  \
> +        static gen_helper_gvec_3_ptr * const fns[2] = {                     \
> +            gen_helper_##HELPER##_h,                                        \
> +            gen_helper_##HELPER##_w,                                        \
> +        };                                                                  \
> +        TCGLabel *over = gen_new_label();                                   \
> +        gen_set_rm(s, FRM);                                                 \
> +        tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);                   \
> +        tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);          \
> +                                                                            \
> +        data = FIELD_DP32(data, VDATA, VM, a->vm);                          \
> +        data = FIELD_DP32(data, VDATA, LMUL, s->lmul);                      \
> +        data = FIELD_DP32(data, VDATA, VTA, s->vta);                        \
> +        data = FIELD_DP32(data, VDATA, VMA, s->vma);                        \
> +        tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),              \
> +                           vreg_ofs(s, a->rs2), cpu_env,                    \
> +                           s->cfg_ptr->vlen / 8,                            \
> +                           s->cfg_ptr->vlen / 8, data,                      \
> +                           fns[s->sew - 1]);                                \
> +        mark_vs_dirty(s);                                                   \
> +        gen_set_label(over);                                                \
> +        return true;                                                        \
> +    }                                                                       \
> +    return false;                                                           \
>  }
>
>  GEN_OPFV_NARROW_TRANS(vfncvt_f_xu_w, opfxv_narrow_check, vfncvt_f_xu_w,
> --

Is it possible to create a test case (tests/tcg/riscv64) for this?

Regards,
Bin
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 4dea4413ae..73f6fab1c5 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2679,8 +2679,10 @@  static bool do_opfv(DisasContext *s, arg_rmr *a,
                     int rm)
 {
     if (checkfn(s, a)) {
+        // the helper will raise an exception if the rounding mode is invalid
         if (rm != RISCV_FRM_DYN) {
-            gen_set_rm(s, RISCV_FRM_DYN);
+            gen_helper_set_rounding_mode(cpu_env,
+                                         tcg_constant_i32(RISCV_FRM_DYN));
         }
 
         uint32_t data = 0;
@@ -3001,38 +3003,39 @@  static bool opffv_narrow_check(DisasContext *s, arg_rmr *a)
            require_scale_zve64f(s);
 }
 
-#define GEN_OPFV_NARROW_TRANS(NAME, CHECK, HELPER, FRM)            \
-static bool trans_##NAME(DisasContext *s, arg_rmr *a)              \
-{                                                                  \
-    if (CHECK(s, a)) {                                             \
-        if (FRM != RISCV_FRM_DYN) {                                \
-            gen_set_rm(s, RISCV_FRM_DYN);                          \
-        }                                                          \
-                                                                   \
-        uint32_t data = 0;                                         \
-        static gen_helper_gvec_3_ptr * const fns[2] = {            \
-            gen_helper_##HELPER##_h,                               \
-            gen_helper_##HELPER##_w,                               \
-        };                                                         \
-        TCGLabel *over = gen_new_label();                          \
-        gen_set_rm(s, FRM);                                        \
-        tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);          \
-        tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
-                                                                   \
-        data = FIELD_DP32(data, VDATA, VM, a->vm);                 \
-        data = FIELD_DP32(data, VDATA, LMUL, s->lmul);             \
-        data = FIELD_DP32(data, VDATA, VTA, s->vta);               \
-        data = FIELD_DP32(data, VDATA, VMA, s->vma);               \
-        tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),     \
-                           vreg_ofs(s, a->rs2), cpu_env,           \
-                           s->cfg_ptr->vlen / 8,                   \
-                           s->cfg_ptr->vlen / 8, data,             \
-                           fns[s->sew - 1]);                       \
-        mark_vs_dirty(s);                                          \
-        gen_set_label(over);                                       \
-        return true;                                               \
-    }                                                              \
-    return false;                                                  \
+#define GEN_OPFV_NARROW_TRANS(NAME, CHECK, HELPER, FRM)                     \
+static bool trans_##NAME(DisasContext *s, arg_rmr *a)                       \
+{                                                                           \
+    if (CHECK(s, a)) {                                                      \
+        if (FRM != RISCV_FRM_DYN) {                                         \
+            gen_helper_set_rounding_mode(cpu_env,                           \
+                                         tcg_constant_i32(RISCV_FRM_DYN));  \
+        }                                                                   \
+                                                                            \
+        uint32_t data = 0;                                                  \
+        static gen_helper_gvec_3_ptr * const fns[2] = {                     \
+            gen_helper_##HELPER##_h,                                        \
+            gen_helper_##HELPER##_w,                                        \
+        };                                                                  \
+        TCGLabel *over = gen_new_label();                                   \
+        gen_set_rm(s, FRM);                                                 \
+        tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);                   \
+        tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);          \
+                                                                            \
+        data = FIELD_DP32(data, VDATA, VM, a->vm);                          \
+        data = FIELD_DP32(data, VDATA, LMUL, s->lmul);                      \
+        data = FIELD_DP32(data, VDATA, VTA, s->vta);                        \
+        data = FIELD_DP32(data, VDATA, VMA, s->vma);                        \
+        tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),              \
+                           vreg_ofs(s, a->rs2), cpu_env,                    \
+                           s->cfg_ptr->vlen / 8,                            \
+                           s->cfg_ptr->vlen / 8, data,                      \
+                           fns[s->sew - 1]);                                \
+        mark_vs_dirty(s);                                                   \
+        gen_set_label(over);                                                \
+        return true;                                                        \
+    }                                                                       \
+    return false;                                                           \
 }
 
 GEN_OPFV_NARROW_TRANS(vfncvt_f_xu_w, opfxv_narrow_check, vfncvt_f_xu_w,