| Message ID | 20240430163125.77430-3-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/sh4: Fix ADDV/SUBV opcodes | expand |
On Wed, 01 May 2024 01:31:23 +0900, Philippe Mathieu-Daudé wrote: > > The documentation says: > > SUBV Rm, Rn Rn - Rm -> Rn, underflow -> T > > The overflow / underflow can be calculated as: > > T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 > > However we were using the incorrect: > > T = ((Rn ^ Rm) & (Result ^ Rm)) >> 31 > > Fix by using the Rn register instead of Rm. > > Add tests provided by Paul Cercueil. > > Cc: qemu-stable@nongnu.org > Fixes: ad8d25a11f ("target-sh4: implement addv and subv using TCG") > Reported-by: Paul Cercueil <paul@crapouillou.net> > Suggested-by: Paul Cercueil <paul@crapouillou.net> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2318 > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > target/sh4/translate.c | 2 +- > tests/tcg/sh4/test-subv.c | 30 ++++++++++++++++++++++++++++++ > tests/tcg/sh4/Makefile.target | 3 +++ > 3 files changed, 34 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/sh4/test-subv.c > > diff --git a/target/sh4/translate.c b/target/sh4/translate.c > index 4a1dd0d1f4..3e013b7c7c 100644 > --- a/target/sh4/translate.c > +++ b/target/sh4/translate.c > @@ -933,7 +933,7 @@ static void _decode_opc(DisasContext * ctx) > t0 = tcg_temp_new(); > tcg_gen_sub_i32(t0, REG(B11_8), REG(B7_4)); > t1 = tcg_temp_new(); > - tcg_gen_xor_i32(t1, t0, REG(B7_4)); > + tcg_gen_xor_i32(t1, t0, REG(B11_8)); > t2 = tcg_temp_new(); > tcg_gen_xor_i32(t2, REG(B11_8), REG(B7_4)); > tcg_gen_and_i32(t1, t1, t2); > diff --git a/tests/tcg/sh4/test-subv.c b/tests/tcg/sh4/test-subv.c > new file mode 100644 > index 0000000000..0dd8fcdaac > --- /dev/null > +++ b/tests/tcg/sh4/test-subv.c > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > + > +#include <limits.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +static void subv(const int a, const int b, const int res, const int carry) > +{ > + int o = a, c; > + > + asm volatile("subv %2,%0\n" > + "movt %1\n" > + : "+r"(o), "=r"(c) : "r"(b) :); > + > + if (c != carry || o != res) { > + printf("SUBV %d, %d = %d/%d [T = %d/%d]\n", a, b, o, res, c, carry); > + abort(); > + } > +} > + > +int main(void) > +{ > + subv(INT_MIN, 1, INT_MAX, 1); > + subv(INT_MAX, -1, INT_MIN, 1); > + subv(INT_MAX, 1, INT_MAX - 1, 0); > + subv(0, 1, -1, 0); > + subv(-1, -1, 0, 0); > + > + return 0; > +} > diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target > index 521b8b0a76..7852fa62d8 100644 > --- a/tests/tcg/sh4/Makefile.target > +++ b/tests/tcg/sh4/Makefile.target > @@ -20,3 +20,6 @@ TESTS += test-macw > > test-addv: CFLAGS += -O -g > TESTS += test-addv > + > +test-subv: CFLAGS += -O -g > +TESTS += test-subv > -- > 2.41.0 > Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 4a1dd0d1f4..3e013b7c7c 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -933,7 +933,7 @@ static void _decode_opc(DisasContext * ctx) t0 = tcg_temp_new(); tcg_gen_sub_i32(t0, REG(B11_8), REG(B7_4)); t1 = tcg_temp_new(); - tcg_gen_xor_i32(t1, t0, REG(B7_4)); + tcg_gen_xor_i32(t1, t0, REG(B11_8)); t2 = tcg_temp_new(); tcg_gen_xor_i32(t2, REG(B11_8), REG(B7_4)); tcg_gen_and_i32(t1, t1, t2); diff --git a/tests/tcg/sh4/test-subv.c b/tests/tcg/sh4/test-subv.c new file mode 100644 index 0000000000..0dd8fcdaac --- /dev/null +++ b/tests/tcg/sh4/test-subv.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <limits.h> +#include <stdio.h> +#include <stdlib.h> + +static void subv(const int a, const int b, const int res, const int carry) +{ + int o = a, c; + + asm volatile("subv %2,%0\n" + "movt %1\n" + : "+r"(o), "=r"(c) : "r"(b) :); + + if (c != carry || o != res) { + printf("SUBV %d, %d = %d/%d [T = %d/%d]\n", a, b, o, res, c, carry); + abort(); + } +} + +int main(void) +{ + subv(INT_MIN, 1, INT_MAX, 1); + subv(INT_MAX, -1, INT_MIN, 1); + subv(INT_MAX, 1, INT_MAX - 1, 0); + subv(0, 1, -1, 0); + subv(-1, -1, 0, 0); + + return 0; +} diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target index 521b8b0a76..7852fa62d8 100644 --- a/tests/tcg/sh4/Makefile.target +++ b/tests/tcg/sh4/Makefile.target @@ -20,3 +20,6 @@ TESTS += test-macw test-addv: CFLAGS += -O -g TESTS += test-addv + +test-subv: CFLAGS += -O -g +TESTS += test-subv
The documentation says: SUBV Rm, Rn Rn - Rm -> Rn, underflow -> T The overflow / underflow can be calculated as: T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 However we were using the incorrect: T = ((Rn ^ Rm) & (Result ^ Rm)) >> 31 Fix by using the Rn register instead of Rm. Add tests provided by Paul Cercueil. Cc: qemu-stable@nongnu.org Fixes: ad8d25a11f ("target-sh4: implement addv and subv using TCG") Reported-by: Paul Cercueil <paul@crapouillou.net> Suggested-by: Paul Cercueil <paul@crapouillou.net> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2318 Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- target/sh4/translate.c | 2 +- tests/tcg/sh4/test-subv.c | 30 ++++++++++++++++++++++++++++++ tests/tcg/sh4/Makefile.target | 3 +++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/sh4/test-subv.c