Hello Andreas Sandberg,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/10621
to review the following change.
Change subject: mem-cache: Use block visitor to update tags stats and print
tags
......................................................................
mem-cache: Use block visitor to update tags stats and print tags
This change uses the block visitor to perform stat updates and print
tags as uneccesary and moves the corresponding functions to the
BaseTags class. This also implements the print, computeStats and
cleanupRefs for the FALRU class.
Change-Id: I2f75f4baa1fdd5a1d343a63ecace3eb9458fbf03
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/mem/cache/base.cc
M src/mem/cache/base.hh
M src/mem/cache/blk.hh
M src/mem/cache/tags/base.cc
M src/mem/cache/tags/base.hh
M src/mem/cache/tags/base_set_assoc.cc
M src/mem/cache/tags/base_set_assoc.hh
M src/mem/cache/tags/fa_lru.hh
8 files changed, 148 insertions(+), 111 deletions(-)
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index c50fcdb..2254303 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1326,14 +1326,16 @@
void
BaseCache::memWriteback()
{
- CacheBlkVisitorWrapper visitor(*this, &BaseCache::writebackVisitor);
+ CacheBlkVisitorWrapper<BaseCache> visitor(
+ *this, &BaseCache::writebackVisitor);
tags->forEachBlk(visitor);
}
void
BaseCache::memInvalidate()
{
- CacheBlkVisitorWrapper visitor(*this, &BaseCache::invalidateVisitor);
+ CacheBlkVisitorWrapper<BaseCache> visitor(
+ *this, &BaseCache::invalidateVisitor);
tags->forEachBlk(visitor);
}
diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index aacf09a..3c318d9 100644
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -1153,31 +1153,6 @@
};
/**
- * Wrap a method and present it as a cache block visitor.
- *
- * For example the forEachBlk method in the tag arrays expects a
- * callable object/function as their parameter. This class wraps a
- * method in an object and presents callable object that adheres to
- * the cache block visitor protocol.
- */
-class CacheBlkVisitorWrapper : public CacheBlkVisitor
-{
- public:
- typedef bool (BaseCache::*VisitorPtr)(CacheBlk &blk);
-
- CacheBlkVisitorWrapper(BaseCache &_cache, VisitorPtr _visitor)
- : cache(_cache), visitor(_visitor) {}
-
- bool operator()(CacheBlk &blk) override {
- return (cache.*visitor)(blk);
- }
-
- private:
- BaseCache &cache;
- VisitorPtr visitor;
-};
-
-/**
* Cache block visitor that determines if there are dirty blocks in a
* cache.
*
diff --git a/src/mem/cache/blk.hh b/src/mem/cache/blk.hh
index 951abd5..efb70d4 100644
--- a/src/mem/cache/blk.hh
+++ b/src/mem/cache/blk.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2017 ARM Limited
+ * Copyright (c) 2012-2018 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -425,4 +425,52 @@
virtual bool operator()(CacheBlk &blk) = 0;
};
+/**
+ * Wrap a method and present it as a cache block visitor.
+ *
+ * For example the forEachBlk method in the tag arrays expects a
+ * callable object/function as their parameter. This class wraps a
+ * method in an object and presents callable object that adheres to
+ * the cache block visitor protocol.
+ */
+template<class Object>
+class CacheBlkVisitorWrapper : public CacheBlkVisitor
+{
+ public:
+ typedef bool (Object::*VisitorPtr)(CacheBlk &blk);
+
+ CacheBlkVisitorWrapper(Object &_object, VisitorPtr _visitor)
+ : object(_object), visitor(_visitor) {}
+
+ bool operator()(CacheBlk &blk) override {
+ return (object.*visitor)(blk);
+ }
+
+ private:
+ Object &object;
+ VisitorPtr visitor;
+};
+
+/**
+ * Wrap a tags print method and present it as a cache block visitor.
+ *
+ * For example the forEachBlk method in the tag arrays expects a
+ * callable object/function as their parameter. This class wraps a
+ * method in an object and presents a callable object that adheres to
+ * the cache block visitor protocol.
+ */
+class CacheBlkPrintVisitor : public CacheBlkVisitor
+{
+ public:
+ bool operator()(CacheBlk &blk) override {
+ if (blk.isValid())
+ str += csprintf("\tset: %d way: %d %s\n", blk.set, blk.way,
+ blk.print());
+ return true;
+ }
+
+ std::string str;
+};
+
+
#endif //__MEM_CACHE_BLK_HH__
diff --git a/src/mem/cache/tags/base.cc b/src/mem/cache/tags/base.cc
index c7ea17b..8ed4278 100644
--- a/src/mem/cache/tags/base.cc
+++ b/src/mem/cache/tags/base.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013,2016 ARM Limited
+ * Copyright (c) 2013,2016,2018 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -110,6 +110,79 @@
dataAccesses += 1;
}
+bool
+BaseTags::cleanupRefsVisitor(CacheBlk &blk)
+{
+ if (blk.isValid()) {
+ totalRefs += blk.refCount;
+ ++sampledRefs;
+ }
+
+ return true;
+}
+
+void
+BaseTags::cleanupRefs()
+{
+ CacheBlkVisitorWrapper<BaseTags> visitor(
+ *this, &BaseTags::cleanupRefsVisitor);
+ forEachBlk(visitor);
+}
+
+bool
+BaseTags::computeStatsVisitor(CacheBlk &blk)
+{
+ if (blk.isValid()) {
+ assert(blk.task_id < ContextSwitchTaskId::NumTaskId);
+ occupanciesTaskId[blk.task_id]++;
+ assert(blk.tickInserted <= curTick());
+ Tick age = curTick() - blk.tickInserted;
+
+ int age_index;
+ if (age / SimClock::Int::us < 10) { // <10us
+ age_index = 0;
+ } else if (age / SimClock::Int::us < 100) { // <100us
+ age_index = 1;
+ } else if (age / SimClock::Int::ms < 1) { // <1ms
+ age_index = 2;
+ } else if (age / SimClock::Int::ms < 10) { // <10ms
+ age_index = 3;
+ } else
+ age_index = 4; // >10ms
+
+ ageTaskId[blk.task_id][age_index]++;
+ }
+
+ return true;
+}
+
+void
+BaseTags::computeStats()
+{
+ for (unsigned i = 0; i < ContextSwitchTaskId::NumTaskId; ++i) {
+ occupanciesTaskId[i] = 0;
+ for (unsigned j = 0; j < 5; ++j) {
+ ageTaskId[i][j] = 0;
+ }
+ }
+
+ CacheBlkVisitorWrapper<BaseTags> visitor(
+ *this, &BaseTags::computeStatsVisitor);
+ forEachBlk(visitor);
+}
+
+std::string
+BaseTags::print()
+{
+ CacheBlkPrintVisitor print_visitor;
+ forEachBlk(print_visitor);
+
+ if (print_visitor.str.empty())
+ print_visitor.str = "no valid tags\n";
+
+ return print_visitor.str;
+}
+
void
BaseTags::regStats()
{
diff --git a/src/mem/cache/tags/base.hh b/src/mem/cache/tags/base.hh
index 806f63a..4997955 100644
--- a/src/mem/cache/tags/base.hh
+++ b/src/mem/cache/tags/base.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2014,2016-2017 ARM Limited
+ * Copyright (c) 2012-2014,2016-2018 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -177,17 +177,17 @@
* Average in the reference count for valid blocks when the simulation
* exits.
*/
- virtual void cleanupRefs() {}
+ void cleanupRefs();
/**
* Computes stats just prior to dump event
*/
- virtual void computeStats() {}
+ void computeStats();
/**
* Print all tags used
*/
- virtual std::string print() const = 0;
+ std::string print();
/**
* Find a block using the memory address
@@ -290,6 +290,23 @@
virtual int extractSet(Addr addr) const = 0;
virtual void forEachBlk(CacheBlkVisitor &visitor) = 0;
+
+ private:
+ /**
+ * Update the reference stats using data from the input block
+ *
+ * @param blk The input block
+ * @return Always true as forEachBlk terminates otherwise
+ */
+ bool cleanupRefsVisitor(CacheBlk &blk);
+
+ /**
+ * Update the occupancy and age stats using data from the input block
+ *
+ * @param blk The input block
+ * @return Always true as forEachBlk terminates otherwise
+ */
+ bool computeStatsVisitor(CacheBlk &blk);
};
class BaseTagsCallback : public Callback
diff --git a/src/mem/cache/tags/base_set_assoc.cc
b/src/mem/cache/tags/base_set_assoc.cc
index d3420b4..2254a33 100644
--- a/src/mem/cache/tags/base_set_assoc.cc
+++ b/src/mem/cache/tags/base_set_assoc.cc
@@ -134,64 +134,6 @@
return sets[set].blks[way];
}
-std::string
-BaseSetAssoc::print() const {
- std::string cache_state;
- for (const CacheBlk& blk : blks) {
- if (blk.isValid())
- cache_state += csprintf("\tset: %d way: %d %s\n", blk.set,
- blk.way, blk.print());
- }
- if (cache_state.empty())
- cache_state = "no valid tags\n";
- return cache_state;
-}
-
-void
-BaseSetAssoc::cleanupRefs()
-{
- for (const CacheBlk& blk : blks) {
- if (blk.isValid()) {
- totalRefs += blk.refCount;
- ++sampledRefs;
- }
- }
-}
-
-void
-BaseSetAssoc::computeStats()
-{
- for (unsigned i = 0; i < ContextSwitchTaskId::NumTaskId; ++i) {
- occupanciesTaskId[i] = 0;
- for (unsigned j = 0; j < 5; ++j) {
- ageTaskId[i][j] = 0;
- }
- }
-
- for (const CacheBlk& blk : blks) {
- if (blk.isValid()) {
- assert(blk.task_id < ContextSwitchTaskId::NumTaskId);
- occupanciesTaskId[blk.task_id]++;
- assert(blk.tickInserted <= curTick());
- Tick age = curTick() - blk.tickInserted;
-
- int age_index;
- if (age / SimClock::Int::us < 10) { // <10us
- age_index = 0;
- } else if (age / SimClock::Int::us < 100) { // <100us
- age_index = 1;
- } else if (age / SimClock::Int::ms < 1) { // <1ms
- age_index = 2;
- } else if (age / SimClock::Int::ms < 10) { // <10ms
- age_index = 3;
- } else
- age_index = 4; // >10ms
-
- ageTaskId[blk.task_id][age_index]++;
- }
- }
-}
-
BaseSetAssoc *
BaseSetAssocParams::create()
{
diff --git a/src/mem/cache/tags/base_set_assoc.hh
b/src/mem/cache/tags/base_set_assoc.hh
index 2aa3a74..6aa2ad7 100644
--- a/src/mem/cache/tags/base_set_assoc.hh
+++ b/src/mem/cache/tags/base_set_assoc.hh
@@ -299,21 +299,6 @@
}
/**
- * Called at end of simulation to complete average block reference
stats.
- */
- void cleanupRefs() override;
-
- /**
- * Print all tags used
- */
- std::string print() const override;
-
- /**
- * Called prior to dumping stats to compute task occupancy
- */
- void computeStats() override;
-
- /**
* Visit each block in the tag store and apply a visitor to the
* block.
*
diff --git a/src/mem/cache/tags/fa_lru.hh b/src/mem/cache/tags/fa_lru.hh
index 98a6457..5785d82 100644
--- a/src/mem/cache/tags/fa_lru.hh
+++ b/src/mem/cache/tags/fa_lru.hh
@@ -241,11 +241,6 @@
}
/**
- * @todo Implement as in lru. Currently not used
- */
- virtual std::string print() const override { return ""; }
-
- /**
* Visit each block in the tag store and apply a visitor to the
* block.
*
--
To view, visit https://gem5-review.googlesource.com/10621
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I2f75f4baa1fdd5a1d343a63ecace3eb9458fbf03
Gerrit-Change-Number: 10621
Gerrit-PatchSet: 1
Gerrit-Owner: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
