> On April 1, 2015, 11:30 a.m., Steve Reinhardt wrote:
> > src/mem/cache/tags/base_set_assoc.cc, line 202
> > <http://reviews.gem5.org/r/2711/diff/1/?file=44369#file44369line202>
> >
> > do we really want to replace the visitor pattern with this? I don't
> > know how often this is used, but building this whole list seems like
> > unnecessary overhead. I guess what we really need is the C++ equivalent of
> > python generators...
>
> Andreas Hansson wrote:
> It is only used as part of flushing and checkpointing so it should not be
> a performance issue. Perhaps leave it as is for now?
>
> Steve Reinhardt wrote:
> Perhaps... if we were just implementing this for the first time, it would
> only be a little sad, but the fact that we had a nice callback structure and
> are replacing it with this inefficient approach makes me extra sad, and the
> fact that we're still layering a callback interface on top of it just deepens
> the sorrow :(.
>
> Wouldn't it be as simple as replacing BaseTags::getBlkList() with
> virtual bool visitBlocks(Cache *, VisitorFnPtr fn) = 0;
>
> then rewrite this function:
>
> bool
> Cache::visitBlocks(VisitorFnPtr fn)
> {
> return tags->visitBlocks(this, fn);
> }
>
> and for example:
>
> bool
> BaseSetAssoc::visitBlocks(Cache *cache, VisitorFnPtr fn)
> {
> for (...) {
> for (...) {
> if (!(cache->*fn)(sets[i].blks[j]))
> return false;
> }
> }
>
> return true;
> }
Sorry about the line wrapping... I hypothesize that I should have un-checked
the 'enable markdown' box on that one. Let's see if that helps:
bool
BaseSetAssoc::visitBlocks(Cache *cache, VisitorFnPtr fn)
{
for (...) {
for (...) {
if (!(cache->*fn)(sets[i].blks[j]))
return false;
}
}
return true;
}
- Steve
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2711/#review6008
-----------------------------------------------------------
On April 2, 2015, 2:09 a.m., Andreas Hansson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2711/
> -----------------------------------------------------------
>
> (Updated April 2, 2015, 2:09 a.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 10777:7a17ca9141fb
> ---------------------------
> mem: Remove templates in cache model
>
> This patch changes the cache implementation to rely on virtual methods
> rather than using the replacement policy as a template argument.
>
> There is no impact on the simulation performance, and overall the
> changes make it easier to modify (and subclass) the cache and/or
> replacement policy.
>
>
> Diffs
> -----
>
> src/mem/cache/base.cc 8a7285d6197e
> src/mem/cache/blk.hh 8a7285d6197e
> src/mem/cache/cache.hh 8a7285d6197e
> src/mem/cache/cache.cc 8a7285d6197e
> src/mem/cache/cache_impl.hh 8a7285d6197e
> src/mem/cache/tags/base.hh 8a7285d6197e
> src/mem/cache/tags/base_set_assoc.hh 8a7285d6197e
> src/mem/cache/tags/base_set_assoc.cc 8a7285d6197e
> src/mem/cache/tags/fa_lru.hh 8a7285d6197e
> src/mem/cache/tags/fa_lru.cc 8a7285d6197e
> src/mem/cache/tags/lru.hh 8a7285d6197e
> src/mem/cache/tags/lru.cc 8a7285d6197e
> src/mem/cache/tags/random_repl.hh 8a7285d6197e
> src/mem/cache/tags/random_repl.cc 8a7285d6197e
>
> Diff: http://reviews.gem5.org/r/2711/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andreas Hansson
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
