qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes
@ 2024-04-30 16:31 Philippe Mathieu-Daudé
  2024-04-30 16:31 ` [PATCH v4 1/4] target/sh4: Fix ADDV opcode Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 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 =?unknown-8bit?q?Mathieu-Daud=C3=A9?=

Pass 'build-legacy' CI job which runs check-tcg-sh4.

Since v3:
- Fixed copy/paste mistake (Paul)
- Tests print values on failure
- Added rth R-b tags

Since v2:
- Add tests (Paul)
- Rename TCGv variables as in manual

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

 target/sh4/translate.c        | 32 ++++++++++++++++++++------------
 tests/tcg/sh4/test-addv.c     | 27 +++++++++++++++++++++++++++
 tests/tcg/sh4/test-subv.c     | 30 ++++++++++++++++++++++++++++++
 tests/tcg/sh4/Makefile.target |  6 ++++++
 4 files changed, 83 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/sh4/test-addv.c
 create mode 100644 tests/tcg/sh4/test-subv.c

-- 
2.41.0



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [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

* [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

* [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

* [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 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

* 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

* 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

* 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

end of thread, other threads:[~2024-05-03 12:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-05-01 12:55   ` Yoshinori Sato
2024-04-30 16:31 ` [PATCH v4 2/4] target/sh4: Fix SUBV opcode 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é
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-01 13:04   ` Yoshinori Sato
2024-05-03 12:54 ` [PATCH v4 0/4] target/sh4: Fix ADDV/SUBV opcodes Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).