[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 13/35] target/sh4: Fix mac.w with saturation enabled
From: |
Richard Henderson |
Subject: |
[PULL 13/35] target/sh4: Fix mac.w with saturation enabled |
Date: |
Mon, 8 Apr 2024 07:49:07 -1000 |
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.
Reviewd-by: Yoshinori Sato <ysato@users.sourceforge.jp>
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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
- [PULL 03/35] linux-user: do_setsockopt: fix SOL_ALG.ALG_SET_KEY, (continued)
- [PULL 03/35] linux-user: do_setsockopt: fix SOL_ALG.ALG_SET_KEY, Richard Henderson, 2024/04/08
- [PULL 06/35] linux-user: do_setsockopt: eliminate goto in switch for SO_SNDTIMEO, Richard Henderson, 2024/04/08
- [PULL 08/35] linux-user: replace calloc() with g_new0(), Richard Henderson, 2024/04/08
- [PULL 07/35] linux-user: Add FITRIM ioctl, Richard Henderson, 2024/04/08
- [PULL 09/35] target/hppa: Fix IIAOQ, IIASQ for pa2.0, Richard Henderson, 2024/04/08
- [PULL 10/35] target/sh4: mac.w: memory accesses are 16-bit words, Richard Henderson, 2024/04/08
- [PULL 11/35] target/sh4: Merge mach and macl into a union, Richard Henderson, 2024/04/08
- [PULL 05/35] linux-user: do_setsockopt: make ip_mreq_source local to the place where it is used, Richard Henderson, 2024/04/08
- [PULL 12/35] target/sh4: Fix mac.l with saturation enabled, Richard Henderson, 2024/04/08
- [PULL 14/35] target/sh4: add missing CHECK_NOT_DELAY_SLOT, Richard Henderson, 2024/04/08
- [PULL 13/35] target/sh4: Fix mac.w with saturation enabled,
Richard Henderson <=
- [PULL 16/35] target/m68k: Pass semihosting arg to exit, Richard Henderson, 2024/04/08
- [PULL 15/35] target/m68k: Map FPU exceptions to FPSR register, Richard Henderson, 2024/04/08
- [PULL 17/35] target/m68k: Perform the semihosting test during translate, Richard Henderson, 2024/04/08
- [PULL 18/35] target/m68k: Support semihosting on non-ColdFire targets, Richard Henderson, 2024/04/08
- [PULL 19/35] tcg: Add TCGContext.emit_before_op, Richard Henderson, 2024/04/08
- [PULL 20/35] accel/tcg: Add insn_start to DisasContextBase, Richard Henderson, 2024/04/08
- [PULL 21/35] target/arm: Use insn_start from DisasContextBase, Richard Henderson, 2024/04/08
- [PULL 23/35] target/i386: Preserve DisasContextBase.insn_start across rewind, Richard Henderson, 2024/04/08
- [PULL 24/35] target/microblaze: Use insn_start from DisasContextBase, Richard Henderson, 2024/04/08
- [PULL 25/35] target/riscv: Use insn_start from DisasContextBase, Richard Henderson, 2024/04/08