* [PATCH] target/riscv: fix vssub.vv saturation bug
@ 2021-04-19 6:01 frank.chang
2021-04-19 6:25 ` Alistair Francis
0 siblings, 1 reply; 4+ messages in thread
From: frank.chang @ 2021-04-19 6:01 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Alistair Francis, Palmer Dabbelt, Sagar Karandikar,
Bastian Koppelmann
From: Frank Chang <frank.chang@sifive.com>
Doing a negate (0x0 – 0x80000000) using vssub.vv produces
an incorrect result of 0x80000000 (should saturate to 0x7fffffff)
Fix this bug by treating zero as a positive number.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
target/riscv/vector_helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index a156573d281..356cef8a090 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -2451,7 +2451,7 @@ static inline int8_t ssub8(CPURISCVState *env, int vxrm, int8_t a, int8_t b)
{
int8_t res = a - b;
if ((res ^ a) & (a ^ b) & INT8_MIN) {
- res = a > 0 ? INT8_MAX : INT8_MIN;
+ res = a >= 0 ? INT8_MAX : INT8_MIN;
env->vxsat = 0x1;
}
return res;
@@ -2461,7 +2461,7 @@ static inline int16_t ssub16(CPURISCVState *env, int vxrm, int16_t a, int16_t b)
{
int16_t res = a - b;
if ((res ^ a) & (a ^ b) & INT16_MIN) {
- res = a > 0 ? INT16_MAX : INT16_MIN;
+ res = a >= 0 ? INT16_MAX : INT16_MIN;
env->vxsat = 0x1;
}
return res;
@@ -2471,7 +2471,7 @@ static inline int32_t ssub32(CPURISCVState *env, int vxrm, int32_t a, int32_t b)
{
int32_t res = a - b;
if ((res ^ a) & (a ^ b) & INT32_MIN) {
- res = a > 0 ? INT32_MAX : INT32_MIN;
+ res = a >= 0 ? INT32_MAX : INT32_MIN;
env->vxsat = 0x1;
}
return res;
@@ -2481,7 +2481,7 @@ static inline int64_t ssub64(CPURISCVState *env, int vxrm, int64_t a, int64_t b)
{
int64_t res = a - b;
if ((res ^ a) & (a ^ b) & INT64_MIN) {
- res = a > 0 ? INT64_MAX : INT64_MIN;
+ res = a >= 0 ? INT64_MAX : INT64_MIN;
env->vxsat = 0x1;
}
return res;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] target/riscv: fix vssub.vv saturation bug
2021-04-19 6:01 [PATCH] target/riscv: fix vssub.vv saturation bug frank.chang
@ 2021-04-19 6:25 ` Alistair Francis
2021-04-19 6:31 ` Frank Chang
0 siblings, 1 reply; 4+ messages in thread
From: Alistair Francis @ 2021-04-19 6:25 UTC (permalink / raw)
To: Frank Chang
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Mon, Apr 19, 2021 at 4:02 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Doing a negate (0x0 – 0x80000000) using vssub.vv produces
> an incorrect result of 0x80000000 (should saturate to 0x7fffffff)
>
> Fix this bug by treating zero as a positive number.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
Thanks for the patch!
A similar fix is already in the riscv-to-apply.next queue
target/riscv: Fixup saturate subtract function
The overflow predication ((a - b) ^ a) & (a ^ b) & INT64_MIN is right.
However, when the predication is ture and a is 0, it should return maximum.
Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20210212150256.885-4-zhiwei_liu@c-sky.com
Message-Id: <20210212150256.885-4-zhiwei_liu@c-sky.com>
Alistair
> ---
> target/riscv/vector_helper.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index a156573d281..356cef8a090 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -2451,7 +2451,7 @@ static inline int8_t ssub8(CPURISCVState *env, int vxrm, int8_t a, int8_t b)
> {
> int8_t res = a - b;
> if ((res ^ a) & (a ^ b) & INT8_MIN) {
> - res = a > 0 ? INT8_MAX : INT8_MIN;
> + res = a >= 0 ? INT8_MAX : INT8_MIN;
> env->vxsat = 0x1;
> }
> return res;
> @@ -2461,7 +2461,7 @@ static inline int16_t ssub16(CPURISCVState *env, int vxrm, int16_t a, int16_t b)
> {
> int16_t res = a - b;
> if ((res ^ a) & (a ^ b) & INT16_MIN) {
> - res = a > 0 ? INT16_MAX : INT16_MIN;
> + res = a >= 0 ? INT16_MAX : INT16_MIN;
> env->vxsat = 0x1;
> }
> return res;
> @@ -2471,7 +2471,7 @@ static inline int32_t ssub32(CPURISCVState *env, int vxrm, int32_t a, int32_t b)
> {
> int32_t res = a - b;
> if ((res ^ a) & (a ^ b) & INT32_MIN) {
> - res = a > 0 ? INT32_MAX : INT32_MIN;
> + res = a >= 0 ? INT32_MAX : INT32_MIN;
> env->vxsat = 0x1;
> }
> return res;
> @@ -2481,7 +2481,7 @@ static inline int64_t ssub64(CPURISCVState *env, int vxrm, int64_t a, int64_t b)
> {
> int64_t res = a - b;
> if ((res ^ a) & (a ^ b) & INT64_MIN) {
> - res = a > 0 ? INT64_MAX : INT64_MIN;
> + res = a >= 0 ? INT64_MAX : INT64_MIN;
> env->vxsat = 0x1;
> }
> return res;
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] target/riscv: fix vssub.vv saturation bug
2021-04-19 6:25 ` Alistair Francis
@ 2021-04-19 6:31 ` Frank Chang
2021-04-19 8:49 ` Alistair Francis
0 siblings, 1 reply; 4+ messages in thread
From: Frank Chang @ 2021-04-19 6:31 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, Sagar Karandikar, Frank Chang,
Bastian Koppelmann, qemu-devel@nongnu.org Developers,
Alistair Francis, Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]
Alistair Francis <alistair23@gmail.com> 於 2021年4月19日 週一 下午2:28寫道：
> On Mon, Apr 19, 2021 at 4:02 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > Doing a negate (0x0 – 0x80000000) using vssub.vv produces
> > an incorrect result of 0x80000000 (should saturate to 0x7fffffff)
> >
> > Fix this bug by treating zero as a positive number.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>
> Thanks for the patch!
>
> A similar fix is already in the riscv-to-apply.next queue
>
> target/riscv: Fixup saturate subtract function
>
> The overflow predication ((a - b) ^ a) & (a ^ b) & INT64_MIN is right.
> However, when the predication is ture and a is 0, it should return
> maximum.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-id: 20210212150256.885-4-zhiwei_liu@c-sky.com
> Message-Id: <20210212150256.885-4-zhiwei_liu@c-sky.com>
>
>
> Alistair
>
Thanks, I might missed that patch.
Frank Chang
>
> > ---
> > target/riscv/vector_helper.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> > index a156573d281..356cef8a090 100644
> > --- a/target/riscv/vector_helper.c
> > +++ b/target/riscv/vector_helper.c
> > @@ -2451,7 +2451,7 @@ static inline int8_t ssub8(CPURISCVState *env, int
> vxrm, int8_t a, int8_t b)
> > {
> > int8_t res = a - b;
> > if ((res ^ a) & (a ^ b) & INT8_MIN) {
> > - res = a > 0 ? INT8_MAX : INT8_MIN;
> > + res = a >= 0 ? INT8_MAX : INT8_MIN;
> > env->vxsat = 0x1;
> > }
> > return res;
> > @@ -2461,7 +2461,7 @@ static inline int16_t ssub16(CPURISCVState *env,
> int vxrm, int16_t a, int16_t b)
> > {
> > int16_t res = a - b;
> > if ((res ^ a) & (a ^ b) & INT16_MIN) {
> > - res = a > 0 ? INT16_MAX : INT16_MIN;
> > + res = a >= 0 ? INT16_MAX : INT16_MIN;
> > env->vxsat = 0x1;
> > }
> > return res;
> > @@ -2471,7 +2471,7 @@ static inline int32_t ssub32(CPURISCVState *env,
> int vxrm, int32_t a, int32_t b)
> > {
> > int32_t res = a - b;
> > if ((res ^ a) & (a ^ b) & INT32_MIN) {
> > - res = a > 0 ? INT32_MAX : INT32_MIN;
> > + res = a >= 0 ? INT32_MAX : INT32_MIN;
> > env->vxsat = 0x1;
> > }
> > return res;
> > @@ -2481,7 +2481,7 @@ static inline int64_t ssub64(CPURISCVState *env,
> int vxrm, int64_t a, int64_t b)
> > {
> > int64_t res = a - b;
> > if ((res ^ a) & (a ^ b) & INT64_MIN) {
> > - res = a > 0 ? INT64_MAX : INT64_MIN;
> > + res = a >= 0 ? INT64_MAX : INT64_MIN;
> > env->vxsat = 0x1;
> > }
> > return res;
> > --
> > 2.17.1
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 4498 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] target/riscv: fix vssub.vv saturation bug
2021-04-19 6:31 ` Frank Chang
@ 2021-04-19 8:49 ` Alistair Francis
0 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2021-04-19 8:49 UTC (permalink / raw)
To: Frank Chang
Cc: open list:RISC-V, Sagar Karandikar, Frank Chang,
Bastian Koppelmann, qemu-devel@nongnu.org Developers,
Alistair Francis, Palmer Dabbelt
On Mon, Apr 19, 2021 at 4:31 PM Frank Chang <0xc0de0125@gmail.com> wrote:
>
> Alistair Francis <alistair23@gmail.com> 於 2021年4月19日 週一 下午2:28寫道：
>>
>> On Mon, Apr 19, 2021 at 4:02 PM <frank.chang@sifive.com> wrote:
>> >
>> > From: Frank Chang <frank.chang@sifive.com>
>> >
>> > Doing a negate (0x0 – 0x80000000) using vssub.vv produces
>> > an incorrect result of 0x80000000 (should saturate to 0x7fffffff)
>> >
>> > Fix this bug by treating zero as a positive number.
>> >
>> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>>
>> Thanks for the patch!
>>
>> A similar fix is already in the riscv-to-apply.next queue
>>
>> target/riscv: Fixup saturate subtract function
>>
>> The overflow predication ((a - b) ^ a) & (a ^ b) & INT64_MIN is right.
>> However, when the predication is ture and a is 0, it should return maximum.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Message-id: 20210212150256.885-4-zhiwei_liu@c-sky.com
>> Message-Id: <20210212150256.885-4-zhiwei_liu@c-sky.com>
>>
>>
>> Alistair
>
>
> Thanks, I might missed that patch.
No worries. Thanks anyway for the fix.
Alistair
> Frank Chang
>
>>
>>
>> > ---
>> > target/riscv/vector_helper.c | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
>> > index a156573d281..356cef8a090 100644
>> > --- a/target/riscv/vector_helper.c
>> > +++ b/target/riscv/vector_helper.c
>> > @@ -2451,7 +2451,7 @@ static inline int8_t ssub8(CPURISCVState *env, int vxrm, int8_t a, int8_t b)
>> > {
>> > int8_t res = a - b;
>> > if ((res ^ a) & (a ^ b) & INT8_MIN) {
>> > - res = a > 0 ? INT8_MAX : INT8_MIN;
>> > + res = a >= 0 ? INT8_MAX : INT8_MIN;
>> > env->vxsat = 0x1;
>> > }
>> > return res;
>> > @@ -2461,7 +2461,7 @@ static inline int16_t ssub16(CPURISCVState *env, int vxrm, int16_t a, int16_t b)
>> > {
>> > int16_t res = a - b;
>> > if ((res ^ a) & (a ^ b) & INT16_MIN) {
>> > - res = a > 0 ? INT16_MAX : INT16_MIN;
>> > + res = a >= 0 ? INT16_MAX : INT16_MIN;
>> > env->vxsat = 0x1;
>> > }
>> > return res;
>> > @@ -2471,7 +2471,7 @@ static inline int32_t ssub32(CPURISCVState *env, int vxrm, int32_t a, int32_t b)
>> > {
>> > int32_t res = a - b;
>> > if ((res ^ a) & (a ^ b) & INT32_MIN) {
>> > - res = a > 0 ? INT32_MAX : INT32_MIN;
>> > + res = a >= 0 ? INT32_MAX : INT32_MIN;
>> > env->vxsat = 0x1;
>> > }
>> > return res;
>> > @@ -2481,7 +2481,7 @@ static inline int64_t ssub64(CPURISCVState *env, int vxrm, int64_t a, int64_t b)
>> > {
>> > int64_t res = a - b;
>> > if ((res ^ a) & (a ^ b) & INT64_MIN) {
>> > - res = a > 0 ? INT64_MAX : INT64_MIN;
>> > + res = a >= 0 ? INT64_MAX : INT64_MIN;
>> > env->vxsat = 0x1;
>> > }
>> > return res;
>> > --
>> > 2.17.1
>> >
>> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-19 8:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-19 6:01 [PATCH] target/riscv: fix vssub.vv saturation bug frank.chang
2021-04-19 6:25 ` Alistair Francis
2021-04-19 6:31 ` Frank Chang
2021-04-19 8:49 ` Alistair Francis
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).