Re: Fix for MUL instruction on the v850

[Jeff Law via Gdb-patches <gdb-patches@sourceware.org>]

On 3/28/2022 9:12 PM, Mike Frysinger wrote:
> 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.
Fair enough.  I'll cover it with a sim test.

jeff