-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support new prefetching methodology for stride prefetcher #7
base: memtrace
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Saba,
thanks for your patch!
The code does not compile on my side. Also added 6 nits. Please fix and send another pull request.
--
build/opt/trace_init.cpp: In function ‘CacheGroup* BuildCacheGroup(Config&, const string&, bool)’:
build/opt/trace_init.cpp:435:73: error: no matching function for call to ‘StreamPrefetcher::StreamPrefetcher(g_string&, uint32_t&, uint32_t&, uint32_t&)’
log_distance, degree);
^
In file included from build/opt/trace_init.cpp:59:0:
build/opt/prefetcher.h:132:18: note: candidate: StreamPrefetcher::StreamPrefetcher(const g_string&, uint32_t, uint32_t, uint32_t, const g_string&)
explicit StreamPrefetcher(const g_string& _name,
^~~~~~~~~~~~~~~~
build/opt/prefetcher.h:132:18: note: candidate expects 5 arguments, 4 provided
In file included from build/opt/trace_init.cpp:59:0:
build/opt/prefetcher.h:68:7: note: candidate: StreamPrefetcher::StreamPrefetcher(const StreamPrefetcher&)
class StreamPrefetcher : public BaseCache{
^~~~~~~~~~~~~~~~
build/opt/prefetcher.h:68:7: note: candidate expects 1 argument, 4 provided
build/opt/prefetcher.h:68:7: note: candidate: StreamPrefetcher::StreamPrefetcher(StreamPrefetcher&&)
build/opt/prefetcher.h:68:7: note: candidate expects 1 argument, 4 provided
scons: *** [build/opt/trace_init.o] Error 1
scons: building terminated because of errors.
@@ -71,7 +73,7 @@ int32_t SetAssocArray::lookup(const Address lineAddr, const MemReq* req, bool up | |||
profPrefSavedCycles.inc(array[id].availCycle - array[id].startCycle); | |||
} | |||
} | |||
//line is in flight, compensate for potential OOO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: no reason for this, please revert
@@ -94,7 +96,6 @@ int32_t SetAssocArray::lookup(const Address lineAddr, const MemReq* req, bool up | |||
profPrefHit.inc(); | |||
} | |||
array[id].prefetch = false; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: no reason for this, please revert
@@ -115,7 +116,6 @@ uint32_t SetAssocArray::preinsert(const Address lineAddr, const MemReq* req, Add | |||
uint32_t candidate = rp->rankCands(req, SetAssocCands(first, first+assoc)); | |||
|
|||
*wbLineAddr = array[candidate].addr; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: no reason for this, please revert
@@ -26,10 +26,12 @@ | |||
#include "cache_arrays.h" | |||
#include "hash.h" | |||
#include "repl_policies.h" | |||
#include <inttypes.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not clear why these are needed. There should be no changes to cache_arrays.cpp in this pull request.
@@ -69,19 +163,40 @@ void StreamPrefetcher::initStats(AggregateStat* parentStat) { | |||
profStrideSwitches.init("strideSwitches", "Predicted stride switches"); s->append(&profStrideSwitches); | |||
profLowConfAccs.init("lcAccs", "Low-confidence accesses with no prefetches"); s->append(&profLowConfAccs); | |||
parentStat->append(s); | |||
prof_emitted_prefetches_.init("pff", "Total prefetches emitted"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: identation
@@ -58,7 +65,7 @@ class SatCounter { | |||
* FIXME: For now, mostly hardcoded; 64-line entries (4KB w/64-byte lines), fixed granularities, etc. | |||
* TODO: Adapt to use weave models | |||
*/ | |||
class StreamPrefetcher : public BaseCache { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: leave spacing
src/prefetcher.cpp
Outdated
|
||
//#define DBG(args...) info(args) | ||
#define DBG(args...) | ||
|
||
void StreamPrefetcher::postInit() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this copied from basecache? This class should just inherit from basecache.
Hi Heiner, |
No description provided.