On 4/27/23 22:22, Weiwei Li wrote:
On 2023/4/28 04:57, Daniel Henrique Barboza wrote:
Commit 3479a814 ("target/riscv: rvv-1.0: add VMA and VTA") added vma and
vta fields in the vtype register, while also defining that QEMU doesn't
need to have a tail agnostic policy to be compliant with the RVV spec.
It ended up removing all tail handling code as well. Later, commit
752614ca ("target/riscv: rvv: Add tail agnostic for vector load / store
instructions") reintroduced the tail agnostic fill for vector load/store
instructions only.
This puts QEMU in a situation where some functions are 1-filling the
tail elements and others don't. This is still a valid implementation,
but the process of 1-filling the tail elements takes valuable emulation
time that can be used doing anything else. If the spec doesn't demand a
specific tail-agostic policy, a proper software wouldn't expect any
policy to be in place. This means that, more often than not, the work
we're doing by 1-filling tail elements is wasted. We would be better of
if vext_set_tail_elems_1s() is removed entirely from the code.
All this said, there's still a debug value associated with it. So,
instead of removing it, let's gate it with cpu->cfg.debug. This way
software can enable this code if desirable, but for the regular case we
shouldn't waste time with it.
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
target/riscv/vector_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 8e6c99e573..e0a292ac24 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 vta = vext_vta(desc);
int k;
- if (vta == 0) {
+ if (vta == 0 || !riscv_cpu_cfg(env)->debug) {
I think this is not correct. 'debug' property is used for debug spec. And this
feature is controlled by another property 'rvv_ta_all_1s' .
You're right. I wasn't aware that this flag exists:
$ git grep 'rvv_ta_all_1s'
target/riscv/cpu.c: DEFINE_PROP_BOOL("rvv_ta_all_1s", RISCVCPU,
cfg.rvv_ta_all_1s, false),
target/riscv/cpu.h: bool rvv_ta_all_1s;
target/riscv/translate.c: ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) &&
cpu->cfg.rvv_ta_all_1s;
target/riscv/translate.c: ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
By the way, cfg.rvv_ta_all_1s have been ANDed intovta value. So additional
check on it is also unnecessary here.
Yes. We can drop this patch then since 'vta' is already accounting for
ta_all_1s.
Thanks,
Daniel
Regards,
Weiwei Li
return;
}
