* [PATCH v4 1/4] target/sh4: Fix ADDV opcode
2024-04-30 16:31 [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
@ 2024-04-30 16:31 ` Philippe Mathieu-Daudé
2024-05-01 12:55 ` Yoshinori Sato
2024-04-30 16:31 ` [PATCH v4 2/4] target/sh4: Fix SUBV opcode Philippe Mathieu-Daudé
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Yoshinori Sato, John Paul Adrian Glaubitz, Paul Cercueil,
Philippe Mathieu-Daudé, qemu-stable, Richard Henderson
The documentation says:
ADDV Rm, Rn Rn + Rm -> Rn, overflow -> T
But QEMU implementation was:
ADDV Rm, Rn Rn + Rm -> Rm, overflow -> T
Fix by filling the correct Rm register.
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>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2317
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-addv.c | 27 +++++++++++++++++++++++++++
tests/tcg/sh4/Makefile.target | 3 +++
3 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/sh4/test-addv.c
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index ebb6c901bf..4a1dd0d1f4 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -714,7 +714,7 @@ static void _decode_opc(DisasContext * ctx)
tcg_gen_xor_i32(t2, REG(B7_4), REG(B11_8));
tcg_gen_andc_i32(cpu_sr_t, t1, t2);
tcg_gen_shri_i32(cpu_sr_t, cpu_sr_t, 31);
- tcg_gen_mov_i32(REG(B7_4), t0);
+ tcg_gen_mov_i32(REG(B11_8), t0);
}
return;
case 0x2009: /* and Rm,Rn */
diff --git a/tests/tcg/sh4/test-addv.c b/tests/tcg/sh4/test-addv.c
new file mode 100644
index 0000000000..64f709161f
--- /dev/null
+++ b/tests/tcg/sh4/test-addv.c
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void addv(const int a, const int b, const int res, const int carry)
+{
+ int o = a, c;
+
+ asm volatile("addv %2,%0\n"
+ "movt %1\n"
+ : "+r"(o), "=r"(c) : "r"(b) :);
+
+ if (c != carry || aw != res) {
+ printf("ADDV %d, %d = %d/%d [T = %d/%d]\n", a, b, o, res, c, carry);
+ abort();
+ }
+}
+
+int main(void)
+{
+ addv(INT_MAX, 1, INT_MIN, 1);
+ addv(INT_MAX - 1, 1, INT_MAX, 0);
+
+ return 0;
+}
diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target
index 4d09291c0c..521b8b0a76 100644
--- a/tests/tcg/sh4/Makefile.target
+++ b/tests/tcg/sh4/Makefile.target
@@ -17,3 +17,6 @@ TESTS += test-macl
test-macw: CFLAGS += -O -g
TESTS += test-macw
+
+test-addv: CFLAGS += -O -g
+TESTS += test-addv
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 1/4] target/sh4: Fix ADDV opcode
2024-04-30 16:31 ` [PATCH v4 1/4] target/sh4: Fix ADDV opcode Philippe Mathieu-Daudé
@ 2024-05-01 12:55 ` Yoshinori Sato
0 siblings, 0 replies; 10+ messages in thread
From: Yoshinori Sato @ 2024-05-01 12:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, John Paul Adrian Glaubitz, Paul Cercueil, qemu-stable,
Richard Henderson
On Wed, 01 May 2024 01:31:22 +0900,
Philippe Mathieu-Daudé wrote:
>
> The documentation says:
>
> ADDV Rm, Rn Rn + Rm -> Rn, overflow -> T
>
> But QEMU implementation was:
>
> ADDV Rm, Rn Rn + Rm -> Rm, overflow -> T
>
> Fix by filling the correct Rm register.
>
> 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>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2317
> 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-addv.c | 27 +++++++++++++++++++++++++++
> tests/tcg/sh4/Makefile.target | 3 +++
> 3 files changed, 31 insertions(+), 1 deletion(-)
> create mode 100644 tests/tcg/sh4/test-addv.c
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index ebb6c901bf..4a1dd0d1f4 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -714,7 +714,7 @@ static void _decode_opc(DisasContext * ctx)
> tcg_gen_xor_i32(t2, REG(B7_4), REG(B11_8));
> tcg_gen_andc_i32(cpu_sr_t, t1, t2);
> tcg_gen_shri_i32(cpu_sr_t, cpu_sr_t, 31);
> - tcg_gen_mov_i32(REG(B7_4), t0);
> + tcg_gen_mov_i32(REG(B11_8), t0);
> }
> return;
> case 0x2009: /* and Rm,Rn */
> diff --git a/tests/tcg/sh4/test-addv.c b/tests/tcg/sh4/test-addv.c
> new file mode 100644
> index 0000000000..64f709161f
> --- /dev/null
> +++ b/tests/tcg/sh4/test-addv.c
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <limits.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +static void addv(const int a, const int b, const int res, const int carry)
> +{
> + int o = a, c;
> +
> + asm volatile("addv %2,%0\n"
> + "movt %1\n"
> + : "+r"(o), "=r"(c) : "r"(b) :);
> +
> + if (c != carry || aw != res) {
> + printf("ADDV %d, %d = %d/%d [T = %d/%d]\n", a, b, o, res, c, carry);
> + abort();
> + }
> +}
> +
> +int main(void)
> +{
> + addv(INT_MAX, 1, INT_MIN, 1);
> + addv(INT_MAX - 1, 1, INT_MAX, 0);
> +
> + return 0;
> +}
> diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target
> index 4d09291c0c..521b8b0a76 100644
> --- a/tests/tcg/sh4/Makefile.target
> +++ b/tests/tcg/sh4/Makefile.target
> @@ -17,3 +17,6 @@ TESTS += test-macl
>
> test-macw: CFLAGS += -O -g
> TESTS += test-macw
> +
> +test-addv: CFLAGS += -O -g
> +TESTS += test-addv
> --
> 2.41.0
>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/4] target/sh4: Fix SUBV opcode
2024-04-30 16:31 [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
2024-04-30 16:31 ` [PATCH v4 1/4] target/sh4: Fix ADDV opcode Philippe Mathieu-Daudé
@ 2024-04-30 16:31 ` Philippe Mathieu-Daudé
2024-05-01 12:56 ` Yoshinori Sato
2024-04-30 16:31 ` [PATCH v4 3/4] target/sh4: Rename TCGv variables as manual for ADDV opcode Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Yoshinori Sato, John Paul Adrian Glaubitz, Paul Cercueil,
Philippe Mathieu-Daudé, qemu-stable, Richard Henderson
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 2/4] target/sh4: Fix SUBV opcode
2024-04-30 16:31 ` [PATCH v4 2/4] target/sh4: Fix SUBV opcode Philippe Mathieu-Daudé
@ 2024-05-01 12:56 ` Yoshinori Sato
0 siblings, 0 replies; 10+ messages in thread
From: Yoshinori Sato @ 2024-05-01 12:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, John Paul Adrian Glaubitz, Paul Cercueil, qemu-stable,
Richard Henderson
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>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/4] target/sh4: Rename TCGv variables as manual for ADDV opcode
2024-04-30 16:31 [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
2024-04-30 16:31 ` [PATCH v4 1/4] target/sh4: Fix ADDV opcode Philippe Mathieu-Daudé
2024-04-30 16:31 ` [PATCH v4 2/4] target/sh4: Fix SUBV opcode Philippe Mathieu-Daudé
@ 2024-04-30 16:31 ` Philippe Mathieu-Daudé
2024-05-01 13:04 ` Yoshinori Sato
2024-04-30 16:31 ` [PATCH v4 4/4] target/sh4: Rename TCGv variables as manual for SUBV opcode Philippe Mathieu-Daudé
2024-05-03 12:54 ` [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Yoshinori Sato, John Paul Adrian Glaubitz, Paul Cercueil,
Philippe Mathieu-Daudé, Richard Henderson
To easily compare with the SH4 manual, rename:
REG(B11_8) -> Rn
REG(B7_4) -> Rm
t0 -> result
Mention how overflow is calculated.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/sh4/translate.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 3e013b7c7c..47c0f3404e 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -705,16 +705,20 @@ static void _decode_opc(DisasContext * ctx)
return;
case 0x300f: /* addv Rm,Rn */
{
- TCGv t0, t1, t2;
- t0 = tcg_temp_new();
- tcg_gen_add_i32(t0, REG(B7_4), REG(B11_8));
+ TCGv Rn = REG(B11_8);
+ TCGv Rm = REG(B7_4);
+ TCGv result, t1, t2;
+
+ result = tcg_temp_new();
t1 = tcg_temp_new();
- tcg_gen_xor_i32(t1, t0, REG(B11_8));
t2 = tcg_temp_new();
- tcg_gen_xor_i32(t2, REG(B7_4), REG(B11_8));
+ tcg_gen_add_i32(result, Rm, Rn);
+ /* T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 */
+ tcg_gen_xor_i32(t1, result, Rn);
+ tcg_gen_xor_i32(t2, Rm, Rn);
tcg_gen_andc_i32(cpu_sr_t, t1, t2);
tcg_gen_shri_i32(cpu_sr_t, cpu_sr_t, 31);
- tcg_gen_mov_i32(REG(B11_8), t0);
+ tcg_gen_mov_i32(Rn, result);
}
return;
case 0x2009: /* and Rm,Rn */
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 3/4] target/sh4: Rename TCGv variables as manual for ADDV opcode
2024-04-30 16:31 ` [PATCH v4 3/4] target/sh4: Rename TCGv variables as manual for ADDV opcode Philippe Mathieu-Daudé
@ 2024-05-01 13:04 ` Yoshinori Sato
0 siblings, 0 replies; 10+ messages in thread
From: Yoshinori Sato @ 2024-05-01 13:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, John Paul Adrian Glaubitz, Paul Cercueil,
Richard Henderson
On Wed, 01 May 2024 01:31:24 +0900,
Philippe Mathieu-Daudé wrote:
>
> To easily compare with the SH4 manual, rename:
>
> REG(B11_8) -> Rn
> REG(B7_4) -> Rm
> t0 -> result
>
> Mention how overflow is calculated.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/sh4/translate.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 3e013b7c7c..47c0f3404e 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -705,16 +705,20 @@ static void _decode_opc(DisasContext * ctx)
> return;
> case 0x300f: /* addv Rm,Rn */
> {
> - TCGv t0, t1, t2;
> - t0 = tcg_temp_new();
> - tcg_gen_add_i32(t0, REG(B7_4), REG(B11_8));
> + TCGv Rn = REG(B11_8);
> + TCGv Rm = REG(B7_4);
> + TCGv result, t1, t2;
> +
> + result = tcg_temp_new();
> t1 = tcg_temp_new();
> - tcg_gen_xor_i32(t1, t0, REG(B11_8));
> t2 = tcg_temp_new();
> - tcg_gen_xor_i32(t2, REG(B7_4), REG(B11_8));
> + tcg_gen_add_i32(result, Rm, Rn);
> + /* T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 */
> + tcg_gen_xor_i32(t1, result, Rn);
> + tcg_gen_xor_i32(t2, Rm, Rn);
> tcg_gen_andc_i32(cpu_sr_t, t1, t2);
> tcg_gen_shri_i32(cpu_sr_t, cpu_sr_t, 31);
> - tcg_gen_mov_i32(REG(B11_8), t0);
> + tcg_gen_mov_i32(Rn, result);
> }
> return;
> case 0x2009: /* and Rm,Rn */
> --
> 2.41.0
>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 4/4] target/sh4: Rename TCGv variables as manual for SUBV opcode
2024-04-30 16:31 [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2024-04-30 16:31 ` [PATCH v4 3/4] target/sh4: Rename TCGv variables as manual for ADDV opcode Philippe Mathieu-Daudé
@ 2024-04-30 16:31 ` Philippe Mathieu-Daudé
2024-05-01 13:04 ` Yoshinori Sato
2024-05-03 12:54 ` [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 16:31 UTC (permalink / raw)
To: qemu-devel
Cc: Yoshinori Sato, John Paul Adrian Glaubitz, Paul Cercueil,
Philippe Mathieu-Daudé, Richard Henderson
To easily compare with the SH4 manual, rename:
REG(B11_8) -> Rn
REG(B7_4) -> Rm
t0 -> result
Mention how underflow is calculated.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/sh4/translate.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 47c0f3404e..e599ab9d1a 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -933,16 +933,20 @@ static void _decode_opc(DisasContext * ctx)
return;
case 0x300b: /* subv Rm,Rn */
{
- TCGv t0, t1, t2;
- t0 = tcg_temp_new();
- tcg_gen_sub_i32(t0, REG(B11_8), REG(B7_4));
+ TCGv Rn = REG(B11_8);
+ TCGv Rm = REG(B7_4);
+ TCGv result, t1, t2;
+
+ result = tcg_temp_new();
t1 = tcg_temp_new();
- 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_sub_i32(result, Rn, Rm);
+ /* T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 */
+ tcg_gen_xor_i32(t1, result, Rn);
+ tcg_gen_xor_i32(t2, Rn, Rm);
tcg_gen_and_i32(t1, t1, t2);
tcg_gen_shri_i32(cpu_sr_t, t1, 31);
- tcg_gen_mov_i32(REG(B11_8), t0);
+ tcg_gen_mov_i32(Rn, result);
}
return;
case 0x2008: /* tst Rm,Rn */
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 4/4] target/sh4: Rename TCGv variables as manual for SUBV opcode
2024-04-30 16:31 ` [PATCH v4 4/4] target/sh4: Rename TCGv variables as manual for SUBV opcode Philippe Mathieu-Daudé
@ 2024-05-01 13:04 ` Yoshinori Sato
0 siblings, 0 replies; 10+ messages in thread
From: Yoshinori Sato @ 2024-05-01 13:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, John Paul Adrian Glaubitz, Paul Cercueil,
Richard Henderson
On Wed, 01 May 2024 01:31:25 +0900,
Philippe Mathieu-Daudé wrote:
>
> To easily compare with the SH4 manual, rename:
>
> REG(B11_8) -> Rn
> REG(B7_4) -> Rm
> t0 -> result
>
> Mention how underflow is calculated.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> target/sh4/translate.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 47c0f3404e..e599ab9d1a 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -933,16 +933,20 @@ static void _decode_opc(DisasContext * ctx)
> return;
> case 0x300b: /* subv Rm,Rn */
> {
> - TCGv t0, t1, t2;
> - t0 = tcg_temp_new();
> - tcg_gen_sub_i32(t0, REG(B11_8), REG(B7_4));
> + TCGv Rn = REG(B11_8);
> + TCGv Rm = REG(B7_4);
> + TCGv result, t1, t2;
> +
> + result = tcg_temp_new();
> t1 = tcg_temp_new();
> - 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_sub_i32(result, Rn, Rm);
> + /* T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31 */
> + tcg_gen_xor_i32(t1, result, Rn);
> + tcg_gen_xor_i32(t2, Rn, Rm);
> tcg_gen_and_i32(t1, t1, t2);
> tcg_gen_shri_i32(cpu_sr_t, t1, 31);
> - tcg_gen_mov_i32(REG(B11_8), t0);
> + tcg_gen_mov_i32(Rn, result);
> }
> return;
> case 0x2008: /* tst Rm,Rn */
> --
> 2.41.0
>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes
2024-04-30 16:31 [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2024-04-30 16:31 ` [PATCH v4 4/4] target/sh4: Rename TCGv variables as manual for SUBV opcode Philippe Mathieu-Daudé
@ 2024-05-03 12:54 ` Philippe Mathieu-Daudé
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-03 12:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato, John Paul Adrian Glaubitz, Paul Cercueil
On 30/4/24 18:31, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (4):
> target/sh4: Fix ADDV opcode
> target/sh4: Fix SUBV opcode
> target/sh4: Rename TCGv variables as manual for ADDV opcode
> target/sh4: Rename TCGv variables as manual for SUBV opcode
Series queued, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread