Ping^1 Re: [SH] Fix mac.w insn simulation for SH2 and above

[Oleg Endo <olegendo1024@gmail.com>]

Ping

On Tue, 2026-08-25 at 09:55 +0900, Oleg Endo wrote:
> On Mon, 2026-08-24 at 13:09 +0100, Andrew Burgess wrote:
> > 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.
> > 
> 
> Added test case and adjusted the other hunks.
> Updated patch attached.
> 
> One of my previous sh-sim changes caused a fallout.  I've posted a separate
> patch for that, awaiting feedback.
> https://marc.info/?l=gdb-patches&m=178567807485933&w=2
> 
> Best regards,
> Oleg Endo
>