> On April 18, 2013, 3:52 a.m., Andreas Hansson wrote: > > Overall this looks like it's going in the right direction. I have some > > concerns and issue we should resolve, but overall it's a nice addition to > > the current functionality.
Thank you very much for the comment! I will delay this patch until the changesets for new bus model are pushed to the main repo. > On April 18, 2013, 3:52 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 1863 > > <http://reviews.gem5.org/r/1826/diff/1/?file=35126#file35126line1863> > > > > I am a little concerned here as I am not sure what will happen first: > > the bank being marked free or the retry sent? > > > > Ultimately I would suggest to complicate things a little bit further > > and set a flag here (mustSendRespRetry or similar) and then in the event > > that unblocks the bank (is there one?) send out the retry once everything > > else is done. I agree. I also feel here is some approximation. But, it should be removed if later a dedicated bank free event is introduced. > On April 18, 2013, 3:52 a.m., Andreas Hansson wrote: > > src/mem/coherent_bus.hh, line 134 > > <http://reviews.gem5.org/r/1826/diff/1/?file=35127#file35127line134> > > > > I'm not thrilled at how it's done at the moment, but I can see the need > > for something similar. > > > > I have quite some bus patches in the pipeline, I'll try and get them on > > the reviewboard early next week. before the bus patch release, I will leave it as is. > On April 18, 2013, 3:52 a.m., Andreas Hansson wrote: > > src/mem/coherent_bus.cc, line 248 > > <http://reviews.gem5.org/r/1826/diff/1/?file=35128#file35128line248> > > > > As above, in general this looks like the right way forward. > > > > I have some major reshuffling of this in the pipeline, making the buses > > properly multi-layered and accounting for their own time, so I'd like to > > wait with adding this functionality until that is done if that's ok. as above - Xiangyu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1826/#review4247 ----------------------------------------------------------- On April 17, 2013, 11:19 a.m., Xiangyu Dong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1826/ > ----------------------------------------------------------- > > (Updated April 17, 2013, 11:19 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9639:2aae67c49743 > --------------------------- > mem: add retry mechanism for cache fills in classic cache model > The changeset 6122d201ff80 modeled the cache bank and blocks the cache access > if the target bank is busy. However, due to the lack of retry mechanism at > the > cache master port, that changeset cannot properly blocks the cache traffic > that > is towards CPU. > This patch modifies the CoherentBus model and adds a flow control scheme to > the > RespLayer. With this modification, cache fill operations can now be properly > modeled. > @todo The modification to CoherentBus is a little hacky. e.g. The recvRetry > functions for Req and Resp are not symmetric > @todo Might also need to modify noncoherent bus > @todo There is no write buffer entries for cache fill operations. An > incremental patch will be needed to model the cache fill buffer > > > Diffs > ----- > > src/mem/coherent_bus.hh 6d4158ff7b82 > src/mem/coherent_bus.cc 6d4158ff7b82 > src/mem/cache/cache_impl.hh 6d4158ff7b82 > src/mem/cache/base.hh 6d4158ff7b82 > > Diff: http://reviews.gem5.org/r/1826/diff/ > > > Testing > ------- > > > Thanks, > > Xiangyu Dong > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
