target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

[PATCH] target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

Posted by Max Chou 2 months, 1 week ago

According to the Vector Reduction Operations section in the RISC-V "V"
Vector Extension spec,
"If vl=0, no operation is performed and the destination register is not
updated."

The vd should be updated when vl is larger than 0.

Signed-off-by: Max Chou <max.chou@sifive.com>
---
 target/riscv/vector_helper.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 5386e3b97c5..7773df6a7c7 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4659,7 +4659,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,          \
         }                                                 \
         s1 = OP(s1, (TD)s2);                              \
     }                                                     \
-    *((TD *)vd + HD(0)) = s1;                             \
+    if (vl > 0) {                                         \
+        *((TD *)vd + HD(0)) = s1;                         \
+    }                                                     \
     env->vstart = 0;                                      \
     /* set tail elements to 1s */                         \
     vext_set_elems_1s(vd, vta, esz, vlenb);               \
@@ -4745,7 +4747,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,           \
         }                                                  \
         s1 = OP(s1, (TD)s2, &env->fp_status);              \
     }                                                      \
-    *((TD *)vd + HD(0)) = s1;                              \
+    if (vl > 0) {                                          \
+        *((TD *)vd + HD(0)) = s1;                          \
+    }                                                      \
     env->vstart = 0;                                       \
     /* set tail elements to 1s */                          \
     vext_set_elems_1s(vd, vta, esz, vlenb);                \
-- 
2.34.1

Re: [PATCH] target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

Posted by Michael Tokarev 4 weeks, 1 day ago

24.01.2025 13:14, Max Chou wrote:
> According to the Vector Reduction Operations section in the RISC-V "V"
> Vector Extension spec,
> "If vl=0, no operation is performed and the destination register is not
> updated."
> 
> The vd should be updated when vl is larger than 0.

Is this a qemu-stable material?

If yes, how far to previous releases it is worth to pick?
(Current older stable series are 7.2 and 8.2).

I currently picked this up for 7.2, 8.2 and 9.2.

Thanks,

/mjt

Re: [PATCH] target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

Posted by Alistair Francis 4 weeks, 1 day ago

On Thu, Mar 6, 2025 at 4:27 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> 24.01.2025 13:14, Max Chou wrote:
> > According to the Vector Reduction Operations section in the RISC-V "V"
> > Vector Extension spec,
> > "If vl=0, no operation is performed and the destination register is not
> > updated."
> >
> > The vd should be updated when vl is larger than 0.
>
> Is this a qemu-stable material?

Yes

>
> If yes, how far to previous releases it is worth to pick?
> (Current older stable series are 7.2 and 8.2).
>
> I currently picked this up for 7.2, 8.2 and 9.2.

That's perfect

Alistair

>
> Thanks,
>
> /mjt
>

Re: [PATCH] target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

Posted by Alistair Francis 2 months ago

On Fri, Jan 24, 2025 at 8:16 PM Max Chou <max.chou@sifive.com> wrote:
>
> According to the Vector Reduction Operations section in the RISC-V "V"
> Vector Extension spec,
> "If vl=0, no operation is performed and the destination register is not
> updated."
>
> The vd should be updated when vl is larger than 0.
>
> Signed-off-by: Max Chou <max.chou@sifive.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/vector_helper.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 5386e3b97c5..7773df6a7c7 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -4659,7 +4659,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,          \
>          }                                                 \
>          s1 = OP(s1, (TD)s2);                              \
>      }                                                     \
> -    *((TD *)vd + HD(0)) = s1;                             \
> +    if (vl > 0) {                                         \
> +        *((TD *)vd + HD(0)) = s1;                         \
> +    }                                                     \
>      env->vstart = 0;                                      \
>      /* set tail elements to 1s */                         \
>      vext_set_elems_1s(vd, vta, esz, vlenb);               \
> @@ -4745,7 +4747,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,           \
>          }                                                  \
>          s1 = OP(s1, (TD)s2, &env->fp_status);              \
>      }                                                      \
> -    *((TD *)vd + HD(0)) = s1;                              \
> +    if (vl > 0) {                                          \
> +        *((TD *)vd + HD(0)) = s1;                          \
> +    }                                                      \
>      env->vstart = 0;                                       \
>      /* set tail elements to 1s */                          \
>      vext_set_elems_1s(vd, vta, esz, vlenb);                \
> --
> 2.34.1
>
>

Re: [PATCH] target/riscv: rvv: Fix unexpected behavior of vector reduction instructions when vl is 0

Posted by Daniel Henrique Barboza 2 months, 1 week ago


On 1/24/25 7:14 AM, Max Chou wrote:
> According to the Vector Reduction Operations section in the RISC-V "V"
> Vector Extension spec,
> "If vl=0, no operation is performed and the destination register is not
> updated."
> 
> The vd should be updated when vl is larger than 0.
> 

Fixes: fe5c9ab1fc ("target/riscv: vector single-width integer reduction instructions")
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")

> Signed-off-by: Max Chou <max.chou@sifive.com>

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

> ---
>   target/riscv/vector_helper.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 5386e3b97c5..7773df6a7c7 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -4659,7 +4659,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,          \
>           }                                                 \
>           s1 = OP(s1, (TD)s2);                              \
>       }                                                     \
> -    *((TD *)vd + HD(0)) = s1;                             \
> +    if (vl > 0) {                                         \
> +        *((TD *)vd + HD(0)) = s1;                         \
> +    }                                                     \
>       env->vstart = 0;                                      \
>       /* set tail elements to 1s */                         \
>       vext_set_elems_1s(vd, vta, esz, vlenb);               \
> @@ -4745,7 +4747,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,           \
>           }                                                  \
>           s1 = OP(s1, (TD)s2, &env->fp_status);              \
>       }                                                      \
> -    *((TD *)vd + HD(0)) = s1;                              \
> +    if (vl > 0) {                                          \
> +        *((TD *)vd + HD(0)) = s1;                          \
> +    }                                                      \
>       env->vstart = 0;                                       \
>       /* set tail elements to 1s */                          \
>       vext_set_elems_1s(vd, vta, esz, vlenb);                \