> On July 31, 2015, 11:13 p.m., Joel Hestness wrote: > > src/mem/ruby/system/Sequencer.cc, line 553 > > <http://reviews.gem5.org/r/2787/diff/3/?file=48362#file48362line553> > > > > I'm still not convinced that this should be allowed, since this assumes > > the L1 cache is RFO. My preference is that the read request be issued to > > the caches instead (mirroring the readCallback). If the caches are RFO, > > then issuing reads here should complete in the next cycle, so this > > shouldn't affect performance much. > > Brad Beckmann wrote: > There is a lot of code in the Sequencer that assumes RfO (example the > block on calls to support RMW).
This is not cool. First, I added this request in my original review here, so it should have been addressed previously. Second, your reasoning here for not addressing my request doesn't hold water: RMW operations are atomics that require ownership prior to performing the operation. Otherwise, the results would be nonsense. Even if there is other code in the Sequencer that assumes RFO, we don't need to perpetuate these assumptions. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2787/#review6875 ----------------------------------------------------------- On July 22, 2015, 6:15 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2787/ > ----------------------------------------------------------- > > (Updated July 22, 2015, 6:15 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10887:1e05089bc991 > --------------------------- > ruby: Fixed pipeline squashes caused by aliased requests > > This patch was created by Bihn Pham during his internship at AMD. > > This patch fixes a very significant performance bug when using the O3 CPU > model > and Ruby. The issue was Ruby returned false when it received a request to the > same address that already has an outstanding request or when the memory is > blocked. As a result, O3 unnecessary squashed the pipeline and re-executed > instructions. This fix merges readRequestTable and writeRequestTable in > Sequencer into a single request table that keeps track of all requests and > allows multiple outstanding requests to the same address. This prevents O3 > from squashing the pipeline. > > > Diffs > ----- > > src/mem/ruby/system/Sequencer.hh ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 > src/mem/ruby/system/Sequencer.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 > src/mem/ruby/system/Sequencer.py ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 > > Diff: http://reviews.gem5.org/r/2787/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
