Any testing done?

src/mem/cache/prefetch/base.hh (Diff revision 1)

Tick and unsigned int rather than int, or is there a reason to ever have negative values?

Show all issues

src/mem/cache/prefetch/base.hh (Diff revision 1)

Is setCache necessary or can we do this in the constructor?

Show all issues

1. Ali Saidi Feb. 3, 2012, 7:28 a.m. (Feb. 3, 2012, 7:28 a.m.)

   how could it be done in the constructor? cache has a pointer to the prefetcher, not the other way around. 

2. Andreas Hansson Feb. 8, 2012, 5:58 p.m. (Feb. 8, 2012, 5:58 p.m.)

   I was more thinking along the lines of knowing who the parent is, similar to previous discussions. There is clearly an object hierarchy here and the cache is the parent of the prefetcher.

src/mem/cache/prefetch/ghb.cc (Diff revision 1)

whitespace?

Show all issues

src/mem/cache/prefetch/stride.cc (Diff revision 1)

whitespace?

Show all issues

Anyone have any final comments?

src/mem/cache/prefetch/Prefetcher.py (Diff revision 1)

Can we take this opportunity to remove the 'prefetch(er)_' prefixes from these param names?  The original purpose was to distinguish them among the cache params, but that's not necessary anymore, and now it's just redundant and verbose.

Show all issues

1. Ali Saidi Feb. 7, 2012, 6:52 a.m. (Feb. 7, 2012, 6:52 a.m.)

   for size I think it makes sense, but what about for the rest of them? e.g. prefetch_past_page? Seems more descriptive than past_page.
   
   size, serial_squash, degree, latency, use_cpu_id seem like good candidates, but I'm a bit unsure about  past_pages nd data_access_only? Either way I don't care so just tell me what you'd prefer and I'll fix it once.

2. Steve Reinhardt Feb. 7, 2012, 7:10 a.m. (Feb. 7, 2012, 7:10 a.m.)

   Good point, I didn't look at them individually.  I think all of them are fine just truncating the prefix except prefetch_past_page.  That one I'd just rename to cross_pages or cross_page_boundaries.  Then the names are all reasonably correlated with their description strings, which seems like a good thing.

src/mem/cache/prefetch/base.hh (Diff revision 1)

How about some comments here?

Show all issues

src/mem/cache/prefetch/ghb.hh (Diff revision 1)

Shouldn't these be removed since they're in the base class now?

Show all issues

src/mem/cache/prefetch/tagged.hh (Diff revision 1)

Same here.

Show all issues

src/mem/cache/prefetch/tagged.cc (Diff revision 1)

These initializations should be in the base constructor too.

Show all issues