| Message ID | 20190207224258.426-1-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | tcg/i386: fix unsigned vector saturating arithmetic | expand |
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~
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.
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~
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
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(-)