----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/823/#review1514 -----------------------------------------------------------
This looks good to me. I don't know the LSQ all that well or how your change affects it other than what you have in your description, but I'm willing to assume you got that right. Thanks for moving the fault to sim. I notice you have another fault in ARM which flushes the pipe and also seems fairly generic (modulo the writes == flush assumption). It would be nice if in a future change you moved that to sim too, but leaving it where it is also makes some sense for various reasons. - Gabe On 2011-09-09 14:48:04, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/823/ > ----------------------------------------------------------- > > (Updated 2011-09-09 14:48:04) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > LSQ: Only trigger a memory violation with a load/load if the value changes. > > Only create a memory ordering violation when the value could have changed > between two subsequent loads, instead of just when loads go out-of-order > to the same address. While not very common in the case of Alpha, with > an architecture with a hardware table walker this can happen reasonably > frequently beacuse a translation will miss and start a table walk and > before the CPU re-schedules the faulting instruction another one will > pass it to the same address (or cache block depending on the dependency > checking). > > The performance improvement on SPEC benchmarks can be substantial (2-10%). > > > Diffs > ----- > > src/arch/arm/faults.hh 1f95c9a0bb2f > src/arch/arm/faults.cc 1f95c9a0bb2f > src/cpu/base_dyn_inst.hh 1f95c9a0bb2f > src/cpu/base_dyn_inst_impl.hh 1f95c9a0bb2f > src/cpu/o3/lsq_impl.hh 1f95c9a0bb2f > src/cpu/o3/lsq_unit.hh 1f95c9a0bb2f > src/cpu/o3/lsq_unit_impl.hh 1f95c9a0bb2f > src/sim/faults.hh 1f95c9a0bb2f > src/sim/faults.cc 1f95c9a0bb2f > > Diff: http://reviews.m5sim.org/r/823/diff > > > Testing > ------- > > This patch has been tested with a couple of self-checking hand crafted > programs I wrote to stress ordering between two cores. With injected bugs it > definitely shows a failure and it doesn't with this implementation (and it > caught one issue during development). > > > Thanks, > > Ali > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
