> On 2011-09-01 11:54:03, Ali Saidi wrote: > > src/arch/alpha/faults.hh, line 44 > > <http://reviews.m5sim.org/r/823/diff/1/?file=14607#file14607line44> > > > > I initially tried the fault in sim approach, however that doesn't work > > because the pcstate object is resolved by the namespace and you can't do > > any template magic to make that happen. There needs to be seperate classes > > each defined in a namespace FooISA{ ... } or at least using namespace > > FooISA. While it could be done by doing using namespace TheISA that gets us > > further away from being able to support a single binary with multiple ISAs, > > so I don't like it nearly as much. As for the fault, it's a generic fault > > that could apply to all the CPU models (inorder, out-of-order, any other > > model) so I think it's more generic than just the o3 even though it's only > > used here. The fault could also be used in the future to get rid of some of > > the more specific squashDueToFoo instructions and just use more generic > > faults to do the job for you. > > > >
I don't think TheISA would be a problem, especially if you don't use that temporary since then you wouldn't need to specify the namespace at all. The problem, though, seems to stem from the fact that you're reading and writing the PC. Really all you need to do is write to the threadcontext to convince O3 it needs to flush the pipe, and you can do that by writing anything. You could even add a method which doesn't write anything and just triggers a flush. That also touches on what I was saying a few comments down. If you read the PC you're really creating a local copy of a structure and copying around a chunk of memory, and then when you write it again you put it back. You're not actually moving around useful data, though, so all that copying is wasted effort. If you were to use, say, an int reg, you'd at least only copy around an int. The problem with using faults more generally is that it presumes that a CPU will care that you're writing to some state. That's really just how O3 works and isn't a requirement at all, so while it might be a useful technique in other CPUs, it's not a generic mechanism. Also, if you're specialized to O3 you can just call functions on the CPU and get things to flush for sure without having to trick the CPU. > On 2011-09-01 11:54:03, Ali Saidi wrote: > > src/arch/x86/faults.hh, line 53 > > <http://reviews.m5sim.org/r/823/diff/1/?file=14617#file14617line53> > > > > This is the correct indentation per the style guide That may be, but it's not consistent with the file. Feel free to fix it, but fix it all at once. Really you shouldn't need to change this file, though, as discussed above. > On 2011-09-01 11:54:03, Ali Saidi wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 457 > > <http://reviews.m5sim.org/r/823/diff/1/?file=14623#file14623line457> > > > > There is a potential case where setPeer() is called, but the port > > doesn't know it's size until the entire memory system is setup (beacuse the > > peer doesn't know it's size directly and needs to query a peer object that > > might not yet be connected. This is a more robust implementation. Ok. > On 2011-09-01 11:54:03, Ali Saidi wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 543 > > <http://reviews.m5sim.org/r/823/diff/1/?file=14623#file14623line543> > > > > It was used before and it is an oh crap... If we actually commit this > > instruction we've done something extremely wrong. This is the same way that > > it was handled previously. Ok. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/823/#review1478 ----------------------------------------------------------- On 2011-08-18 19:41:19, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/823/ > ----------------------------------------------------------- > > (Updated 2011-08-18 19:41:19) > > > 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/alpha/faults.hh 7f49e6a176b8 > src/arch/alpha/faults.cc 7f49e6a176b8 > src/arch/arm/faults.hh 7f49e6a176b8 > src/arch/mips/faults.hh 7f49e6a176b8 > src/arch/mips/faults.cc 7f49e6a176b8 > src/arch/power/SConscript 7f49e6a176b8 > src/arch/power/faults.hh 7f49e6a176b8 > src/arch/power/faults.cc PRE-CREATION > src/arch/sparc/faults.hh 7f49e6a176b8 > src/arch/sparc/faults.cc 7f49e6a176b8 > src/arch/x86/faults.hh 7f49e6a176b8 > src/arch/x86/faults.cc 7f49e6a176b8 > src/cpu/base_dyn_inst.hh 7f49e6a176b8 > src/cpu/base_dyn_inst_impl.hh 7f49e6a176b8 > src/cpu/o3/lsq_impl.hh 7f49e6a176b8 > src/cpu/o3/lsq_unit.hh 7f49e6a176b8 > src/cpu/o3/lsq_unit_impl.hh 7f49e6a176b8 > > 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
