[Qemu-devel] [PATCH] tcg/i386: fix unsigned vector saturating arithmetic

[Qemu-devel] [PATCH] tcg/i386: fix unsigned vector saturating arithmetic

[Mark Cave-Ayland]

Due to a cut/paste error in the original implementation, the unsigned vector
saturating arithmetic was erroneously being calculated as signed vector saturating
arithmetic.

Fixes: 8ffafbcec2 ("tcg/i386: Implement vector saturating arithmetic")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 tcg/i386/tcg-target.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 4d84aea3a9..e0670e5098 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2615,7 +2615,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
         OPC_PADDSB, OPC_PADDSW, OPC_UD2, OPC_UD2
     };
     static int const usadd_insn[4] = {
-        OPC_PADDSB, OPC_PADDSW, OPC_UD2, OPC_UD2
+        OPC_PADDUB, OPC_PADDUW, OPC_UD2, OPC_UD2
     };
     static int const sub_insn[4] = {
         OPC_PSUBB, OPC_PSUBW, OPC_PSUBD, OPC_PSUBQ
@@ -2624,7 +2624,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
         OPC_PSUBSB, OPC_PSUBSW, OPC_UD2, OPC_UD2
     };
     static int const ussub_insn[4] = {
-        OPC_PSUBSB, OPC_PSUBSW, OPC_UD2, OPC_UD2
+        OPC_PSUBUB, OPC_PSUBUW, OPC_UD2, OPC_UD2
     };
     static int const mul_insn[4] = {
         OPC_UD2, OPC_PMULLW, OPC_PMULLD, OPC_UD2
-- 
2.11.0

Re: [Qemu-devel] [PATCH] tcg/i386: fix unsigned vector saturating arithmetic

[Richard Henderson]

On 2/7/19 2:42 PM, Mark Cave-Ayland wrote:
> Due to a cut/paste error in the original implementation, the unsigned vector
> saturating arithmetic was erroneously being calculated as signed vector saturating
> arithmetic.
> 
> Fixes: 8ffafbcec2 ("tcg/i386: Implement vector saturating arithmetic")
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  tcg/i386/tcg-target.inc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Ooof.  Thanks.


r~

Re: [Qemu-devel] [PATCH] tcg/i386: fix unsigned vector saturating arithmetic

[Mark Cave-Ayland]

On 08/02/2019 17:39, Richard Henderson wrote:

> On 2/7/19 2:42 PM, Mark Cave-Ayland wrote:
>> Due to a cut/paste error in the original implementation, the unsigned vector
>> saturating arithmetic was erroneously being calculated as signed vector saturating
>> arithmetic.
>>
>> Fixes: 8ffafbcec2 ("tcg/i386: Implement vector saturating arithmetic")
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  tcg/i386/tcg-target.inc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Ooof.  Thanks.

AFAICT none of the other TCG backends currently make use of it, but it was this bug
causing the graphical corruption that myself and Howard saw testing the PPC vector
patchset.

FWIW I've now rebased and repushed the outstanding patches from your patchset along
with this fix to https://github.com/mcayland/qemu/commits/ppc-altivec-v6 as it would
be great if this could make it into 4.0.


ATB,

Mark.

Re: [Qemu-devel] [PATCH] tcg/i386: fix unsigned vector saturating arithmetic

[Richard Henderson]

On 2/8/19 10:09 AM, Mark Cave-Ayland wrote:
> On 08/02/2019 17:39, Richard Henderson wrote:
> 
>> On 2/7/19 2:42 PM, Mark Cave-Ayland wrote:
>>> Due to a cut/paste error in the original implementation, the unsigned vector
>>> saturating arithmetic was erroneously being calculated as signed vector saturating
>>> arithmetic.
>>>
>>> Fixes: 8ffafbcec2 ("tcg/i386: Implement vector saturating arithmetic")
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>  tcg/i386/tcg-target.inc.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Ooof.  Thanks.
> 
> AFAICT none of the other TCG backends currently make use of it, but it was this bug
> causing the graphical corruption that myself and Howard saw testing the PPC vector
> patchset.

I would have seen the error if I had done ARM SVE regression testing.
But I only did ARM NEON testing, which does not make use of this, and
I had forgotten that.

Like PPC, NEON sets a "saw saturation" bit.  I think I'll do the same
trick as we did for ppc to compute that with vector insns.


> FWIW I've now rebased and repushed the outstanding patches from your patchset along
> with this fix to https://github.com/mcayland/qemu/commits/ppc-altivec-v6 as it would
> be great if this could make it into 4.0.

I've given all of my r-b...


r~