From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2119) id 401D93857415; Wed, 6 Apr 2022 15:08:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 401D93857415 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Jeff Law To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Fix "bins" simulation for v850e3v5 X-Act-Checkin: binutils-gdb X-Git-Author: Jeff Law X-Git-Refname: refs/heads/master X-Git-Oldrev: 7fb56b98937a2feef5a3e12d8b00506ff4d132be X-Git-Newrev: 49fffa58f7e6da777d10fe77663bc7c8f531fe7f Message-Id: <20220406150827.401D93857415@sourceware.org> Date: Wed, 6 Apr 2022 15:08:27 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Apr 2022 15:08:27 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D49fffa58f7e6= da777d10fe77663bc7c8f531fe7f commit 49fffa58f7e6da777d10fe77663bc7c8f531fe7f Author: Jeff Law Date: Wed Apr 6 11:06:53 2022 -0400 Fix "bins" simulation for v850e3v5 =20 I've been carrying this for a few years. One test in the GCC testsuit= e is failing due to a bug in the handling of the v850e3v5 instruction "bins". =20 When the "bins" instruction specifies a 32bit bitfield size, the simula= tor 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 b= e. So we can just set it. =20 That seemed better than using 1UL for the constant (on a 32bit host uns= igned long might still just be 32 bits) or needlessly forcing everything to long long types. =20 Thankfully the case where this shows up is only bins , 0, 32, which would normally be encoded as a simple move. =20 * testsuite/v850/allinsns.exp: Add v850e3v5. * testsuite/v850/bins.cgs: New test. * v850/simops.c (v850_bins): Avoid undefined behavior on left s= hift. Diff: --- sim/testsuite/v850/allinsns.exp | 2 +- sim/testsuite/v850/bins.cgs | 12 ++++++++++++ sim/v850/simops.c | 9 ++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/sim/testsuite/v850/allinsns.exp b/sim/testsuite/v850/allinsns.= exp index 4cc461c30fd..ee22a5d93b7 100644 --- a/sim/testsuite/v850/allinsns.exp +++ b/sim/testsuite/v850/allinsns.exp @@ -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" =20 # gas doesn't support any '=3D' option for v850. set cpu_option_sep "" diff --git a/sim/testsuite/v850/bins.cgs b/sim/testsuite/v850/bins.cgs new file mode 100644 index 00000000000..dedc5ceaafd --- /dev/null +++ b/sim/testsuite/v850/bins.cgs @@ -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 diff --git a/sim/v850/simops.c b/sim/v850/simops.c index d2640577fc8..e9a5d489d88 100644 --- a/sim/v850/simops.c +++ b/sim/v850/simops.c @@ -3267,7 +3267,14 @@ v850_bins (SIM_DESC sd, unsigned int source, unsigne= d int lsb, unsigned int msb, pos =3D lsb; width =3D (msb - lsb) + 1; =20 - mask =3D ~ (-(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 >=3D 32) + mask =3D 0xffffffff; + else + mask =3D ~ (-(1 << width)); + source &=3D mask; mask <<=3D pos; result =3D (* dest) & ~ mask;