Re: [SH] Fix mac.w insn simulation for SH2 and above
Oleg Endo <[email protected]>
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2026-08-24 at 13:09 +0100, Andrew Burgess wrote: > Oleg Endo <[email protected]> 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 <[email protected]> > > 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
0001-simsh-fix-macw-insn-for-SH2_1.patch
(text/x-patch, 3.3 KB)
From af1b5bf366406270696dddd399f32ae7e48f15a6 Mon Sep 17 00:00:00 2001 From: Oleg Endo <[email protected]> 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