[PATCH v2 1/3] target/riscv: Remove redundant insn length check for zama16b

LIU Zhiwei posted 3 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 1/3] target/riscv: Remove redundant insn length check for zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago
Compressed encodings also applies to zama16b.
https://github.com/riscv/riscv-isa-manual/pull/1557

Suggested-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 target/riscv/insn_trans/trans_rvd.c.inc | 9 +++++++--
 target/riscv/insn_trans/trans_rvf.c.inc | 4 ++--
 target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 1f5fac65a2..2be037930a 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -47,7 +47,12 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVD);
 
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    /*
+     * Zama16b applies to loads and stores of no more than MXLEN bits defined
+     * in the F, D, and Q extensions. Otherwise, it falls through to default
+     * MO_ATOM_IFALIGN.
+     */
+    if ((ctx->xl >= MXL_RV64) && (ctx->cfg_ptr->ext_zama16b)) {
         memop |= MO_ATOM_WITHIN16;
     }
 
@@ -67,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVD);
 
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
 
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index f771aa1939..0222a728df 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -48,7 +48,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVF);
 
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
 
@@ -70,7 +70,7 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVF);
 
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
 
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 98e3806d5e..fab5c06719 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -268,7 +268,7 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
 {
     bool out;
 
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
     decode_save_opc(ctx);
@@ -369,7 +369,7 @@ static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop)
 
 static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
 {
-    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
+    if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
     decode_save_opc(ctx);
-- 
2.25.1
Re: [PATCH v2 1/3] target/riscv: Remove redundant insn length check for zama16b
Posted by Richard Henderson 3 months, 3 weeks ago
On 8/2/24 13:16, LIU Zhiwei wrote:
> @@ -47,7 +47,12 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>       REQUIRE_FPU;
>       REQUIRE_EXT(ctx, RVD);
>   
> -    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
> +    /*
> +     * Zama16b applies to loads and stores of no more than MXLEN bits defined
> +     * in the F, D, and Q extensions. Otherwise, it falls through to default
> +     * MO_ATOM_IFALIGN.
> +     */
> +    if ((ctx->xl >= MXL_RV64) && (ctx->cfg_ptr->ext_zama16b)) {

I think you meant to add the mxlen check in the next patch,
because you modify this line again.


r~
Re: [PATCH v2 1/3] target/riscv: Remove redundant insn length check for zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago
On 2024/8/2 13:38, Richard Henderson wrote:
> On 8/2/24 13:16, LIU Zhiwei wrote:
>> @@ -47,7 +47,12 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>>       REQUIRE_FPU;
>>       REQUIRE_EXT(ctx, RVD);
>>   -    if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
>> +    /*
>> +     * Zama16b applies to loads and stores of no more than MXLEN 
>> bits defined
>> +     * in the F, D, and Q extensions. Otherwise, it falls through to 
>> default
>> +     * MO_ATOM_IFALIGN.
>> +     */
>> +    if ((ctx->xl >= MXL_RV64) && (ctx->cfg_ptr->ext_zama16b)) {
>
> I think you meant to add the mxlen check in the next patch,
> because you modify this line again.
>
Oh, I didn't notice it.  I once split the whole patch into two patches. 
But obviously I did only a half.

Thanks,
Zhiwei

>
> r~