[PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw]

[PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw]

[Richard Henderson]

Zack's recent patches, tidied a little bit, and with
test cases added.

r~


Richard Henderson (1):
  target/sh4: Merge mach and macl into a union

Zack Buhman (3):
  target/sh4: mac.w: memory accesses are 16-bit words
  target/sh4: Fix mac.l with saturation enabled
  target/sh4: Fix mac.w with saturation enabled

 target/sh4/cpu.h              | 14 ++++++--
 target/sh4/helper.h           |  4 +--
 target/sh4/op_helper.c        | 51 +++++++++++++++-----------
 target/sh4/translate.c        |  4 +--
 tests/tcg/sh4/test-macl.c     | 67 +++++++++++++++++++++++++++++++++++
 tests/tcg/sh4/test-macw.c     | 61 +++++++++++++++++++++++++++++++
 tests/tcg/sh4/Makefile.target |  8 +++++
 7 files changed, 182 insertions(+), 27 deletions(-)
 create mode 100644 tests/tcg/sh4/test-macl.c
 create mode 100644 tests/tcg/sh4/test-macw.c

-- 
2.34.1

[PATCH v3 1/4] target/sh4: mac.w: memory accesses are 16-bit words

[Richard Henderson]

From: Zack Buhman <zack@buhman.org>

Before this change, executing a code sequence such as:

           mova   tblm,r0
           mov    r0,r1
           mova   tbln,r0
           clrs
           clrmac
           mac.w  @r0+,@r1+
           mac.w  @r0+,@r1+

           .align 4
  tblm:    .word  0x1234
           .word  0x5678
  tbln:    .word  0x9abc
           .word  0xdefg

Does not result in correct behavior:

Expected behavior:
  first macw : macl = 0x1234 * 0x9abc + 0x0
               mach = 0x0

  second macw: macl = 0x5678 * 0xdefg + 0xb00a630
               mach = 0x0

Observed behavior (qemu-sh4eb, prior to this commit):

  first macw : macl = 0x5678 * 0xdefg + 0x0
               mach = 0x0

  second macw: (unaligned longword memory access, SIGBUS)

Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory
access, not a 32-bit longword memory access.

Signed-off-by: Zack Buhman <zack@buhman.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240402093756.27466-1-zack@buhman.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index a9b1bc7524..6643c14dde 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -816,10 +816,10 @@ static void _decode_opc(DisasContext * ctx)
             TCGv arg0, arg1;
             arg0 = tcg_temp_new();
             tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx,
-                                MO_TESL | MO_ALIGN);
+                                MO_TESW | MO_ALIGN);
             arg1 = tcg_temp_new();
             tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx,
-                                MO_TESL | MO_ALIGN);
+                                MO_TESW | MO_ALIGN);
             gen_helper_macw(tcg_env, arg0, arg1);
             tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 2);
             tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2);
-- 
2.34.1

[PATCH v3 2/4] target/sh4: Merge mach and macl into a union

[Richard Henderson]

Allow host access to the entire 64-bit accumulator.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/cpu.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 9211da6bde..d928bcf006 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -155,12 +155,22 @@ typedef struct CPUArchState {
     uint32_t pc;                /* program counter */
     uint32_t delayed_pc;        /* target of delayed branch */
     uint32_t delayed_cond;      /* condition of delayed branch */
-    uint32_t mach;              /* multiply and accumulate high */
-    uint32_t macl;              /* multiply and accumulate low */
     uint32_t pr;                /* procedure register */
     uint32_t fpscr;             /* floating point status/control register */
     uint32_t fpul;              /* floating point communication register */
 
+    /* multiply and accumulate: high, low and combined. */
+    union {
+        uint64_t mac;
+        struct {
+#if HOST_BIG_ENDIAN
+            uint32_t mach, macl;
+#else
+            uint32_t macl, mach;
+#endif
+        };
+    };
+
     /* float point status register */
     float_status fp_status;
 
-- 
2.34.1

[PATCH v3 3/4] target/sh4: Fix mac.l with saturation enabled

[Richard Henderson]

From: Zack Buhman <zack@buhman.org>

The saturation arithmetic logic in helper_macl is not correct.
I tested and verified this behavior on a SH7091.

Signed-off-by: Zack Buhman <zack@buhman.org>
Message-Id: <20240404162641.27528-2-zack@buhman.org>
[rth: Reformat helper_macl, add a test case.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/helper.h           |  2 +-
 target/sh4/op_helper.c        | 23 ++++++------
 tests/tcg/sh4/test-macl.c     | 67 +++++++++++++++++++++++++++++++++++
 tests/tcg/sh4/Makefile.target |  5 +++
 4 files changed, 86 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/sh4/test-macl.c

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index 8d792f6b55..64056e4a39 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -11,7 +11,7 @@ DEF_HELPER_3(movcal, void, env, i32, i32)
 DEF_HELPER_1(discard_movcal_backup, void, env)
 DEF_HELPER_2(ocbi, void, env, i32)
 
-DEF_HELPER_3(macl, void, env, i32, i32)
+DEF_HELPER_3(macl, void, env, s32, s32)
 DEF_HELPER_3(macw, void, env, i32, i32)
 
 DEF_HELPER_2(ld_fpscr, void, env, i32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 4559d0d376..d0bae0cc00 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -158,20 +158,23 @@ void helper_ocbi(CPUSH4State *env, uint32_t address)
     }
 }
 
-void helper_macl(CPUSH4State *env, uint32_t arg0, uint32_t arg1)
+void helper_macl(CPUSH4State *env, int32_t arg0, int32_t arg1)
 {
+    const int64_t min = -(1ll << 47);
+    const int64_t max = (1ll << 47) - 1;
+    int64_t mul = (int64_t)arg0 * arg1;
+    int64_t mac = env->mac;
     int64_t res;
 
-    res = ((uint64_t) env->mach << 32) | env->macl;
-    res += (int64_t) (int32_t) arg0 *(int64_t) (int32_t) arg1;
-    env->mach = (res >> 32) & 0xffffffff;
-    env->macl = res & 0xffffffff;
-    if (env->sr & (1u << SR_S)) {
-        if (res < 0)
-            env->mach |= 0xffff0000;
-        else
-            env->mach &= 0x00007fff;
+    if (!(env->sr & (1u << SR_S))) {
+        res = mac + mul;
+    } else if (sadd64_overflow(mac, mul, &res)) {
+        res = mac < 0 ? min : max;
+    } else {
+        res = MIN(MAX(res, min), max);
     }
+
+    env->mac = res;
 }
 
 void helper_macw(CPUSH4State *env, uint32_t arg0, uint32_t arg1)
diff --git a/tests/tcg/sh4/test-macl.c b/tests/tcg/sh4/test-macl.c
new file mode 100644
index 0000000000..b66c854365
--- /dev/null
+++ b/tests/tcg/sh4/test-macl.c
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#define MACL_S_MIN  (-(1ll << 47))
+#define MACL_S_MAX  ((1ll << 47) - 1)
+
+int64_t mac_l(int64_t mac, const int32_t *a, const int32_t *b)
+{
+    register uint32_t macl __asm__("macl") = mac;
+    register uint32_t mach __asm__("mach") = mac >> 32;
+
+    asm volatile("mac.l @%0+,@%1+"
+                 : "+r"(a), "+r"(b), "+x"(macl), "+x"(mach));
+
+    return ((uint64_t)mach << 32) | macl;
+}
+
+typedef struct {
+    int64_t mac;
+    int32_t a, b;
+    int64_t res[2];
+} Test;
+
+__attribute__((noinline))
+void test(const Test *t, int sat)
+{
+    int64_t res;
+
+    if (sat) {
+        asm volatile("sets");
+    } else {
+        asm volatile("clrs");
+    }
+    res = mac_l(t->mac, &t->a, &t->b);
+
+    if (res != t->res[sat]) {
+        fprintf(stderr, "%#llx + (%#x * %#x) = %#llx -- got %#llx\n",
+                t->mac, t->a, t->b, t->res[sat], res);
+        abort();
+    }
+}
+
+int main()
+{
+    static const Test tests[] = {
+        { 0x00007fff12345678ll, INT32_MAX, INT32_MAX,
+          { 0x40007ffe12345679ll, MACL_S_MAX } },
+        { MACL_S_MIN, -1, 1,
+          { 0xffff7fffffffffffll, MACL_S_MIN } },
+        { INT64_MIN, -1, 1,
+          { INT64_MAX, MACL_S_MIN } },
+        { 0x00007fff00000000ll, INT32_MAX, INT32_MAX,
+          { 0x40007ffe00000001ll, MACL_S_MAX } },
+        { 4, 1, 2, { 6, 6 } },
+        { -4, -1, -2, { -2, -2 } },
+    };
+
+    for (int i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) {
+        for (int j = 0; j < 2; ++j) {
+            test(&tests[i], j);
+        }
+    }
+    return 0;
+}
diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target
index 16eaa850a8..9a11c10924 100644
--- a/tests/tcg/sh4/Makefile.target
+++ b/tests/tcg/sh4/Makefile.target
@@ -9,3 +9,8 @@ run-signals: signals
 	$(call skip-test, $<, "BROKEN")
 run-plugin-signals-with-%:
 	$(call skip-test, $<, "BROKEN")
+
+VPATH += $(SRC_PATH)/tests/tcg/sh4
+
+test-macl: CFLAGS += -O -g
+TESTS += test-macl
-- 
2.34.1

[PATCH v3 4/4] target/sh4: Fix mac.w with saturation enabled

[Richard Henderson]

From: Zack Buhman <zack@buhman.org>

The saturation arithmetic logic in helper_macw is not correct.
I tested and verified this behavior on a SH7091.

Signed-off-by: Zack Buhman <zack@buhman.org>
Message-Id: <20240405233802.29128-3-zack@buhman.org>
[rth: Reformat helper_macw, add a test case.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/helper.h           |  2 +-
 target/sh4/op_helper.c        | 28 +++++++++-------
 tests/tcg/sh4/test-macw.c     | 61 +++++++++++++++++++++++++++++++++++
 tests/tcg/sh4/Makefile.target |  3 ++
 4 files changed, 82 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/sh4/test-macw.c

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index 64056e4a39..29011d3dbb 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -12,7 +12,7 @@ DEF_HELPER_1(discard_movcal_backup, void, env)
 DEF_HELPER_2(ocbi, void, env, i32)
 
 DEF_HELPER_3(macl, void, env, s32, s32)
-DEF_HELPER_3(macw, void, env, i32, i32)
+DEF_HELPER_3(macw, void, env, s32, s32)
 
 DEF_HELPER_2(ld_fpscr, void, env, i32)
 
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index d0bae0cc00..99394b714c 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -177,22 +177,28 @@ void helper_macl(CPUSH4State *env, int32_t arg0, int32_t arg1)
     env->mac = res;
 }
 
-void helper_macw(CPUSH4State *env, uint32_t arg0, uint32_t arg1)
+void helper_macw(CPUSH4State *env, int32_t arg0, int32_t arg1)
 {
-    int64_t res;
+    /* Inputs are already sign-extended from 16 bits. */
+    int32_t mul = arg0 * arg1;
 
-    res = ((uint64_t) env->mach << 32) | env->macl;
-    res += (int64_t) (int16_t) arg0 *(int64_t) (int16_t) arg1;
-    env->mach = (res >> 32) & 0xffffffff;
-    env->macl = res & 0xffffffff;
     if (env->sr & (1u << SR_S)) {
-        if (res < -0x80000000) {
+        /*
+         * In saturation arithmetic mode, the accumulator is 32-bit
+         * with carry. MACH is not considered during the addition
+         * operation nor the 32-bit saturation logic.
+         */
+        int32_t res, macl = env->macl;
+
+        if (sadd32_overflow(macl, mul, &res)) {
+            res = macl < 0 ? INT32_MIN : INT32_MAX;
+            /* If overflow occurs, the MACH register is set to 1. */
             env->mach = 1;
-            env->macl = 0x80000000;
-        } else if (res > 0x000000007fffffff) {
-            env->mach = 1;
-            env->macl = 0x7fffffff;
         }
+        env->macl = res;
+    } else {
+        /* In non-saturation arithmetic mode, the accumulator is 64-bit */
+        env->mac += mul;
     }
 }
 
diff --git a/tests/tcg/sh4/test-macw.c b/tests/tcg/sh4/test-macw.c
new file mode 100644
index 0000000000..4eceec8634
--- /dev/null
+++ b/tests/tcg/sh4/test-macw.c
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+int64_t mac_w(int64_t mac, const int16_t *a, const int16_t *b)
+{
+    register uint32_t macl __asm__("macl") = mac;
+    register uint32_t mach __asm__("mach") = mac >> 32;
+
+    asm volatile("mac.w @%0+,@%1+"
+                 : "+r"(a), "+r"(b), "+x"(macl), "+x"(mach));
+
+    return ((uint64_t)mach << 32) | macl;
+}
+
+typedef struct {
+    int64_t mac;
+    int16_t a, b;
+    int64_t res[2];
+} Test;
+
+__attribute__((noinline))
+void test(const Test *t, int sat)
+{
+    int64_t res;
+
+    if (sat) {
+        asm volatile("sets");
+    } else {
+        asm volatile("clrs");
+    }
+    res = mac_w(t->mac, &t->a, &t->b);
+
+    if (res != t->res[sat]) {
+        fprintf(stderr, "%#llx + (%#x * %#x) = %#llx -- got %#llx\n",
+                t->mac, t->a, t->b, t->res[sat], res);
+        abort();
+    }
+}
+
+int main()
+{
+    static const Test tests[] = {
+        { 0, 2, 3, { 6, 6 } },
+        { 0x123456787ffffffell, 2, -3,
+          { 0x123456787ffffff8ll, 0x123456787ffffff8ll } },
+        { 0xabcdef127ffffffall, 2, 3,
+          { 0xabcdef1280000000ll, 0x000000017fffffffll } },
+        { 0xfffffffffll, INT16_MAX, INT16_MAX,
+          { 0x103fff0000ll, 0xf3fff0000ll } },
+    };
+
+    for (int i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) {
+        for (int j = 0; j < 2; ++j) {
+            test(&tests[i], j);
+        }
+    }
+    return 0;
+}
diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target
index 9a11c10924..4d09291c0c 100644
--- a/tests/tcg/sh4/Makefile.target
+++ b/tests/tcg/sh4/Makefile.target
@@ -14,3 +14,6 @@ VPATH += $(SRC_PATH)/tests/tcg/sh4
 
 test-macl: CFLAGS += -O -g
 TESTS += test-macl
+
+test-macw: CFLAGS += -O -g
+TESTS += test-macw
-- 
2.34.1

Re: [PATCH v3 3/4] target/sh4: Fix mac.l with saturation enabled

[Philippe Mathieu-Daudé]

On 6/4/24 07:37, Richard Henderson wrote:
> From: Zack Buhman <zack@buhman.org>
> 
> The saturation arithmetic logic in helper_macl is not correct.
> I tested and verified this behavior on a SH7091.
> 
> Signed-off-by: Zack Buhman <zack@buhman.org>
> Message-Id: <20240404162641.27528-2-zack@buhman.org>
> [rth: Reformat helper_macl, add a test case.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sh4/helper.h           |  2 +-
>   target/sh4/op_helper.c        | 23 ++++++------
>   tests/tcg/sh4/test-macl.c     | 67 +++++++++++++++++++++++++++++++++++
>   tests/tcg/sh4/Makefile.target |  5 +++
>   4 files changed, 86 insertions(+), 11 deletions(-)
>   create mode 100644 tests/tcg/sh4/test-macl.c

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 2/4] target/sh4: Merge mach and macl into a union

[Philippe Mathieu-Daudé]

On 6/4/24 07:37, Richard Henderson wrote:
> Allow host access to the entire 64-bit accumulator.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sh4/cpu.h | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 4/4] target/sh4: Fix mac.w with saturation enabled

[Philippe Mathieu-Daudé]

On 6/4/24 07:37, Richard Henderson wrote:
> From: Zack Buhman <zack@buhman.org>
> 
> The saturation arithmetic logic in helper_macw is not correct.
> I tested and verified this behavior on a SH7091.
> 
> Signed-off-by: Zack Buhman <zack@buhman.org>
> Message-Id: <20240405233802.29128-3-zack@buhman.org>
> [rth: Reformat helper_macw, add a test case.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sh4/helper.h           |  2 +-
>   target/sh4/op_helper.c        | 28 +++++++++-------
>   tests/tcg/sh4/test-macw.c     | 61 +++++++++++++++++++++++++++++++++++
>   tests/tcg/sh4/Makefile.target |  3 ++
>   4 files changed, 82 insertions(+), 12 deletions(-)
>   create mode 100644 tests/tcg/sh4/test-macw.c

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw]

[Michael Tokarev]

06.04.2024 08:37, Richard Henderson wrote:
> Zack's recent patches, tidied a little bit, and with
> test cases added.

These fixes ended up in stable-8.2, but not in stable-7.2.
This is because in 7.2, the context is a bit different.

Later, a couple other fixes in this area come from Philippe
(Fix ADDV & SUBV opcodes) which are easy to pick up but it
wants changes in tests/tcg/sh4/Makefile.target introduced
in this patchset.

b0f2f2976b "target/sh4: mac.w: memory accesses are 16-bit words"
also needs 03a0d87e8d "target/sh4: Use MO_ALIGN where required",
but this one, while simple, is a big one and doesn't apply to
7.2 directly in many places in target/sh4/translate.c, in parts
due to bebd5cb300 "target/sh4: Drop tcg_temp_free" (but can be
easily tweaked manually).

Or I can hand-apply b0f2f2976b (s/MO_TESL/MO_TESW) without
03a0d87e8d (add MO_ALIGN).

Does picking up this stuff for 7.2 make sense?

(Cc'ing Cole for general stable-7.2 feedback on redhat side).

Thanks,

/mjt

> Richard Henderson (1):
>    target/sh4: Merge mach and macl into a union
> 
> Zack Buhman (3):
>    target/sh4: mac.w: memory accesses are 16-bit words
>    target/sh4: Fix mac.l with saturation enabled
>    target/sh4: Fix mac.w with saturation enabled
> 
>   target/sh4/cpu.h              | 14 ++++++--
>   target/sh4/helper.h           |  4 +--
>   target/sh4/op_helper.c        | 51 +++++++++++++++-----------
>   target/sh4/translate.c        |  4 +--
>   tests/tcg/sh4/test-macl.c     | 67 +++++++++++++++++++++++++++++++++++
>   tests/tcg/sh4/test-macw.c     | 61 +++++++++++++++++++++++++++++++
>   tests/tcg/sh4/Makefile.target |  8 +++++
>   7 files changed, 182 insertions(+), 27 deletions(-)
>   create mode 100644 tests/tcg/sh4/test-macl.c
>   create mode 100644 tests/tcg/sh4/test-macw.c
> 

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt

Re: [PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw]

[Yoshinori Sato]

On Sat, 04 May 2024 17:25:52 +0900,
Michael Tokarev wrote:
> 
> 06.04.2024 08:37, Richard Henderson wrote:
> > Zack's recent patches, tidied a little bit, and with
> > test cases added.
> 
> These fixes ended up in stable-8.2, but not in stable-7.2.
> This is because in 7.2, the context is a bit different.
> 
> Later, a couple other fixes in this area come from Philippe
> (Fix ADDV & SUBV opcodes) which are easy to pick up but it
> wants changes in tests/tcg/sh4/Makefile.target introduced
> in this patchset.
> 
> b0f2f2976b "target/sh4: mac.w: memory accesses are 16-bit words"
> also needs 03a0d87e8d "target/sh4: Use MO_ALIGN where required",
> but this one, while simple, is a big one and doesn't apply to
> 7.2 directly in many places in target/sh4/translate.c, in parts
> due to bebd5cb300 "target/sh4: Drop tcg_temp_free" (but can be
> easily tweaked manually).
> 
> Or I can hand-apply b0f2f2976b (s/MO_TESL/MO_TESW) without
> 03a0d87e8d (add MO_ALIGN).
> 
> Does picking up this stuff for 7.2 make sense?
> 
> (Cc'ing Cole for general stable-7.2 feedback on redhat side).
> 
> Thanks,
> 
> /mjt
> 
> > Richard Henderson (1):
> >    target/sh4: Merge mach and macl into a union
> > 
> > Zack Buhman (3):
> >    target/sh4: mac.w: memory accesses are 16-bit words
> >    target/sh4: Fix mac.l with saturation enabled
> >    target/sh4: Fix mac.w with saturation enabled
> > 
> >   target/sh4/cpu.h              | 14 ++++++--
> >   target/sh4/helper.h           |  4 +--
> >   target/sh4/op_helper.c        | 51 +++++++++++++++-----------
> >   target/sh4/translate.c        |  4 +--
> >   tests/tcg/sh4/test-macl.c     | 67 +++++++++++++++++++++++++++++++++++
> >   tests/tcg/sh4/test-macw.c     | 61 +++++++++++++++++++++++++++++++
> >   tests/tcg/sh4/Makefile.target |  8 +++++
> >   7 files changed, 182 insertions(+), 27 deletions(-)
> >   create mode 100644 tests/tcg/sh4/test-macl.c
> >   create mode 100644 tests/tcg/sh4/test-macw.c
> > 
> 
> -- 
> GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
> New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
> Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
> Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
> 

Does this mean you changed it like this?
I think this is fine.

index 7db3468b01..f3bf0fc50a 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -844,9 +844,9 @@ static void _decode_opc(DisasContext * ctx)
 	{
 	    TCGv arg0, arg1;
 	    arg0 = tcg_temp_new();
-            tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx, MO_TESL);
+            tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx, MO_TESW);
 	    arg1 = tcg_temp_new();
-            tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx, MO_TESL);
+            tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx, MO_TESW);
             gen_helper_macw(cpu_env, arg0, arg1);
 	    tcg_temp_free(arg1);
 	    tcg_temp_free(arg0);

-- 
Yosinori Sato

Re: [PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw]

[Michael Tokarev]

06.05.2024 15:38, Yoshinori Sato wrote:
[...]
> Does this mean you changed it like this?
> I think this is fine.

Yes, the main part is exactly like this, and there's no questions here.
My question was more about was the testsuite changes which comes with
the same patch, and parts of the Makefile there, which requires other
patches too, at least to apply more or less cleanly.

Thanks,

/mjt

> index 7db3468b01..f3bf0fc50a 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -844,9 +844,9 @@ static void _decode_opc(DisasContext * ctx)
>   	{
>   	    TCGv arg0, arg1;
>   	    arg0 = tcg_temp_new();
> -            tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx, MO_TESL);
> +            tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx, MO_TESW);
>   	    arg1 = tcg_temp_new();
> -            tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx, MO_TESL);
> +            tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx, MO_TESW);
>               gen_helper_macw(cpu_env, arg0, arg1);
>   	    tcg_temp_free(arg1);
>   	    tcg_temp_free(arg0);
> 

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt