> On Sept. 30, 2015, 7:01 p.m., Sooraj Puthoor wrote: > > Hi Nilay, > > I could be wrong but it looks to me that this patch is more like a > > "buffering of requests in the sequencer" and not really coalescing of > > requests. The reason being this patch actually issues the aliased requests > > to the memory subsystem on a hit callback whereas I was expecting > > coalescing to reduce the number of requests to the memory subsystem by > > issuing only a single request to the cacheline. Did I miss something? > > Nilay Vaish wrote: > You are right, the requests are only buffered in the sequencer and a new > one is issued when > the previous one finishes. I don't think we can assume that the > controller attached > to the sequencer does not need to see all the requests. The stats go off > and then the > controller might be doing things that would not happen if does not get to > see all the requests. > > Sooraj Puthoor wrote: > Thanks for the clarification. > I think it will be better to replace the term "coalesce" to "buffer" in > the patch description. And rename the "coalesce_reqs" parameter into > "buffer_reqs". > > By the way, is there any particular reason for adding this functionality?
Ok, I'll change the names. I have seen the o3 cpu misbehaving when requests are not allowed. So we do need a solution for that. I am personally using a slightly different version of this patch, where the latency of queueing to the mandatory queue is reduced by the amount of time the request was buffered in the sequencer. > On Sept. 30, 2015, 7:01 p.m., Sooraj Puthoor wrote: > > src/mem/ruby/system/Sequencer.cc, line 546 > > <http://reviews.gem5.org/r/3099/diff/1/?file=49223#file49223line546> > > > > From a readability perspective, would it be better if this one large > > condition is broken down into smaller conditions with some comments? > > Nilay Vaish wrote: > What would you like to see? > > Sooraj Puthoor wrote: > Some comments like "//if buffering enabled and request is aliased, do xyz > else do zyx Other than changing coalescing to buffering, I think what I have here is similar to what you are suggesting. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3099/#review7313 ----------------------------------------------------------- On Sept. 5, 2015, 12:20 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3099/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2015, 12:20 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11106:04307eea6681 > --------------------------- > ruby: sequencer: coalesce requests > > This patch coalesces requests to that are for an address which already > has a pending request. The requests coming in later are issued after the > original request has completed. > > > Diffs > ----- > > src/mem/ruby/system/Sequencer.hh 95eb22928d0f > src/mem/ruby/system/Sequencer.cc 95eb22928d0f > src/mem/ruby/system/Sequencer.py 95eb22928d0f > > Diff: http://reviews.gem5.org/r/3099/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
