> On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 1782 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34752#file34752line1782> > > > > Can we not use the same method for both cases and consider the bank one > > of the reasons used for blocking (and thus also call unblock once the bank > > is available)?
I considered to model it in this way. But, a busy bank only "partially" blocks a cache. The existing block causes indicate whether a cache is entirely blocked or not. I need to think about how to combine these two schemes. > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 1835 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34752#file34752line1835> > > > > I am not sure I understand this bit. Is the bank blocked until a > > response comes back? If so, why? Is the idea not to block while the > > read/write port of the SRAM is busy? Ideally, the code should unblock the bank as long as curTick() reaches bank's nextIdleTick. However, I don't want to create a new event to do that. Instead, I add these check-and-unblock codes at the beginning of each existing event handler. I will add some comments here. > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 84 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34752#file34752line84> > > > > bank.push_back? I claimed the vector size in the initialization list, and didn't want to dynamically change the vector size. > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 315 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34752#file34752line315> > > > > Are there other cases here? lat was originally set from the inside of accessBlock(). There are other cases here, but I leave lat untouched in those cases. Maybe I need to rewrite accessBlock() and set lat to the correct value at the very beginning. > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/cache_impl.hh, line 101 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34752#file34752line101> > > > > I would suggest iterators. At least use bank.size() for the limit Use bank.size() now. And also use ++i instead of i++. I will change them to iterators later if c++11 auto type is widely used. > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > configs/common/Options.py, line 74 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34747#file34747line74> > > > > For the address striping in the bus I opted for the high bit. I don't > > really care too much, but it would be good to keep it consistent. Changed from lowest bit to highest bit > On April 16, 2013, 10:47 a.m., Andreas Hansson wrote: > > src/mem/cache/base.hh, line 184 > > <http://reviews.gem5.org/r/1809/diff/1/?file=34750#file34750line184> > > > > This is rather unfortunate. Please keep as much as possible > > private/protected changed to protected - Xiangyu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1809/#review4234 ----------------------------------------------------------- On March 31, 2013, 3:48 p.m., Xiangyu Dong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1809/ > ----------------------------------------------------------- > > (Updated March 31, 2013, 3:48 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9627:6122d201ff80 > --------------------------- > mem: model data array bank in classic cache > The classic cache does not model data array bank, i.e. if a read/write is > being > serviced by a cache bank, no other requests should be sent to this bank. > This patch models a multi-bank cache. Features include: > 1. detect if the bank interleave granularity is larger than cache line size > 2. add CacheBank debug flag > 3. Differentiate read and write latency > 3a. read latency is still named as hit_latency > 3b. write latency is named as write_latency > 4. Add write_latency, num_banks, bank_itlv_bit into the Python parser > Not modeled in this patch: > Due to the lack of retry mechanism in the cache master port, the access form > the memory side will not be denied if the bank is in service. Instead, the > bank > service time will be extended. This is equivalent to an infinite write buffer > for cache fill operations. > > > Diffs > ----- > > configs/common/Caches.py 47591444a7c5 > configs/common/Options.py 47591444a7c5 > src/mem/cache/BaseCache.py 47591444a7c5 > src/mem/cache/SConscript 47591444a7c5 > src/mem/cache/base.hh 47591444a7c5 > src/mem/cache/base.cc 47591444a7c5 > src/mem/cache/cache_impl.hh 47591444a7c5 > configs/common/CacheConfig.py 47591444a7c5 > > Diff: http://reviews.gem5.org/r/1809/diff/ > > > Testing > ------- > > > Thanks, > > Xiangyu Dong > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
