> On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/inst_queue.hh, line 322 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40490#file40490line322> > > > > I think we need better differentiation between this list and the one > > declared after it. > > > > On further reading, it seems that we may not need the two lists. Can > > we just mark the instructions that they should be retried? While adding > > them back to the ready queue, we can check which ones are marked. Or may > > be keep an iterator that tracks the point till which we > > should retry. > > > > One more thought, can we do with a queue instead of a list?
That could be done. Real hardware would likely mark these "blocked" instructions with a bit to guard from execution in the IQ. The clearing of the blocked cause (cache blocked in this case) would flash-clear that guard bit for all load instructions in the IQ. Functionally these two approaches should be equivalent. The same alternative implementation could have been done for "deferred" memory instructions. I just coded it similarly to deferred instructions were already handled. I haven't looked to verify how easy this alternative implementation would be to do. A queue could be used, the only difference would be that the clear() would be replaced with a swap with an empty queue. > On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/inst_queue_impl.hh, line 416 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40491#file40491line416> > > > > Should we not clear retryMemInsts as well? Yep, will add. > On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/lsq_unit.hh, line 888 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40494#file40494line888> > > > > Do we need all these changes that appear over next 15-20 lines? It > > seems from my initial reading that the previous code structure could have > > been retained. I moved it below during the coding to clearly show the deletion events. I hit a few issues due to bad deletions and split loads. Having all deletions done together and not mixed in with code segments above made it clearer to me. > On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/inst_queue_impl.hh, line 759 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40491#file40491line759> > > > > Let's retain the new line above this while loop. Ok. > On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/inst_queue_impl.hh, line 1116 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40491#file40491line1116> > > > > We should use nullptr now that we have gcc minimum dependency at 4.6. Hmm, at one point in this patch or another I had to try to return nullptr and someone took issue with it I believe. Either way, personally I don't care. Grepping our code base (this might be different from the mainline gem5) it seems we only use nullptr eleven times in the code base and only in network-related code. Should a later "consistency" patch try to change this? Because currently it looks like NULL is the preferred. $ grep -r NULL * | wc -l 1515 $ grep -r nullptr * | wc -l 11 > On Aug. 19, 2014, 5:11 p.m., Nilay Vaish wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 111 > > <http://reviews.gem5.org/r/2332/diff/1/?file=40495#file40495line111> > > > > New line after. Will do. - Mitch ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2332/#review5272 ----------------------------------------------------------- On Aug. 13, 2014, 2:06 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2332/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2014, 2:06 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10300:bddebc19285f > --------------------------- > cpu: Fix cached block load behavior in o3 cpu > > This patch fixes the load blocked/replay mechanism in the o3 cpu. Rather than > flushing the entire pipeline, this patch replays loads once the cache becomes > unblocked. > > Additionally, deferred memory instructions (loads which had conflicting > stores), > when replayed would not respect the number of functional units (only respected > issue width). This patch also corrects that. > > Improvements over 20% have been observed on a microbenchmark designed to > exercise this behavior. > > > Diffs > ----- > > src/cpu/o3/iew.hh 79fde1c67ed8 > src/cpu/o3/iew_impl.hh 79fde1c67ed8 > src/cpu/o3/inst_queue.hh 79fde1c67ed8 > src/cpu/o3/inst_queue_impl.hh 79fde1c67ed8 > src/cpu/o3/lsq.hh 79fde1c67ed8 > src/cpu/o3/lsq_impl.hh 79fde1c67ed8 > src/cpu/o3/lsq_unit.hh 79fde1c67ed8 > src/cpu/o3/lsq_unit_impl.hh 79fde1c67ed8 > src/cpu/o3/mem_dep_unit.hh 79fde1c67ed8 > src/cpu/o3/mem_dep_unit_impl.hh 79fde1c67ed8 > > Diff: http://reviews.gem5.org/r/2332/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
