[v1] target/riscv: Fix vcompress with rvv_ta_all_1s

[PATCH] target/riscv: Fix vcompress with rvv_ta_all_1s

Anton Blanchard posted 1 patch 6 months, 3 weeks ago

Diff against v2
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241030004146.935656-1-antonb@tenstorrent.com

Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>

There is a newer version of this series

target/riscv/vector_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Expand all Fold all

[PATCH] target/riscv: Fix vcompress with rvv_ta_all_1s

Posted by Anton Blanchard 6 months, 3 weeks ago

vcompress packs vl or less fields into vd, so the tail starts after the
last packed field.

Signed-off-by: Anton Blanchard <antonb@tenstorrent.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 072bd444b1..ccb32e6122 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5132,7 +5132,7 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2,               \
     }                                                                     \
     env->vstart = 0;                                                      \
     /* set tail elements to 1s */                                         \
-    vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);              \
+    vext_set_elems_1s(vd, vta, num * esz, total_elems * esz);             \
 }
 
 /* Compress into vd elements of vs2 where vs1 is enabled */
-- 
2.34.1

Re: [PATCH] target/riscv: Fix vcompress with rvv_ta_all_1s

Posted by Alistair Francis 6 months, 3 weeks ago

On Wed, Oct 30, 2024 at 11:13 AM Anton Blanchard <antonb@tenstorrent.com> wrote:
>
> vcompress packs vl or less fields into vd, so the tail starts after the
> last packed field.

Is that right?

It's different from every other vector command. Although the wording
in the spec is very confusing

Alistair

Re: [CAUTION - External Sender] Re: [PATCH] target/riscv: Fix vcompress with rvv_ta_all_1s

Posted by Anton Blanchard 6 months, 3 weeks ago

Hi Alistair,

On Wed, Oct 30, 2024 at 2:39 PM Alistair Francis <alistair23@gmail.com>
wrote:
> > vcompress packs vl or less fields into vd, so the tail starts after the
> > last packed field.
>
> Is that right?
>
> It's different from every other vector command. Although the wording
> in the spec is very confusing

It is confusing. This thread has some clarification, and we should probably
follow up on the suggestion to improve the ISA wording:

https://github.com/riscv/riscv-v-spec/issues/796

Thanks,
Anton

Re: [CAUTION - External Sender] Re: [PATCH] target/riscv: Fix vcompress with rvv_ta_all_1s

Posted by Alistair Francis 6 months, 3 weeks ago

On Wed, Oct 30, 2024 at 2:09 PM Anton Blanchard <antonb@tenstorrent.com> wrote:
>
> Hi Alistair,
>
> On Wed, Oct 30, 2024 at 2:39 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > vcompress packs vl or less fields into vd, so the tail starts after the
> > > last packed field.
> >
> > Is that right?
> >
> > It's different from every other vector command. Although the wording
> > in the spec is very confusing
>
> It is confusing. This thread has some clarification, and we should probably
> follow up on the suggestion to improve the ISA wording:
>
> https://github.com/riscv/riscv-v-spec/issues/796

Ah, at least it isn't just me.

It's worth pointing to that in the commit message, as the spec is not
clear about this

Alistair

>
> Thanks,
> Anton