> 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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
