Probably we all agree that x86 could use some automated error checking, but I’m not sure of any way to do this without a ton of work. CheckerCPU seemed good, but as Gabe points out, this may not be comparing against the host hardware.
Jason, do you know any way that’s partially or mostly implemented that we could reuse? Joe From: Jason Lowe-Power [mailto:[email protected]] Sent: Monday, May 15, 2017 2:54 PM To: Gabe Black <[email protected]>; gem5 Developer List <[email protected]> Cc: Gutierrez, Anthony <[email protected]>; Gross, Joe <[email protected]> Subject: Re: [gem5-dev] Review Request 3800: x86: fix Mul1u instruction Gabe: Do you want me to wait to push this patch? We can have Tony re-post to gerrit if you want to review it. That would be way easier for me anyway. We need a way to test for these kinds of errors. It's quite upsetting that we've had this error in multiply for so long. Jason On Mon, May 15, 2017 at 2:50 PM Gabe Black <[email protected]<mailto:[email protected]>> wrote: Please give me a chance to look at it. I think the checker CPU is for comparing, say, the O3 CPU against what's effectively the simple CPU. If an instruction doesn't do the right thing, it will do the wrong thing no matter which CPU is executing it. You can compare against actual execution on a real CPU using the statetrace tool, although I saw that somebody rearranged all the process startup code which I'd originally carefully matched against actual Linux process startup, so that tool probably doesn't give any useful information any more. Gabe On Mon, May 15, 2017 at 12:45 PM, Joe Gross <[email protected]<mailto:[email protected]>> wrote: > On Feb. 4, 2017, 10:21 a.m., Jason Lowe-Power wrote: > > Lol, that code is hard to understand. But, LGTM. > > > > What are you using to test this? Any chance you can commit the test so we > > don't accidentally break this again in the future? > > Tony Gutierrez wrote: > This bug was manifesting in the ROCr runtime while it is loading > libraries, it manifested as a segfualt (unmapped addr panic) because an > address calculation was corrupted due to this instruction. I manually tested > this instruction by comparing its output to the result of 64b multiplication > using __uint128_t. This is a perfect example of where instruction tests would > be useful. It only seemed to give bad output with certain inputs, which gave > the error the appearance of being non-deterministic. > > Jason Lowe-Power wrote: > Resurecting this from the grave... does anyone have any objections to me > just pushing this? I spent all day tracking down a bug, which was fixed by > this patch. This is an important patch for x86, I say push it. Also it would be nice if we were eventually able to use the CheckerCpu for x86 (I believe it's ARM-only right now), as there might be more hard-to-find bugs like this one. - Joe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3800/#review9400 ----------------------------------------------------------- On Feb. 3, 2017, 6:13 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3800/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2017, 6:13 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11778:c76b78110490 > --------------------------- > x86: fix Mul1u instruction > > the Mul1uFlags and Mul1u instructions perform > the 64b multiplication using only 64b registers, however the > method used causes the high 64b to be corrupted for certain > inputs. here we fix the computation. > > > Diffs > ----- > > src/arch/x86/isa/microops/regop.isa ed89cb178ecd7586296d2a2e83595174474db554 > > Diff: http://reviews.gem5.org/r/3800/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected]<mailto:[email protected]> http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
