Re: Fix for MUL instruction on the v850

[Hans-Peter Nilsson <hp@bitrange.com>]

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

(Wrong list; I CC:ed the right one.)

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?
>
> Thanks,
> Jeff

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.

brgds, H-P

[-- Attachment #2: Type: text/plain, Size: 525 bytes --]

	* sim/v850/simops.c (Multiply64): Properly test if we need to negate
	either of the operand.

diff --git a/sim/v850/simops.c b/sim/v850/simops.c
index 8fac8bd9891..94399e2ad4f 100644
--- a/sim/v850/simops.c
+++ b/sim/v850/simops.c
@@ -339,10 +339,10 @@ Multiply64 (int sign, unsigned long op0)
 	  
       sign = (op0 ^ op1) & 0x80000000;
 	  
-      if (((signed long) op0) < 0)
+      if (op0 & 0x80000000)
 	op0 = - op0;
 	  
-      if (((signed long) op1) < 0)
+      if (op1 & 0x80000000)
 	op1 = - op1;
     }