> On 2012-01-13 19:29:21, Korey Sewell wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 773 > > <http://reviews.m5sim.org/r/908/diff/5/?file=21036#file21036line773> > > > > Here, > > we should use a member variable inside the LSQUnit instead of the > > pointer reference to the CPU's member variable. > > > > That way we get: > > if (needsTSO) > > storeInFlight=false > > > > vs. > > > > if (cpu->needsTSO) > > storeInFlight=false > > Nilay Vaish wrote: > Why does this matter?
In terms of overall runtime of the simulator, it probably wont matter considering all the other things with larger overheads (e.g. event scheduling, memory allocation, etc.) But isolating just this code snippet, the "cpu->" part of that condition is going to cause unnecessary overhead of looking up that pointer to get to the value of "needsTSO". It caught my eye because you are doing that 3 times per store: - once to check if a store's in flight - once to mark a store in flight - once to unmark a store in flight Ideally, things that are actually changing values are worth the pointer reference (e.g. the interruptPending flag) whereas things that are generally constant parameters (e.g. needTSO) aren't worth it. Again, it's not going to be the biggest deal in terms of overall impact and I'm definitely nitpicking. Just something to consider as you go forward with these O3 patches... - Korey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/908/#review1947 ----------------------------------------------------------- On 2012-01-13 04:57:18, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/908/ > ----------------------------------------------------------- > > (Updated 2012-01-13 04:57:18) > > > Review request for Default. > > > Summary > ------- > > Changeset 8702:9a5651e7bd5b > --------------------------- > O3 LSQ: Implement TSO > This patch makes O3's LSQ maintain total order between stores. Essentially > only the store at the head of the store buffer is allowed to be in flight. > Only after that store completes, the next store is issued to the memory > system. > > > Diffs > ----- > > src/cpu/o3/O3CPU.py f348cf78072c > src/cpu/o3/cpu.hh f348cf78072c > src/cpu/o3/cpu.cc f348cf78072c > src/cpu/o3/lsq_unit.hh f348cf78072c > src/cpu/o3/lsq_unit_impl.hh f348cf78072c > > Diff: http://reviews.m5sim.org/r/908/diff > > > Testing > ------- > > > Thanks, > > Nilay > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
