Nikos Nikoleris has submitted this change and it was merged. ( https://gem5-review.googlesource.com/10621 )

Change subject: mem-cache: Replace block visitor with std::function
......................................................................

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.sandb...@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/10621
Reviewed-by: Jason Lowe-Power <ja...@lowepower.com>
Reviewed-by: Daniel Carvalho <oda...@yahoo.com.br>
Maintainer: Nikos Nikoleris <nikos.nikole...@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, 145 insertions(+), 211 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Daniel Carvalho: Looks good to me, approved
  Nikos Nikoleris: Looks good to me, approved



diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index c50fcdb..e6315cf 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1326,27 +1326,22 @@
 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 @@

         blk.status &= ~BlkDirty;
     }
-
-    return true;
 }

-bool
+void
 BaseCache::invalidateVisitor(CacheBlk &blk)
 {
     if (blk.isDirty())
@@ -1381,8 +1374,6 @@
         assert(!blk.isDirty());
         invalidateBlock(&blk);
     }
-
-    return true;
 }

 Tick
diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index aacf09a..04225c1 100644
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -1108,19 +1108,15 @@
     /**
      * 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 @@

 };

-/**
- * 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 951abd5..c94c3ba 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 @@
                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 c7ea17b..13e1245 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 @@
 }

 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 806f63a..90469cd 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 @@
      * 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 @@

     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 d3420b4..9cd93db 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 @@
     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 bf227c0..33b2d39 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,39 +299,20 @@
         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;
-
-    /**
-     * 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 {
+    void forEachBlk(std::function<void(CacheBlk &)> visitor) override {
         for (CacheBlk& blk : blks) {
-            if (!visitor(blk))
-                return;
+            visitor(blk);
         }
     }
+
+    bool anyBlk(std::function<bool(CacheBlk &)> visitor) override {
+        for (CacheBlk& blk : blks) {
+            if (visitor(blk)) {
+                return true;
+            }
+        }
+        return false;
+    }
 };

 #endif //__MEM_CACHE_TAGS_BASE_SET_ASSOC_HH__
diff --git a/src/mem/cache/tags/fa_lru.hh b/src/mem/cache/tags/fa_lru.hh
index 98a6457..e063a6c 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,30 +241,21 @@
         return blk->tag;
     }

-    /**
-     * @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.
-     *
-     * 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 {
+    void forEachBlk(std::function<void(CacheBlk &)> visitor) override {
         for (int i = 0; i < numBlocks; i++) {
-            if (!visitor(blks[i]))
-                return;
+            visitor(blks[i]);
         }
     }

+    bool anyBlk(std::function<bool(CacheBlk &)> visitor) override {
+        for (int i = 0; i < numBlocks; i++) {
+            if (visitor(blks[i])) {
+                return true;
+            }
+        }
+        return false;
+    }
+
   private:
     /**
      * Mechanism that allows us to simultaneously collect miss

--
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: 5
Gerrit-Owner: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to