1. Patchew
  2. QEMU
  3. [PATCH v2 0/3] target/riscv: Remove redundant insn length check for zama16b
  4. View patch

[PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b

LIU Zhiwei posted 3 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago 
Zama16b loads and stores of no more than MXLEN bits defined in the F, D, and Q
extensions.

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 2be037930a..dbe508c7e0 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -52,7 +52,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
      * 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)) {
+    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
 
@@ -72,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
     REQUIRE_FPU;
     REQUIRE_EXT(ctx, RVD);
 
-    if (ctx->cfg_ptr->ext_zama16b) {
+    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;
     }
 
-- 
2.25.1
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by Richard Henderson 3 months, 3 weeks ago 
On 8/2/24 13:16, LIU Zhiwei wrote:
> Zama16b loads and stores of no more than MXLEN bits defined in the F, D, and Q
> extensions.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>   target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 2be037930a..dbe508c7e0 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -52,7 +52,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>        * 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)) {
> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>           memop |= MO_ATOM_WITHIN16;
>       }
>   
> @@ -72,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>       REQUIRE_FPU;
>       REQUIRE_EXT(ctx, RVD);
>   
> -    if (ctx->cfg_ptr->ext_zama16b) {
> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>           memop |= MO_ATOM_WITHIN16;
>       }

I guess this is ok, because MXL cannot currently be changed. But since that is a possible 
future enhancement, perhaps add a get_mxl(ctx) accessor anyway, for documentation purposes.


r~
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago 
On 2024/8/2 13:47, Richard Henderson wrote:
> On 8/2/24 13:16, LIU Zhiwei wrote:
>> Zama16b loads and stores of no more than MXLEN bits defined in the F, 
>> D, and Q
>> extensions.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
>> b/target/riscv/insn_trans/trans_rvd.c.inc
>> index 2be037930a..dbe508c7e0 100644
>> --- a/target/riscv/insn_trans/trans_rvd.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
>> @@ -52,7 +52,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>>        * 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)) {
>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>           memop |= MO_ATOM_WITHIN16;
>>       }
>>   @@ -72,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>>       REQUIRE_FPU;
>>       REQUIRE_EXT(ctx, RVD);
>>   -    if (ctx->cfg_ptr->ext_zama16b) {
>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>           memop |= MO_ATOM_WITHIN16;
>>       }
>
> I guess this is ok, because MXL cannot currently be changed. But since 
> that is a possible future enhancement, perhaps add a get_mxl(ctx) 
> accessor anyway, for documentation purposes.
Can we use the existing get_xl_max(ctx) instead of adding a new 
get_mxl(ctx).

Thanks,
Zhiwei

>
>
> r~
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by Richard Henderson 3 months, 3 weeks ago 
On 8/2/24 16:42, LIU Zhiwei wrote:
> 
> On 2024/8/2 13:47, Richard Henderson wrote:
>> On 8/2/24 13:16, LIU Zhiwei wrote:
>>> Zama16b loads and stores of no more than MXLEN bits defined in the F, D, and Q
>>> extensions.
>>>
>>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>>> ---
>>>   target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/ 
>>> trans_rvd.c.inc
>>> index 2be037930a..dbe508c7e0 100644
>>> --- a/target/riscv/insn_trans/trans_rvd.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
>>> @@ -52,7 +52,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>>>        * 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)) {
>>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>>           memop |= MO_ATOM_WITHIN16;
>>>       }
>>>   @@ -72,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>>>       REQUIRE_FPU;
>>>       REQUIRE_EXT(ctx, RVD);
>>>   -    if (ctx->cfg_ptr->ext_zama16b) {
>>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>>           memop |= MO_ATOM_WITHIN16;
>>>       }
>>
>> I guess this is ok, because MXL cannot currently be changed. But since that is a 
>> possible future enhancement, perhaps add a get_mxl(ctx) accessor anyway, for 
>> documentation purposes.
> Can we use the existing get_xl_max(ctx) instead of adding a new get_mxl(ctx).

Yes, that looks fine.
That could be renamed get_mxl.  :-)


r~
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago 
On 2024/8/2 13:47, Richard Henderson wrote:
> On 8/2/24 13:16, LIU Zhiwei wrote:
>> Zama16b loads and stores of no more than MXLEN bits defined in the F, 
>> D, and Q
>> extensions.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
>> b/target/riscv/insn_trans/trans_rvd.c.inc
>> index 2be037930a..dbe508c7e0 100644
>> --- a/target/riscv/insn_trans/trans_rvd.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
>> @@ -52,7 +52,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>>        * 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)) {
>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>           memop |= MO_ATOM_WITHIN16;
>>       }
>>   @@ -72,7 +72,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>>       REQUIRE_FPU;
>>       REQUIRE_EXT(ctx, RVD);
>>   -    if (ctx->cfg_ptr->ext_zama16b) {
>> +    if ((ctx->misa_mxl_max >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
>>           memop |= MO_ATOM_WITHIN16;
>>       }
>
> I guess this is ok, because MXL cannot currently be changed. But since 
> that is a possible future enhancement, perhaps add a get_mxl(ctx) 
> accessor anyway, for documentation purposes.

OK, I will use it. By the way, the MXL is const now in recently updated 
RISC-V specification.

Thanks，
Zhiwei

>
>
> r~
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by Richard Henderson 3 months, 3 weeks ago 
On 8/2/24 16:21, LIU Zhiwei wrote:
> By the way, the MXL is const now in recently updated RISC-V specification.

Oh yes?  Then perhaps we should rename misa_mxl_max to misa_mxl.


r~
Re: [PATCH v2 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
Posted by LIU Zhiwei 3 months, 3 weeks ago 
On 2024/8/2 14:45, Richard Henderson wrote:
> On 8/2/24 16:21, LIU Zhiwei wrote:
>> By the way, the MXL is const now in recently updated RISC-V 
>> specification.
>
> Oh yes? 
Yes.  In 1.13 privileged specification about MISA CSR:

"The MXL field is read-only. If misa is nonzero, the MXL field indicates 
the effective XLEN in M-mode, a
constant termed MXLEN."

> Then perhaps we should rename misa_mxl_max to misa_mxl.

I will mark this on my todo list.

Thanks,
Zhiwei

>
>
> r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.