qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Zack Buhman <zack@buhman.org>
To: qemu-devel@nongnu.org
Cc: Zack Buhman <zack@buhman.org>
Subject: [PATCH] sh4: mac.l: implement saturation arithmetic logic
Date: Thu,  4 Apr 2024 23:10:35 +0800	[thread overview]
Message-ID: <20240404151100.24063-1-zack@buhman.org> (raw)

The saturation arithmetic logic in helper_macl is not correct.

I tested and verified this behavior on a SH7091, the general pattern
is a code sequence such as:

	sets

	mov.l _mach,r2
	lds r2,mach
	mov.l _macl,r2
	lds r2,macl

	mova _n,r0
	mov r0,r1
	mova _m,r0
	mac.l @r0+,@r1+

    _mach: .long 0x00007fff
    _macl: .long 0x12345678
    _m:    .long 0x7fffffff
    _n:    .long 0x7fffffff

Test case 0: (no int64_t overflow)
  given; prior to saturation mac.l:
    mach = 0x00007fff macl = 0x12345678
    @r0  = 0x7fffffff @r1  = 0x7fffffff

  expected saturation mac.l result:
    mach = 0x00007fff macl = 0xffffffff

  qemu saturation mac.l result (prior to this commit):
    mach = 0x00007ffe macl = 0x12345678

Test case 1: (no int64_t overflow)
  given; prior to saturation mac.l:
    mach = 0xffff8000 macl = 0x00000000
    @r0  = 0xffffffff @r1  = 0x00000001

  expected saturation mac.l result:
    mach = 0xffff8000 macl = 0x00000000

  qemu saturation mac.l result (prior to this commit):
    mach = 0xffff7fff macl = 0xffffffff

Test case 2: (int64_t addition overflow)
  given; prior to saturation mac.l:
    mach = 0x80000000 macl = 0x00000000
    @r0  = 0xffffffff @r1  = 0x00000001

  expected saturation mac.l result:
    mach = 0xffff8000 macl = 0x00000000

  qemu saturation mac.l result (prior to this commit):
    mach = 0xffff7fff macl = 0xffffffff

Test case 3: (int64_t addition overflow)
  given; prior to saturation mac.l:
    mach = 0x7fffffff macl = 0x00000000
    @r0 = 0x7fffffff @r1 = 0x7fffffff

  expected saturation mac.l result:
    mach = 0x00007fff macl = 0xffffffff

  qemu saturation mac.l result (prior to this commit):
    mach = 0xfffffffe macl = 0x00000001

All of the above also matches the description of MAC.L as documented
in cd00147165-sh-4-32-bit-cpu-core-architecture-stmicroelectronics.pdf
---
 target/sh4/op_helper.c | 45 ++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 4559d0d376..a3eb2f5281 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -160,18 +160,43 @@ void helper_ocbi(CPUSH4State *env, uint32_t address)
 
 void helper_macl(CPUSH4State *env, uint32_t arg0, uint32_t arg1)
 {
-    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;
+    int32_t value0 = (int32_t)arg0;
+    int32_t value1 = (int32_t)arg1;
+    int64_t mul = ((int64_t)value0) * ((int64_t)value1);
+    int64_t mac = (((uint64_t)env->mach) << 32) | env->macl;
+    int64_t result = mac + mul;
+    /* Perform 48-bit saturation arithmetic if the S flag is set */
     if (env->sr & (1u << SR_S)) {
-        if (res < 0)
-            env->mach |= 0xffff0000;
-        else
-            env->mach &= 0x00007fff;
+        /*
+         * The following xor/and expression is necessary to detect an
+         * overflow in MSB of res; this is logic necessary because the
+         * sign bit of `mac + mul` may overflow. The MAC unit on real
+         * SH-4 hardware has carry/saturation logic that is equivalent
+         * to the following:
+         */
+        const int64_t upper_bound =  ((1ull << 47) - 1);
+        const int64_t lower_bound = -((1ull << 47) - 0);
+
+        if (((((result ^ mac) & (result ^ mul)) >> 63) & 1) == 1) {
+            /* An overflow occured during 64-bit addition */
+            if (((mac >> 63) & 1) == 0) {
+                result = upper_bound;
+            } else {
+                result = lower_bound;
+            }
+        } else {
+            /* An overflow did not occur during 64-bit addition */
+            if (result > upper_bound) {
+                result = upper_bound;
+            } else if (result < lower_bound) {
+                result = lower_bound;
+            } else {
+                /* leave result unchanged */
+            }
+        }
     }
+    env->macl = result;
+    env->mach = result >> 32;
 }
 
 void helper_macw(CPUSH4State *env, uint32_t arg0, uint32_t arg1)
-- 
2.41.0



             reply	other threads:[~2024-04-04 15:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-04 15:10 Zack Buhman [this message]
2024-04-04 15:37 ` [PATCH] sh4: mac.l: implement saturation arithmetic logic Peter Maydell
2024-04-04 16:26   ` [PATCH v2] " Zack Buhman
2024-04-04 16:39     ` Peter Maydell
2024-04-04 17:26       ` Philippe Mathieu-Daudé
2024-04-04 22:55         ` Zack Buhman
2024-04-05 23:02     ` Richard Henderson

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=20240404151100.24063-1-zack@buhman.org \
    --to=zack@buhman.org \
    --cc=qemu-devel@nongnu.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 NNTP newsgroup(s).