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

[Oleg Endo <olegendo1024@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]


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


[-- Attachment #2: 0001-simsh-fix-macw-insn-for-SH2_1.patch --]
[-- Type: text/x-patch, Size: 3407 bytes --]

From af1b5bf366406270696dddd399f32ae7e48f15a6 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 +++++--
 sim/testsuite/sh/macw-sh1.s | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 sim/testsuite/sh/macw.s     | 21 +++++++++++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 sim/testsuite/sh/macw-sh1.s

diff --git a/sim/sh/interp.c b/sim/sh/interp.c
index c8c2a74..33f825b 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 10th bit in MACH.
+	 SH2+: MACH:MACL is 64 bits wide.  */
+      if (saved_state.asregs.bfd_mach == bfd_mach_sh)
+	mach = (mach & 0x1ff) | -(mach & 0x200);
+      MACH = mach;
     }
   MACL = sum;
 }
diff --git a/sim/testsuite/sh/macw-sh1.s b/sim/testsuite/sh/macw-sh1.s
new file mode 100644
index 0000000..bb073d5
--- /dev/null
+++ b/sim/testsuite/sh/macw-sh1.s
@@ -0,0 +1,46 @@
+# sh testcase SH1 mac.w
+# mach: sh
+# as(sh):	--isa=sh -defsym sim_cpu=1
+
+# --isa=sh marks the resulting object as bfd_mach_sh, i.e. SH1.
+# When executing an SH1 object the simulator should implement the SH1 behavior
+# of the narrow 42-bit accumulator.
+# sim_cpu=1 prevents testutils.inc from emitting the floating point helpers,
+# which SH1 does not have.
+
+	.include "testutils.inc"
+
+	start
+	set_grs_a5a5
+
+	# On SH1 MACH:MACL is only 42 bits wide.  MACH keeps its low 10 bits
+	# and bit 9 is the sign bit of the accumulator.
+
+	# Bits above bit 9 are dropped.
+	set_sreg	0, macl
+	set_sreg	0x456, mach
+	mova		coeff0,r0
+	mac.w		@r0+, @r0+
+	assert_sreg	0x56, mach
+	assert_sreg	6, macl
+
+	# Bit 9 is sign extended into the upper bits.
+	set_sreg	0, macl
+	set_sreg	0x200, mach
+	mova		coeff0,r0
+	mac.w		@r0+, @r0+
+	assert_sreg	0xfffffe00, mach
+	assert_sreg	6, macl
+
+	set_greg	0xa5a5a5a5, r0
+	set_greg	0xa5a5a5a5, r1
+
+	test_grs_a5a5
+
+	pass
+	exit 0
+
+	.align 2
+coeff0:
+	.word	2
+	.word	3
diff --git a/sim/testsuite/sh/macw.s b/sim/testsuite/sh/macw.s
index 7e3ebc0..b2424d8 100644
--- a/sim/testsuite/sh/macw.s
+++ b/sim/testsuite/sh/macw.s
@@ -33,6 +33,24 @@ doubleinc:
 	mac.w	@r0+, @r0+
 	assertreg0 four00+4
 
+acc64:
+	# On SH2+ MACH:MACL is 64 bits wide.  Check that MACH bits > 9
+	# are kept.  For SH1 case see macw-sh1.s.
+	set_sreg	0, macl
+	set_sreg	0x456, mach
+	mova		coeff0,r0
+	mac.w		@r0+, @r0+
+	assert_sreg	0x456, mach
+	assert_sreg	6, macl
+
+	# Bit 9 of MACH is an ordinary data bit here, not a sign bit.
+	set_sreg	0, macl
+	set_sreg	0x200, mach
+	mova		coeff0,r0
+	mac.w		@r0+, @r0+
+	assert_sreg	0x200, mach
+	assert_sreg	6, macl
+
 	set_greg	0xa5a5a5a5, r0
 	set_greg	0xa5a5a5a5, r1
 
@@ -48,6 +66,9 @@ four00:
 four12:
 	.word	17
 	.word	3
+coeff0:
+	.word	2
+	.word	3
 
 
 pfour00:
--
libgit2 1.9.0