Re: [PATCH] riscv: do not set the rounding mode via `gen_set_rm`

2023-01-07 Thread Bin Meng
On Fri, Dec 30, 2022 at 2:20 AM Saleem Abdulrasool  wrote:
>
> From: Saleem Abdulrasool 
>
> 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 
> ---
>  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,\
> +};

[PATCH] riscv: do not set the rounding mode via `gen_set_rm`

2022-12-29 Thread Saleem Abdulrasool
From: Saleem Abdulrasool 

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 
---
 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
 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, 

Re: [PATCH] riscv: do not set the rounding mode via `gen_set_rm`

2022-12-29 Thread Philippe Mathieu-Daudé

On 29/12/22 18:37, Saleem Abdulrasool wrote:

From: Saleem Abdulrasool 

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 
---
  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
  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);  

[PATCH] riscv: do not set the rounding mode via `gen_set_rm`

2022-12-29 Thread Saleem Abdulrasool
From: Saleem Abdulrasool 

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 
---
 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
 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,