* [PATCH v3 1/3] target/riscv: Remove redundant insn length check for zama16b
2024-08-02 7:24 [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b LIU Zhiwei
@ 2024-08-02 7:24 ` LIU Zhiwei
2024-08-02 7:53 ` Richard Henderson
2024-08-04 23:52 ` Alistair Francis
2024-08-02 7:24 ` [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b LIU Zhiwei
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: LIU Zhiwei @ 2024-08-02 7:24 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn, zhiwei_liu, richard.henderson
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 | 4 ++--
target/riscv/insn_trans/trans_rvf.c.inc | 4 ++--
target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
3 files changed, 6 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..0ac42c3223 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -47,7 +47,7 @@ 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)) {
+ if (ctx->cfg_ptr->ext_zama16b) {
memop |= MO_ATOM_WITHIN16;
}
@@ -67,7 +67,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Remove redundant insn length check for zama16b
2024-08-02 7:24 ` [PATCH v3 1/3] " LIU Zhiwei
@ 2024-08-02 7:53 ` Richard Henderson
2024-08-04 23:52 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2024-08-02 7:53 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn
On 8/2/24 17:24, LIU Zhiwei wrote:
> 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 | 4 ++--
> target/riscv/insn_trans/trans_rvf.c.inc | 4 ++--
> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] target/riscv: Remove redundant insn length check for zama16b
2024-08-02 7:24 ` [PATCH v3 1/3] " LIU Zhiwei
2024-08-02 7:53 ` Richard Henderson
@ 2024-08-04 23:52 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-08-04 23:52 UTC (permalink / raw)
To: LIU Zhiwei
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, dbarboza,
liwei1518, bmeng.cn, richard.henderson
On Fri, Aug 2, 2024 at 5:26 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> 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>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/insn_trans/trans_rvd.c.inc | 4 ++--
> target/riscv/insn_trans/trans_rvf.c.inc | 4 ++--
> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> 3 files changed, 6 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..0ac42c3223 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -47,7 +47,7 @@ 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)) {
> + if (ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> }
>
> @@ -67,7 +67,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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
2024-08-02 7:24 [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b LIU Zhiwei
2024-08-02 7:24 ` [PATCH v3 1/3] " LIU Zhiwei
@ 2024-08-02 7:24 ` LIU Zhiwei
2024-08-02 7:54 ` Richard Henderson
2024-08-04 23:54 ` Alistair Francis
2024-08-02 7:24 ` [PATCH v3 3/3] target/riscv: Relax fld alignment requirement LIU Zhiwei
2024-08-05 1:51 ` [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b Alistair Francis
3 siblings, 2 replies; 11+ messages in thread
From: LIU Zhiwei @ 2024-08-02 7:24 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn, zhiwei_liu, richard.henderson
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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 0ac42c3223..49682292b8 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -47,7 +47,11 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
REQUIRE_FPU;
REQUIRE_EXT(ctx, RVD);
- if (ctx->cfg_ptr->ext_zama16b) {
+ /*
+ * Zama16b applies to loads and stores of no more than MXLEN bits defined
+ * in the F, D, and Q extensions.
+ */
+ if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
memop |= MO_ATOM_WITHIN16;
}
@@ -67,7 +71,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
REQUIRE_FPU;
REQUIRE_EXT(ctx, RVD);
- if (ctx->cfg_ptr->ext_zama16b) {
+ if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
memop |= MO_ATOM_WITHIN16;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
2024-08-02 7:24 ` [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b LIU Zhiwei
@ 2024-08-02 7:54 ` Richard Henderson
2024-08-04 23:54 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2024-08-02 7:54 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn
On 8/2/24 17:24, 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 | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 0ac42c3223..49682292b8 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -47,7 +47,11 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
> REQUIRE_FPU;
> REQUIRE_EXT(ctx, RVD);
>
> - if (ctx->cfg_ptr->ext_zama16b) {
> + /*
> + * Zama16b applies to loads and stores of no more than MXLEN bits defined
> + * in the F, D, and Q extensions.
> + */
> + if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> }
>
> @@ -67,7 +71,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
> REQUIRE_FPU;
> REQUIRE_EXT(ctx, RVD);
>
> - if (ctx->cfg_ptr->ext_zama16b) {
> + if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> }
>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b
2024-08-02 7:24 ` [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b LIU Zhiwei
2024-08-02 7:54 ` Richard Henderson
@ 2024-08-04 23:54 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-08-04 23:54 UTC (permalink / raw)
To: LIU Zhiwei
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, dbarboza,
liwei1518, bmeng.cn, richard.henderson
On Fri, Aug 2, 2024 at 5:27 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> 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>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/insn_trans/trans_rvd.c.inc | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 0ac42c3223..49682292b8 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -47,7 +47,11 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
> REQUIRE_FPU;
> REQUIRE_EXT(ctx, RVD);
>
> - if (ctx->cfg_ptr->ext_zama16b) {
> + /*
> + * Zama16b applies to loads and stores of no more than MXLEN bits defined
> + * in the F, D, and Q extensions.
> + */
> + if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> }
>
> @@ -67,7 +71,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
> REQUIRE_FPU;
> REQUIRE_EXT(ctx, RVD);
>
> - if (ctx->cfg_ptr->ext_zama16b) {
> + if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] target/riscv: Relax fld alignment requirement
2024-08-02 7:24 [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b LIU Zhiwei
2024-08-02 7:24 ` [PATCH v3 1/3] " LIU Zhiwei
2024-08-02 7:24 ` [PATCH v3 2/3] target/riscv: Add MXLEN check for F/D/Q applies to zama16b LIU Zhiwei
@ 2024-08-02 7:24 ` LIU Zhiwei
2024-08-02 7:54 ` Richard Henderson
2024-08-04 23:56 ` Alistair Francis
2024-08-05 1:51 ` [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b Alistair Francis
3 siblings, 2 replies; 11+ messages in thread
From: LIU Zhiwei @ 2024-08-02 7:24 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn, zhiwei_liu, richard.henderson
According to the risc-v specification:
"FLD and FSD are only guaranteed to execute atomically if the effective
address is naturally aligned and XLEN≥64."
We currently implement fld as MO_ATOM_IFALIGN when XLEN < 64, which does
not violate the rules. But it will hide some problems. So relax it to
MO_ATOM_NONE.
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
target/riscv/insn_trans/trans_rvd.c.inc | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 49682292b8..8a46124f98 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -48,11 +48,17 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
REQUIRE_EXT(ctx, RVD);
/*
- * Zama16b applies to loads and stores of no more than MXLEN bits defined
- * in the F, D, and Q extensions.
+ * FLD and FSD are only guaranteed to execute atomically if the effective
+ * address is naturally aligned and XLEN≥64. Also, zama16b applies to
+ * loads and stores of no more than MXLEN bits defined in the F, D, and
+ * Q extensions.
*/
- if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
+ if (get_xl_max(ctx) == MXL_RV32) {
+ memop |= MO_ATOM_NONE;
+ } else if (ctx->cfg_ptr->ext_zama16b) {
memop |= MO_ATOM_WITHIN16;
+ } else {
+ memop |= MO_ATOM_IFALIGN;
}
decode_save_opc(ctx);
@@ -71,8 +77,12 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
REQUIRE_FPU;
REQUIRE_EXT(ctx, RVD);
- if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
+ if (get_xl_max(ctx) == MXL_RV32) {
+ memop |= MO_ATOM_NONE;
+ } else if (ctx->cfg_ptr->ext_zama16b) {
memop |= MO_ATOM_WITHIN16;
+ } else {
+ memop |= MO_ATOM_IFALIGN;
}
decode_save_opc(ctx);
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] target/riscv: Relax fld alignment requirement
2024-08-02 7:24 ` [PATCH v3 3/3] target/riscv: Relax fld alignment requirement LIU Zhiwei
@ 2024-08-02 7:54 ` Richard Henderson
2024-08-04 23:56 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2024-08-02 7:54 UTC (permalink / raw)
To: LIU Zhiwei, qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, dbarboza, liwei1518,
bmeng.cn
On 8/2/24 17:24, LIU Zhiwei wrote:
> According to the risc-v specification:
> "FLD and FSD are only guaranteed to execute atomically if the effective
> address is naturally aligned and XLEN≥64."
>
> We currently implement fld as MO_ATOM_IFALIGN when XLEN < 64, which does
> not violate the rules. But it will hide some problems. So relax it to
> MO_ATOM_NONE.
>
> Signed-off-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
> ---
> target/riscv/insn_trans/trans_rvd.c.inc | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] target/riscv: Relax fld alignment requirement
2024-08-02 7:24 ` [PATCH v3 3/3] target/riscv: Relax fld alignment requirement LIU Zhiwei
2024-08-02 7:54 ` Richard Henderson
@ 2024-08-04 23:56 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-08-04 23:56 UTC (permalink / raw)
To: LIU Zhiwei
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, dbarboza,
liwei1518, bmeng.cn, richard.henderson
On Fri, Aug 2, 2024 at 5:27 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> According to the risc-v specification:
> "FLD and FSD are only guaranteed to execute atomically if the effective
> address is naturally aligned and XLEN≥64."
>
> We currently implement fld as MO_ATOM_IFALIGN when XLEN < 64, which does
> not violate the rules. But it will hide some problems. So relax it to
> MO_ATOM_NONE.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/insn_trans/trans_rvd.c.inc | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 49682292b8..8a46124f98 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -48,11 +48,17 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
> REQUIRE_EXT(ctx, RVD);
>
> /*
> - * Zama16b applies to loads and stores of no more than MXLEN bits defined
> - * in the F, D, and Q extensions.
> + * FLD and FSD are only guaranteed to execute atomically if the effective
> + * address is naturally aligned and XLEN≥64. Also, zama16b applies to
> + * loads and stores of no more than MXLEN bits defined in the F, D, and
> + * Q extensions.
> */
> - if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> + if (get_xl_max(ctx) == MXL_RV32) {
> + memop |= MO_ATOM_NONE;
> + } else if (ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> + } else {
> + memop |= MO_ATOM_IFALIGN;
> }
>
> decode_save_opc(ctx);
> @@ -71,8 +77,12 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
> REQUIRE_FPU;
> REQUIRE_EXT(ctx, RVD);
>
> - if ((get_xl_max(ctx) >= MXL_RV64) && ctx->cfg_ptr->ext_zama16b) {
> + if (get_xl_max(ctx) == MXL_RV32) {
> + memop |= MO_ATOM_NONE;
> + } else if (ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> + } else {
> + memop |= MO_ATOM_IFALIGN;
> }
>
> decode_save_opc(ctx);
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b
2024-08-02 7:24 [PATCH v3 0/3] target/riscv: Remove redundant insn length check for zama16b LIU Zhiwei
` (2 preceding siblings ...)
2024-08-02 7:24 ` [PATCH v3 3/3] target/riscv: Relax fld alignment requirement LIU Zhiwei
@ 2024-08-05 1:51 ` Alistair Francis
3 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-08-05 1:51 UTC (permalink / raw)
To: LIU Zhiwei
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, dbarboza,
liwei1518, bmeng.cn, richard.henderson
On Fri, Aug 2, 2024 at 5:25 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> In this patch set, we remove the redundant insn length check for zama16b as the
> specification clarified that zama16b applies to compressed encodings[1].
>
> Richard points out we should obey the MXLEN requirement for F/D/Q loads or stores,
> so we add this constraint for trans_fld/fsd.
>
> I notice that we have a too strict aligment implementation for fld/fsd when xlen < 64.
> It will hide some problems. So relex it from MO_ATOM_IFALIGN to MO_ATOM_NONE.
>
> [1]: https://github.com/riscv/riscv-isa-manual/pull/1557
>
> v3<-v2:
> 1. Using get_xl_max instead of ctx->misa_mxl_max as documentation.
> 2. Fix not clean split in patch 1.
> 3. Explicitly specified aligment for fld/fsd under all cases.
>
> v2<-v1:
> 1. Add mxlen check for fld when applies zama16b.
> 2. Relax fld/fsd alignment for MO_ATOM_IFALIGN to MO_ATOM_NONE.
>
> LIU Zhiwei (3):
> target/riscv: Remove redundant insn length check for zama16b
> target/riscv: Add MXLEN check for F/D/Q applies to zama16b
> target/riscv: Relax fld alignment requirement
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> target/riscv/insn_trans/trans_rvd.c.inc | 18 ++++++++++++++++--
> target/riscv/insn_trans/trans_rvf.c.inc | 4 ++--
> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread