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

Mark Cave-Ayland posted 1 patch 10 weeks ago
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190207224258.426-1-mark.cave-ayland@ilande.co.uk
tcg/i386/tcg-target.inc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

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

Posted by Mark Cave-Ayland 10 weeks ago
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

Posted by Richard Henderson 10 weeks ago
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

Posted by Mark Cave-Ayland 10 weeks ago
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

Posted by Richard Henderson 10 weeks ago
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~