Re: Fix for MUL instruction on the v850

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

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

On 28 Mar 2022 09:38, Hans-Peter Nilsson wrote:
> On Sat, 26 Mar 2022, Jeff Law via Binutils wrote:
> > The v850 gcc testsuite started failing test vrp13 about a week ago. This was
> > ultimately tracked down on a bug in the simulator's handling of the MUL
> > instruction (a change in the compiler resulted in the test no longer
> > compile-time optimized away).
> >
> > MUL on the v850 is a 32x32->64 multiply with the hi/low parts of the result
> > going into distinct registers.? ?? mul r1, r2, r3 is r1xr2 -> (r3, r2) where
> > r2 holds the low 32 bits of the result and r3 holds the high 32 bits of the
> > result.
> >
> > We had inputs of 2, -10 and produced an output of 20.? Opps ;-)
> >
> > The v850 is a 32bit processor using 2s complement.? So to check if a value is
> > negative we merely need to check if bit 0x80000000 is on. Anything else is
> > just introducing dependencies on the host system's types, argument promotions,
> > etc, which is precisely what happened in this case.
> >
> > No new testcase for the simulator.? Not only does vrp13 test this, but about
> > 1800 other tests which have flipped from failing to passing in the GCC
> > testsuite...? So I can say it's being tested and if it were to regress again,
> > we'd catch it.
> >
> > OK for the trunk?
> 
> OK if you mention the gcc test-suite in the commit message or a
> comment, but please (re)consider adding a self-contained (i.e.
> just binutils+sim) test-case.  Suggested inspiration: "git grep
> sim" in the CRIS sim test-suite, though without a "dump
> register" framework macro you need to compare with expected
> results instead of matching output.

that was going to be my feedback as well.  v850 already has a testsuite
for the sim, so adding another testcase shouldn't be that hard.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]