Fix "bins" simulation for v850e3v5

I've been carrying this for a few years.   One test in the GCC testsuite is
failing due to a bug in the handling of the v850e3v5 instruction "bins".

When the "bins" instruction specifies a 32bit bitfield size, the simulator
exhibits undefined behavior by trying to shift a 32 bit quantity by 32 bits.
In the case of a 32 bit shift, we know what the resultant mask should be.  So
we can just set it.

That seemed better than using 1UL for the constant (on a 32bit host unsigned
long might still just be 32 bits) or needlessly forcing everything to
long long types.

Thankfully the case where this shows up is only bins <src>, 0, 32, <dest>
which would normally be encoded as a simple move.

	* testsuite/v850/allinsns.exp: Add v850e3v5.
	* testsuite/v850/bins.cgs: New test.
	* v850/simops.c (v850_bins): Avoid undefined behavior on left shift.
This commit is contained in:
Jeff Law 2022-04-06 11:06:53 -04:00
parent 7fb56b9893
commit 49fffa58f7
3 changed files with 21 additions and 2 deletions

View file

@ -5,7 +5,7 @@ sim_init
# All machines.
# Should add more cpus if the testsuite adds coverage for their insns, but
# at the core level, there's no deviation beyond these two.
set all_machs "v850e v850"
set all_machs "v850e3v5 v850e v850"
# gas doesn't support any '=' option for v850.
set cpu_option_sep ""

View file

@ -0,0 +1,12 @@
# v850 bins
# mach: v850e3v5
# as: -mv850e3v5
.include "testutils.inc"
seti 0x7fff, r10
seti 0x0, r11
bins r10, 0, 32, r11
reg r11, 0x7fff
pass

View file

@ -3267,7 +3267,14 @@ v850_bins (SIM_DESC sd, unsigned int source, unsigned int lsb, unsigned int msb,
pos = lsb;
width = (msb - lsb) + 1;
mask = ~ (-(1 << width));
/* A width of 32 exhibits undefined behavior on the shift. The easiest
way to make this code safe is to just avoid that case and set the mask
to the right value. */
if (width >= 32)
mask = 0xffffffff;
else
mask = ~ (-(1 << width));
source &= mask;
mask <<= pos;
result = (* dest) & ~ mask;