* [PATCH 0/2] target/riscv: some vector_helper.c cleanups
@ 2023-02-26 17:05 Daniel Henrique Barboza
2023-02-26 17:05 ` [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s() Daniel Henrique Barboza
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-26 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu,
Daniel Henrique Barboza
Based-on: 20230222185205.355361-2-dbarboza@ventanamicro.com
("[PATCH v7 01/10] target/riscv: introduce riscv_cpu_cfg()")
Hi,
This is a re-send of patch 1, which is already reviewed, with a
follow-up that uses riscv_cpu_cfg() in the remaining of the file. This
was suggested by Weiwei Li in the "[PATCH 0/4] RISCVCPUConfig related
cleanups" review. Patch 1 makes the work of patch 2 easier since it
eliminated some uses of env_archcpu() we want to avoid.
Both patches depends on patch "[PATCH v7 01/10] target/riscv: introduce
riscv_cpu_cfg()" that can be found here:
https://patchew.org/QEMU/20230222185205.355361-1-dbarboza@ventanamicro.com/20230222185205.355361-2-dbarboza@ventanamicro.com/
Daniel Henrique Barboza (2):
target/riscv/vector_helper.c: create vext_set_tail_elems_1s()
target/riscv/vector_helper.c: avoid env_archcpu() when reading
RISCVCPUConfig
target/riscv/vector_helper.c | 104 +++++++++++++----------------------
1 file changed, 39 insertions(+), 65 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s()
2023-02-26 17:05 [PATCH 0/2] target/riscv: some vector_helper.c cleanups Daniel Henrique Barboza
@ 2023-02-26 17:05 ` Daniel Henrique Barboza
2023-02-26 18:23 ` Philippe Mathieu-Daudé
2023-02-26 17:05 ` [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig Daniel Henrique Barboza
2023-03-02 2:32 ` [PATCH 0/2] target/riscv: some vector_helper.c cleanups Palmer Dabbelt
2 siblings, 1 reply; 8+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-26 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu,
Daniel Henrique Barboza, Frank Chang
Commit 752614cab8e6 ("target/riscv: rvv: Add tail agnostic for vector
load / store instructions") added code to set the tail elements to 1 in
the end of vext_ldst_stride(), vext_ldst_us(), vext_ldst_index() and
vext_ldff(). Aside from a env->vl versus an evl value being used in the
first loop, the code is being repeated 4 times.
Create a helper to avoid code repetition in all those functions.
Arguments that are used in the callers (nf, esz and max_elems) are
passed as arguments. All other values are being derived inside the
helper.
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/vector_helper.c | 86 +++++++++++++-----------------------
1 file changed, 30 insertions(+), 56 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 00de879787..7d2e3978f1 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -267,6 +267,28 @@ GEN_VEXT_ST_ELEM(ste_h, int16_t, H2, stw)
GEN_VEXT_ST_ELEM(ste_w, int32_t, H4, stl)
GEN_VEXT_ST_ELEM(ste_d, int64_t, H8, stq)
+static void vext_set_tail_elems_1s(CPURISCVState *env, target_ulong vl,
+ void *vd, uint32_t desc, uint32_t nf,
+ uint32_t esz, uint32_t max_elems)
+{
+ uint32_t total_elems = vext_get_total_elems(env, desc, esz);
+ uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
+ uint32_t vta = vext_vta(desc);
+ uint32_t registers_used;
+ int k;
+
+ for (k = 0; k < nf; ++k) {
+ vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
+ (k * max_elems + max_elems) * esz);
+ }
+
+ if (nf * max_elems % total_elems != 0) {
+ registers_used = ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
+ vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
+ registers_used * vlenb);
+ }
+}
+
/*
*** stride: access vector element from strided memory
*/
@@ -281,8 +303,6 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
uint32_t nf = vext_nf(desc);
uint32_t max_elems = vext_max_elems(desc, log2_esz);
uint32_t esz = 1 << log2_esz;
- uint32_t total_elems = vext_get_total_elems(env, desc, esz);
- uint32_t vta = vext_vta(desc);
uint32_t vma = vext_vma(desc);
for (i = env->vstart; i < env->vl; i++, env->vstart++) {
@@ -301,18 +321,8 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
}
}
env->vstart = 0;
- /* set tail elements to 1s */
- for (k = 0; k < nf; ++k) {
- vext_set_elems_1s(vd, vta, (k * max_elems + env->vl) * esz,
- (k * max_elems + max_elems) * esz);
- }
- if (nf * max_elems % total_elems != 0) {
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
- uint32_t registers_used =
- ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
- vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
- registers_used * vlenb);
- }
+
+ vext_set_tail_elems_1s(env, env->vl, vd, desc, nf, esz, max_elems);
}
#define GEN_VEXT_LD_STRIDE(NAME, ETYPE, LOAD_FN) \
@@ -359,8 +369,6 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
uint32_t nf = vext_nf(desc);
uint32_t max_elems = vext_max_elems(desc, log2_esz);
uint32_t esz = 1 << log2_esz;
- uint32_t total_elems = vext_get_total_elems(env, desc, esz);
- uint32_t vta = vext_vta(desc);
/* load bytes from guest memory */
for (i = env->vstart; i < evl; i++, env->vstart++) {
@@ -372,18 +380,8 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
}
}
env->vstart = 0;
- /* set tail elements to 1s */
- for (k = 0; k < nf; ++k) {
- vext_set_elems_1s(vd, vta, (k * max_elems + evl) * esz,
- (k * max_elems + max_elems) * esz);
- }
- if (nf * max_elems % total_elems != 0) {
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
- uint32_t registers_used =
- ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
- vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
- registers_used * vlenb);
- }
+
+ vext_set_tail_elems_1s(env, evl, vd, desc, nf, esz, max_elems);
}
/*
@@ -484,8 +482,6 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
uint32_t vm = vext_vm(desc);
uint32_t max_elems = vext_max_elems(desc, log2_esz);
uint32_t esz = 1 << log2_esz;
- uint32_t total_elems = vext_get_total_elems(env, desc, esz);
- uint32_t vta = vext_vta(desc);
uint32_t vma = vext_vma(desc);
/* load bytes from guest memory */
@@ -505,18 +501,8 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
}
}
env->vstart = 0;
- /* set tail elements to 1s */
- for (k = 0; k < nf; ++k) {
- vext_set_elems_1s(vd, vta, (k * max_elems + env->vl) * esz,
- (k * max_elems + max_elems) * esz);
- }
- if (nf * max_elems % total_elems != 0) {
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
- uint32_t registers_used =
- ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
- vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
- registers_used * vlenb);
- }
+
+ vext_set_tail_elems_1s(env, env->vl, vd, desc, nf, esz, max_elems);
}
#define GEN_VEXT_LD_INDEX(NAME, ETYPE, INDEX_FN, LOAD_FN) \
@@ -585,8 +571,6 @@ vext_ldff(void *vd, void *v0, target_ulong base,
uint32_t vm = vext_vm(desc);
uint32_t max_elems = vext_max_elems(desc, log2_esz);
uint32_t esz = 1 << log2_esz;
- uint32_t total_elems = vext_get_total_elems(env, desc, esz);
- uint32_t vta = vext_vta(desc);
uint32_t vma = vext_vma(desc);
target_ulong addr, offset, remain;
@@ -647,18 +631,8 @@ ProbeSuccess:
}
}
env->vstart = 0;
- /* set tail elements to 1s */
- for (k = 0; k < nf; ++k) {
- vext_set_elems_1s(vd, vta, (k * max_elems + env->vl) * esz,
- (k * max_elems + max_elems) * esz);
- }
- if (nf * max_elems % total_elems != 0) {
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
- uint32_t registers_used =
- ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
- vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
- registers_used * vlenb);
- }
+
+ vext_set_tail_elems_1s(env, env->vl, vd, desc, nf, esz, max_elems);
}
#define GEN_VEXT_LDFF(NAME, ETYPE, LOAD_FN) \
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig
2023-02-26 17:05 [PATCH 0/2] target/riscv: some vector_helper.c cleanups Daniel Henrique Barboza
2023-02-26 17:05 ` [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s() Daniel Henrique Barboza
@ 2023-02-26 17:05 ` Daniel Henrique Barboza
2023-02-26 18:24 ` Philippe Mathieu-Daudé
2023-02-27 0:58 ` liweiwei
2023-03-02 2:32 ` [PATCH 0/2] target/riscv: some vector_helper.c cleanups Palmer Dabbelt
2 siblings, 2 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-26 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu,
Daniel Henrique Barboza
This file has several uses of env_archcpu() that are used solely to read
cfg->vlen. Use the new riscv_cpu_cfg() inline instead.
Suggested-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/vector_helper.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 7d2e3978f1..a7fb09efa3 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -272,7 +272,7 @@ static void vext_set_tail_elems_1s(CPURISCVState *env, target_ulong vl,
uint32_t esz, uint32_t max_elems)
{
uint32_t total_elems = vext_get_total_elems(env, desc, esz);
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
+ uint32_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
uint32_t vta = vext_vta(desc);
uint32_t registers_used;
int k;
@@ -671,7 +671,7 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
{
uint32_t i, k, off, pos;
uint32_t nf = vext_nf(desc);
- uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
+ uint32_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
uint32_t max_elems = vlenb >> log2_esz;
k = env->vstart / max_elems;
@@ -1141,7 +1141,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
{ \
uint32_t vl = env->vl; \
uint32_t vm = vext_vm(desc); \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t i; \
\
@@ -1177,7 +1177,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, \
{ \
uint32_t vl = env->vl; \
uint32_t vm = vext_vm(desc); \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t i; \
\
@@ -1376,7 +1376,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl; \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t vma = vext_vma(desc); \
uint32_t i; \
@@ -1439,7 +1439,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl; \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t vma = vext_vma(desc); \
uint32_t i; \
@@ -4152,7 +4152,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl; \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t vma = vext_vma(desc); \
uint32_t i; \
@@ -4190,7 +4190,7 @@ void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl; \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t vma = vext_vma(desc); \
uint32_t i; \
@@ -4721,7 +4721,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, \
uint32_t desc) \
{ \
uint32_t vl = env->vl; \
- uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t i; \
int a, b; \
@@ -4808,7 +4808,7 @@ static void vmsetm(void *vd, void *v0, void *vs2, CPURISCVState *env,
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
- uint32_t total_elems = env_archcpu(env)->cfg.vlen;
+ uint32_t total_elems = riscv_cpu_cfg(env)->vlen;
uint32_t vta_all_1s = vext_vta_all_1s(desc);
uint32_t vma = vext_vma(desc);
int i;
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s()
2023-02-26 17:05 ` [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s() Daniel Henrique Barboza
@ 2023-02-26 18:23 ` Philippe Mathieu-Daudé
2023-03-02 2:13 ` Palmer Dabbelt
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-26 18:23 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu,
Frank Chang
On 26/2/23 18:05, Daniel Henrique Barboza wrote:
> Commit 752614cab8e6 ("target/riscv: rvv: Add tail agnostic for vector
> load / store instructions") added code to set the tail elements to 1 in
> the end of vext_ldst_stride(), vext_ldst_us(), vext_ldst_index() and
> vext_ldff(). Aside from a env->vl versus an evl value being used in the
> first loop, the code is being repeated 4 times.
>
> Create a helper to avoid code repetition in all those functions.
> Arguments that are used in the callers (nf, esz and max_elems) are
> passed as arguments. All other values are being derived inside the
> helper.
>
> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/vector_helper.c | 86 +++++++++++++-----------------------
> 1 file changed, 30 insertions(+), 56 deletions(-)
> +static void vext_set_tail_elems_1s(CPURISCVState *env, target_ulong vl,
> + void *vd, uint32_t desc, uint32_t nf,
> + uint32_t esz, uint32_t max_elems)
> +{
> + uint32_t total_elems = vext_get_total_elems(env, desc, esz);
> + uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> + uint32_t vta = vext_vta(desc);
> + uint32_t registers_used;
> + int k;
> +
> + for (k = 0; k < nf; ++k) {
> + vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
> + (k * max_elems + max_elems) * esz);
> + }
> +
> + if (nf * max_elems % total_elems != 0) {
> + registers_used = ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
> + vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
> + registers_used * vlenb);
> + }
for (unsigned k = 0; k < nf; ++k) {
vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
(k * max_elems + max_elems) * esz);
}
if (nf * max_elems % total_elems != 0) {
uint32_t cnt = (nf * max_elems) * esz;
vext_set_elems_1s(vd, vta, cnt, QEMU_ALIGN_UP(cnt, vlenb));
}
I suspect ROUND_UP() could be used if vlenb is a power of 2.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig
2023-02-26 17:05 ` [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig Daniel Henrique Barboza
@ 2023-02-26 18:24 ` Philippe Mathieu-Daudé
2023-02-27 0:58 ` liweiwei
1 sibling, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-26 18:24 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu
On 26/2/23 18:05, Daniel Henrique Barboza wrote:
> This file has several uses of env_archcpu() that are used solely to read
> cfg->vlen. Use the new riscv_cpu_cfg() inline instead.
>
> Suggested-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/vector_helper.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig
2023-02-26 17:05 ` [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig Daniel Henrique Barboza
2023-02-26 18:24 ` Philippe Mathieu-Daudé
@ 2023-02-27 0:58 ` liweiwei
1 sibling, 0 replies; 8+ messages in thread
From: liweiwei @ 2023-02-27 0:58 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu
On 2023/2/27 01:05, Daniel Henrique Barboza wrote:
> This file has several uses of env_archcpu() that are used solely to read
> cfg->vlen. Use the new riscv_cpu_cfg() inline instead.
>
> Suggested-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Weiwei Li<liweiwei@iscas.ac.cn>
Weiwei Li
> ---
> target/riscv/vector_helper.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 7d2e3978f1..a7fb09efa3 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -272,7 +272,7 @@ static void vext_set_tail_elems_1s(CPURISCVState *env, target_ulong vl,
> uint32_t esz, uint32_t max_elems)
> {
> uint32_t total_elems = vext_get_total_elems(env, desc, esz);
> - uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> + uint32_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
> uint32_t vta = vext_vta(desc);
> uint32_t registers_used;
> int k;
> @@ -671,7 +671,7 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
> {
> uint32_t i, k, off, pos;
> uint32_t nf = vext_nf(desc);
> - uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> + uint32_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
> uint32_t max_elems = vlenb >> log2_esz;
>
> k = env->vstart / max_elems;
> @@ -1141,7 +1141,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
> { \
> uint32_t vl = env->vl; \
> uint32_t vm = vext_vm(desc); \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t i; \
> \
> @@ -1177,7 +1177,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, \
> { \
> uint32_t vl = env->vl; \
> uint32_t vm = vext_vm(desc); \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t i; \
> \
> @@ -1376,7 +1376,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
> { \
> uint32_t vm = vext_vm(desc); \
> uint32_t vl = env->vl; \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t vma = vext_vma(desc); \
> uint32_t i; \
> @@ -1439,7 +1439,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> { \
> uint32_t vm = vext_vm(desc); \
> uint32_t vl = env->vl; \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t vma = vext_vma(desc); \
> uint32_t i; \
> @@ -4152,7 +4152,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
> { \
> uint32_t vm = vext_vm(desc); \
> uint32_t vl = env->vl; \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t vma = vext_vma(desc); \
> uint32_t i; \
> @@ -4190,7 +4190,7 @@ void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> { \
> uint32_t vm = vext_vm(desc); \
> uint32_t vl = env->vl; \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t vma = vext_vma(desc); \
> uint32_t i; \
> @@ -4721,7 +4721,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, \
> uint32_t desc) \
> { \
> uint32_t vl = env->vl; \
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen; \
> uint32_t vta_all_1s = vext_vta_all_1s(desc); \
> uint32_t i; \
> int a, b; \
> @@ -4808,7 +4808,7 @@ static void vmsetm(void *vd, void *v0, void *vs2, CPURISCVState *env,
> {
> uint32_t vm = vext_vm(desc);
> uint32_t vl = env->vl;
> - uint32_t total_elems = env_archcpu(env)->cfg.vlen;
> + uint32_t total_elems = riscv_cpu_cfg(env)->vlen;
> uint32_t vta_all_1s = vext_vta_all_1s(desc);
> uint32_t vma = vext_vma(desc);
> int i;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s()
2023-02-26 18:23 ` Philippe Mathieu-Daudé
@ 2023-03-02 2:13 ` Palmer Dabbelt
0 siblings, 0 replies; 8+ messages in thread
From: Palmer Dabbelt @ 2023-03-02 2:13 UTC (permalink / raw)
To: philmd
Cc: dbarboza, qemu-devel, qemu-riscv, Alistair Francis, bmeng,
liweiwei, zhiwei_liu, frank.chang
On Sun, 26 Feb 2023 10:23:01 PST (-0800), philmd@linaro.org wrote:
> On 26/2/23 18:05, Daniel Henrique Barboza wrote:
>> Commit 752614cab8e6 ("target/riscv: rvv: Add tail agnostic for vector
>> load / store instructions") added code to set the tail elements to 1 in
>> the end of vext_ldst_stride(), vext_ldst_us(), vext_ldst_index() and
>> vext_ldff(). Aside from a env->vl versus an evl value being used in the
>> first loop, the code is being repeated 4 times.
>>
>> Create a helper to avoid code repetition in all those functions.
>> Arguments that are used in the callers (nf, esz and max_elems) are
>> passed as arguments. All other values are being derived inside the
>> helper.
>>
>> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Reviewed-by: Frank Chang <frank.chang@sifive.com>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>> target/riscv/vector_helper.c | 86 +++++++++++++-----------------------
>> 1 file changed, 30 insertions(+), 56 deletions(-)
>
>
>> +static void vext_set_tail_elems_1s(CPURISCVState *env, target_ulong vl,
>> + void *vd, uint32_t desc, uint32_t nf,
>> + uint32_t esz, uint32_t max_elems)
>> +{
>> + uint32_t total_elems = vext_get_total_elems(env, desc, esz);
>> + uint32_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
>> + uint32_t vta = vext_vta(desc);
>> + uint32_t registers_used;
>> + int k;
>> +
>> + for (k = 0; k < nf; ++k) {
>> + vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
>> + (k * max_elems + max_elems) * esz);
>> + }
>> +
>> + if (nf * max_elems % total_elems != 0) {
>> + registers_used = ((nf * max_elems) * esz + (vlenb - 1)) / vlenb;
>> + vext_set_elems_1s(vd, vta, (nf * max_elems) * esz,
>> + registers_used * vlenb);
>> + }
>
> for (unsigned k = 0; k < nf; ++k) {
> vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
> (k * max_elems + max_elems) * esz);
> }
>
> if (nf * max_elems % total_elems != 0) {
> uint32_t cnt = (nf * max_elems) * esz;
> vext_set_elems_1s(vd, vta, cnt, QEMU_ALIGN_UP(cnt, vlenb));
> }
>
> I suspect ROUND_UP() could be used if vlenb is a power of 2.
As far as I can tell there's nothing in the ISA that requires vlenb be a
power of two, it's just defined as
The XLEN-bit-wide read-only CSR vlenb holds the value VLEN/8, i.e.,
the vector register length in bytes.
I'm pretty surprised to see that's the case and I'd doubt anything
actually works with non-power-of-two vlenb. It's possible I'm just
missing something in the ISA so I opened a bug at
<https://github.com/riscv/riscv-v-spec/issues/864>.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] target/riscv: some vector_helper.c cleanups
2023-02-26 17:05 [PATCH 0/2] target/riscv: some vector_helper.c cleanups Daniel Henrique Barboza
2023-02-26 17:05 ` [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s() Daniel Henrique Barboza
2023-02-26 17:05 ` [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig Daniel Henrique Barboza
@ 2023-03-02 2:32 ` Palmer Dabbelt
2 siblings, 0 replies; 8+ messages in thread
From: Palmer Dabbelt @ 2023-03-02 2:32 UTC (permalink / raw)
To: dbarboza
Cc: qemu-devel, qemu-riscv, Alistair Francis, bmeng, liweiwei,
zhiwei_liu, dbarboza
On Sun, 26 Feb 2023 09:05:12 PST (-0800), dbarboza@ventanamicro.com wrote:
> Based-on: 20230222185205.355361-2-dbarboza@ventanamicro.com
> ("[PATCH v7 01/10] target/riscv: introduce riscv_cpu_cfg()")
>
> Hi,
>
> This is a re-send of patch 1, which is already reviewed, with a
> follow-up that uses riscv_cpu_cfg() in the remaining of the file. This
> was suggested by Weiwei Li in the "[PATCH 0/4] RISCVCPUConfig related
> cleanups" review. Patch 1 makes the work of patch 2 easier since it
> eliminated some uses of env_archcpu() we want to avoid.
>
> Both patches depends on patch "[PATCH v7 01/10] target/riscv: introduce
> riscv_cpu_cfg()" that can be found here:
>
> https://patchew.org/QEMU/20230222185205.355361-1-dbarboza@ventanamicro.com/20230222185205.355361-2-dbarboza@ventanamicro.com/
>
>
> Daniel Henrique Barboza (2):
> target/riscv/vector_helper.c: create vext_set_tail_elems_1s()
> target/riscv/vector_helper.c: avoid env_archcpu() when reading
> RISCVCPUConfig
>
> target/riscv/vector_helper.c | 104 +++++++++++++----------------------
> 1 file changed, 39 insertions(+), 65 deletions(-)
Thanks, these are queued up. If we're already broken on
non-power-of-two then that ROUND_UP() suggestion might be worth looking
at, as I doubt we'd want to support them even if the ISA allows for it.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-02 2:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-26 17:05 [PATCH 0/2] target/riscv: some vector_helper.c cleanups Daniel Henrique Barboza
2023-02-26 17:05 ` [PATCH 1/2] target/riscv/vector_helper.c: create vext_set_tail_elems_1s() Daniel Henrique Barboza
2023-02-26 18:23 ` Philippe Mathieu-Daudé
2023-03-02 2:13 ` Palmer Dabbelt
2023-02-26 17:05 ` [PATCH 2/2] target/riscv/vector_helper.c: avoid env_archcpu() when reading RISCVCPUConfig Daniel Henrique Barboza
2023-02-26 18:24 ` Philippe Mathieu-Daudé
2023-02-27 0:58 ` liweiwei
2023-03-02 2:32 ` [PATCH 0/2] target/riscv: some vector_helper.c cleanups Palmer Dabbelt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).