From: Richard Henderson <richard.henderson@linaro.org>
To: qemu-devel@nongnu.org
Cc: zack@buhman.org, peter.maydell@linaro.org, ysato@users.sourceforge.jp
Subject: [PATCH v3 3/4] target/sh4: Fix mac.l with saturation enabled
Date: Fri, 5 Apr 2024 19:37:31 -1000 [thread overview]
Message-ID: <20240406053732.191398-4-richard.henderson@linaro.org> (raw)
In-Reply-To: <20240406053732.191398-1-richard.henderson@linaro.org>
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
next prev parent reply other threads:[~2024-04-06 5:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-06 5:37 [PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw] Richard Henderson
2024-04-06 5:37 ` [PATCH v3 1/4] target/sh4: mac.w: memory accesses are 16-bit words Richard Henderson
2024-04-06 5:37 ` [PATCH v3 2/4] target/sh4: Merge mach and macl into a union Richard Henderson
2024-04-08 6:07 ` Philippe Mathieu-Daudé
2024-04-06 5:37 ` Richard Henderson [this message]
2024-04-08 6:06 ` [PATCH v3 3/4] target/sh4: Fix mac.l with saturation enabled Philippe Mathieu-Daudé
2024-04-06 5:37 ` [PATCH v3 4/4] target/sh4: Fix mac.w " Richard Henderson
2024-04-08 6:08 ` Philippe Mathieu-Daudé
2024-05-04 8:25 ` [PATCH for-9.0 v3 0/4] target/sh4: Fix mac.[lw] Michael Tokarev
2024-05-06 12:38 ` Yoshinori Sato
2024-05-06 12:46 ` Michael Tokarev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240406053732.191398-4-richard.henderson@linaro.org \
--to=richard.henderson@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=ysato@users.sourceforge.jp \
--cc=zack@buhman.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).