Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Oleg Endo <olegendo1024@gmail.com>, gdb-patches@sourceware.org
Subject: Re: [SH] Fix mac.w insn simulation for SH2 and above
Date: Mon, 24 Aug 2026 13:09:47 +0100	[thread overview]
Message-ID: <871pbnvhx0.fsf@redhat.com> (raw)
In-Reply-To: <ea983e6d997f4d793fa7486b363a66b2345a1bb5.camel@gmail.com>

Oleg Endo <olegendo1024@gmail.com> writes:

> Hi,
>
> The simulator currently implements only the SH1 version of the mac.w
> instruction.  On SH1 the MACH:MACL accumulator register is only 42-bit.  It
> was extended to 64-bit on SH2 and every variant that followed after.
>
> The issue probably never showed up because GCC hasn't been able to emit the
> SH integer mac instructions.  I ran into this because I was trying to do
> exactly that and while running some tests on sh-sim.
>
> The attached patch uses the bfd_mach field to distinguish between SH1 and
> not-SH1 in the simulation of the mac.w instruction, in a similar way as it's
> already been done for some SH2A instructions.
>
>
> OK to commit & push?
>
> Best regards,
> Oleg Endo
> From 6b07e3daa73acac1d1800c94cdfae0b83509f2d1 Mon Sep 17 00:00:00 2001
> From: Oleg Endo <olegendo@gcc.gnu.org>
> Date: Sun, 2 Aug 2026 19:37:21 +0900
> Subject: [PATCH] sim/sh: fix mac.w insn for SH2+
>
> On SH1 the MACH:MACL accumulator is only 42 bits wide.  On SH2 and above it's 64
> bits wide.  Emulate the corresponding behavior based on the bfd_mach field.
> ---
>  sim/sh/interp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/sim/sh/interp.c b/sim/sh/interp.c
> index c8c2a74..cc4ae2c 100644
> --- a/sim/sh/interp.c
> +++ b/sim/sh/interp.c
> @@ -1197,8 +1197,11 @@ macw (int *regs, unsigned char *memory, int n, int m, int endianw)
>        long mach;
>        /* Add to MACH the sign extended product, and carry from low sum.  */
>        mach = MACH + (-(prod < 0)) + ((unsigned long) sum < prod);
> -      /* Sign extend at 10:th bit in MACH.  */
> -      MACH = (mach & 0x1ff) | -(mach & 0x200);
> +      /* SH1:  MACH:MACL is 42 bits wide, sign extend at the 10:th bit in MACH.

Could you fix '10:th' to '10th' please.

> +         SH2+: MACH:MACL is 64 bits wide.  */

This line should be indented with TAB.

> +      if (saved_state.asregs.bfd_mach == bfd_mach_sh)
> +	mach = (mach & 0x1ff) | -(mach & 0x200);
> +      MACH = mach;
>      }

Is it possible to write a test for this change?  I know simulator
testing is very patchy, but if we add tests for things that are fixed
we'll slowly start to build up some decent test coverage.

Thanks,
Andrew


>    MACL = sum;
>  }
> --
> libgit2 1.9.0


  parent reply	other threads:[~2026-08-24 12:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02 13:53 Oleg Endo
2026-08-24 11:03 ` Ping^1 " Oleg Endo
2026-08-24 12:09 ` Andrew Burgess [this message]
2026-08-25  0:55   ` Oleg Endo

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=871pbnvhx0.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=olegendo1024@gmail.com \
    /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