mem-cache: Replace block visitor with std::function

This change modifies forEachBlk tags function to accept std::function
as parameter. It also adds an anyBlk tags function that given a
condition, it iterates through the blocks and returns whether the
condition is met.

Finally, it uses forEachBlk to implement the print, computeStats and
cleanupRefs functions that also work for the FALRU class.

Change-Id: I2f75f4baa1fdd5a1d343a63ecace3eb9458fbf03
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/10621
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>

8 files changed, 143 insertions, 209 deletions

diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index c50fcdb6e..e6315cf04 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1326,27 +1326,22 @@ BaseCache::writecleanBlk(CacheBlk *blk, Request::Flags dest, PacketId id)
 void
 BaseCache::memWriteback()
 {
-    CacheBlkVisitorWrapper visitor(*this, &BaseCache::writebackVisitor);
-    tags->forEachBlk(visitor);
+    tags->forEachBlk([this](CacheBlk &blk) { writebackVisitor(blk); });
 }
 
 void
 BaseCache::memInvalidate()
 {
-    CacheBlkVisitorWrapper visitor(*this, &BaseCache::invalidateVisitor);
-    tags->forEachBlk(visitor);
+    tags->forEachBlk([this](CacheBlk &blk) { invalidateVisitor(blk); });
 }
 
 bool
 BaseCache::isDirty() const
 {
-    CacheBlkIsDirtyVisitor visitor;
-    tags->forEachBlk(visitor);
-
-    return visitor.isDirty();
+    return tags->anyBlk([](CacheBlk &blk) { return blk.isDirty(); });
 }
 
-bool
+void
 BaseCache::writebackVisitor(CacheBlk &blk)
 {
     if (blk.isDirty()) {
@@ -1366,11 +1361,9 @@ BaseCache::writebackVisitor(CacheBlk &blk)
 
         blk.status &= ~BlkDirty;
     }
-
-    return true;
 }
 
-bool
+void
 BaseCache::invalidateVisitor(CacheBlk &blk)
 {
     if (blk.isDirty())
@@ -1381,8 +1374,6 @@ BaseCache::invalidateVisitor(CacheBlk &blk)
         assert(!blk.isDirty());
         invalidateBlock(&blk);
     }
-
-    return true;
 }
 
 Tick
diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index aacf09afb..04225c12f 100644
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -1108,19 +1108,15 @@ class BaseCache : public MemObject
     /**
      * Cache block visitor that writes back dirty cache blocks using
      * functional writes.
-     *
-     * @return Always returns true.
      */
-    bool writebackVisitor(CacheBlk &blk);
+    void writebackVisitor(CacheBlk &blk);
 
     /**
      * Cache block visitor that invalidates all blocks in the cache.
      *
      * @warn Dirty cache lines will not be written back to memory.
-     *
-     * @return Always returns true.
      */
-    bool invalidateVisitor(CacheBlk &blk);
+    void invalidateVisitor(CacheBlk &blk);
 
     /**
      * Take an MSHR, turn it into a suitable downstream packet, and
@@ -1152,62 +1148,4 @@ class BaseCache : public MemObject
 
 };
 
-/**
- * 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.
- *
- * Use with the forEachBlk method in the tag array to determine if the
- * array contains dirty blocks.
- */
-class CacheBlkIsDirtyVisitor : public CacheBlkVisitor
-{
-  public:
-    CacheBlkIsDirtyVisitor()
-        : _isDirty(false) {}
-
-    bool operator()(CacheBlk &blk) override {
-        if (blk.isDirty()) {
-            _isDirty = true;
-            return false;
-        } else {
-            return true;
-        }
-    }
-
-    /**
-     * Does the array contain a dirty line?
-     *
-     * @return true if yes, false otherwise.
-     */
-    bool isDirty() const { return _isDirty; };
-
-  private:
-    bool _isDirty;
-};
-
 #endif //__MEM_CACHE_BASE_HH__
diff --git a/src/mem/cache/blk.hh b/src/mem/cache/blk.hh
index 951abd5be..c94c3ba7d 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
@@ -409,20 +409,4 @@ class CacheBlkPrintWrapper : public Printable
                const std::string &prefix = "") const;
 };
 
-/**
- * Base class for cache block visitor, operating on the cache block
- * base class (later subclassed for the various tag classes). This
- * visitor class is used as part of the forEachBlk interface in the
- * tag classes.
- */
-class CacheBlkVisitor
-{
-  public:
-
-    CacheBlkVisitor() {}
-    virtual ~CacheBlkVisitor() {}
-
-    virtual bool operator()(CacheBlk &blk) = 0;
-};
-
 #endif //__MEM_CACHE_BLK_HH__
diff --git a/src/mem/cache/tags/base.cc b/src/mem/cache/tags/base.cc
index c7ea17bd1..13e1245c6 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
@@ -111,6 +111,77 @@ BaseTags::insertBlock(PacketPtr pkt, CacheBlk *blk)
 }
 
 void
+BaseTags::cleanupRefsVisitor(CacheBlk &blk)
+{
+    if (blk.isValid()) {
+        totalRefs += blk.refCount;
+        ++sampledRefs;
+    }
+}
+
+void
+BaseTags::cleanupRefs()
+{
+    forEachBlk([this](CacheBlk &blk) { cleanupRefsVisitor(blk); });
+}
+
+void
+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]++;
+    }
+}
+
+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;
+        }
+    }
+
+    forEachBlk([this](CacheBlk &blk) { computeStatsVisitor(blk); });
+}
+
+std::string
+BaseTags::print()
+{
+    std::string str;
+
+    auto print_blk = [&str](CacheBlk &blk) {
+        if (blk.isValid())
+            str += csprintf("\tset: %d way: %d %s\n", blk.set, blk.way,
+                            blk.print());
+    };
+    forEachBlk(print_blk);
+
+    if (str.empty())
+        str = "no valid tags\n";
+
+    return str;
+}
+
+void
 BaseTags::regStats()
 {
     ClockedObject::regStats();
diff --git a/src/mem/cache/tags/base.hh b/src/mem/cache/tags/base.hh
index 806f63aae..90469cd8e 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
@@ -50,6 +50,7 @@
 #define __MEM_CACHE_TAGS_BASE_HH__
 
 #include <cassert>
+#include <functional>
 #include <string>
 
 #include "base/callback.hh"
@@ -177,17 +178,17 @@ class BaseTags : public ClockedObject
      * 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
@@ -289,7 +290,42 @@ class BaseTags : public ClockedObject
 
     virtual int extractSet(Addr addr) const = 0;
 
-    virtual void forEachBlk(CacheBlkVisitor &visitor) = 0;
+
+    /**
+     * Visit each block in the tags and apply a visitor
+     *
+     * The visitor should be a std::function that takes a cache block
+     * reference as its parameter.
+     *
+     * @param visitor Visitor to call on each block.
+     */
+    virtual void forEachBlk(std::function<void(CacheBlk &)> visitor) = 0;
+
+    /**
+     * Find if any of the blocks satisfies a condition
+     *
+     * The visitor should be a std::function that takes a cache block
+     * reference as its parameter. The visitor will terminate the
+     * traversal early if the condition is satisfied.
+     *
+     * @param visitor Visitor to call on each block.
+     */
+    virtual bool anyBlk(std::function<bool(CacheBlk &)> visitor) = 0;
+
+  private:
+    /**
+     * Update the reference stats using data from the input block
+     *
+     * @param blk The input block
+     */
+    void cleanupRefsVisitor(CacheBlk &blk);
+
+    /**
+     * Update the occupancy and age stats using data from the input block
+     *
+     * @param blk The input block
+     */
+    void 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 d3420b440..9cd93dbea 100644
--- a/src/mem/cache/tags/base_set_assoc.cc
+++ b/src/mem/cache/tags/base_set_assoc.cc
@@ -47,11 +47,9 @@
 
 #include "mem/cache/tags/base_set_assoc.hh"
 
-#include <cassert>
 #include <string>
 
 #include "base/intmath.hh"
-#include "mem/request.hh"
 
 BaseSetAssoc::BaseSetAssoc(const Params *p)
     :BaseTags(p), assoc(p->assoc), allocAssoc(p->assoc),
@@ -134,64 +132,6 @@ BaseSetAssoc::findBlockBySetAndWay(int set, int way) const
     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 bf227c067..33b2d39c4 100644
--- a/src/mem/cache/tags/base_set_assoc.hh
+++ b/src/mem/cache/tags/base_set_assoc.hh
@@ -48,6 +48,7 @@
 #ifndef __MEM_CACHE_TAGS_BASE_SET_ASSOC_HH__
 #define __MEM_CACHE_TAGS_BASE_SET_ASSOC_HH__
 
+#include <functional>
 #include <string>
 #include <vector>
 
@@ -298,38 +299,19 @@ class BaseSetAssoc : public BaseTags
         return ((blk->tag << tagShift) | ((Addr)blk->set << setShift));
     }
 
-    /**
-     * 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;
+    void forEachBlk(std::function<void(CacheBlk &)> visitor) override {
+        for (CacheBlk& blk : blks) {
+            visitor(blk);
+        }
+    }
 
-    /**
-     * Visit each block in the tag store and apply a visitor to the
-     * block.
-     *
-     * The visitor should be a function (or object that behaves like a
-     * function) that takes a cache block reference as its parameter
-     * and returns a bool. A visitor can request the traversal to be
-     * stopped by returning false, returning true causes it to be
-     * called for the next block in the tag store.
-     *
-     * \param visitor Visitor to call on each block.
-     */
-    void forEachBlk(CacheBlkVisitor &visitor) override {
+    bool anyBlk(std::function<bool(CacheBlk &)> visitor) override {
         for (CacheBlk& blk : blks) {
-            if (!visitor(blk))
-                return;
+            if (visitor(blk)) {
+                return true;
+            }
         }
+        return false;
     }
 };
 
diff --git a/src/mem/cache/tags/fa_lru.hh b/src/mem/cache/tags/fa_lru.hh
index 98a64577e..e063a6ca1 100644
--- a/src/mem/cache/tags/fa_lru.hh
+++ b/src/mem/cache/tags/fa_lru.hh
@@ -50,6 +50,7 @@
 #define __MEM_CACHE_TAGS_FA_LRU_HH__
 
 #include <cstdint>
+#include <functional>
 #include <string>
 #include <unordered_map>
 
@@ -240,28 +241,19 @@ class FALRU : public BaseTags
         return blk->tag;
     }
 
-    /**
-     * @todo Implement as in lru. Currently not used
-     */
-    virtual std::string print() const override { return ""; }
+    void forEachBlk(std::function<void(CacheBlk &)> visitor) override {
+        for (int i = 0; i < numBlocks; i++) {
+            visitor(blks[i]);
+        }
+    }
 
-    /**
-     * Visit each block in the tag store and apply a visitor to the
-     * block.
-     *
-     * The visitor should be a function (or object that behaves like a
-     * function) that takes a cache block reference as its parameter
-     * and returns a bool. A visitor can request the traversal to be
-     * stopped by returning false, returning true causes it to be
-     * called for the next block in the tag store.
-     *
-     * \param visitor Visitor to call on each block.
-     */
-    void forEachBlk(CacheBlkVisitor &visitor) override {
+    bool anyBlk(std::function<bool(CacheBlk &)> visitor) override {
         for (int i = 0; i < numBlocks; i++) {
-            if (!visitor(blks[i]))
-                return;
+            if (visitor(blks[i])) {
+                return true;
+            }
         }
+        return false;
     }
 
   private: