From 3692ad77b396ccb9189832dfb942e8ef05830293 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Mon, 19 Jul 2021 13:06:11 +0100 Subject: [PATCH 1/9] i#3068: API for accessing cache model metrics This patch contains implementation of the API for accessing separate metrics gathered during the simulation. Functions for accessing metrics were added for all caches in the default configuration: L1I, L1D and LL. Also additional functions for getting configuration options of the cache_simulator_t. Issue: #3068 Change-Id: I2970ca57886d96c68c92d7c17f0c9df920163152 --- .../drcachesim/simulator/cache_simulator.cpp | 85 +++++++++++++++++++ .../drcachesim/simulator/cache_simulator.h | 22 +++++ clients/drcachesim/simulator/cache_stats.cpp | 3 + .../simulator/caching_device_stats.cpp | 9 ++ .../simulator/caching_device_stats.h | 29 +++++++ 5 files changed, 148 insertions(+) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 78ddd43933f..8ccae3a53a0 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -594,6 +594,91 @@ cache_simulator_t::print_results() return true; } +inline std::string +cache_simulator_t::get_l1_icache_name(unsigned int core) const { + return "L1_I_Cache_" + std::to_string(core); +} + +inline std::string +cache_simulator_t::get_l1_dcache_name(unsigned int core) const { + return "L1_D_Cache_" + std::to_string(core); +} + +const caching_device_stats_t * +cache_simulator_t::get_cache_stats(const std::string& cache_type) const { + auto pos = all_caches_.find(cache_type); + + if (pos != all_caches_.end()) { + return pos->second->get_stats(); + } else { + ERRMSG("Couldn't get statistics. Wrong cache type.\n"); + return nullptr; + } +} + +int_least64_t +cache_simulator_t::get_l1i_metric(metric_name_t metric, unsigned int core) const +{ + if (core >= knobs_.num_cores) { + ERRMSG("Wrong core number: %u.\n", core); + return 0; + } + + const auto *stats = get_cache_stats(get_l1_icache_name(core)); + + if (!stats) { + return 0; + } + + return stats->get_metric(metric); +} + +int_least64_t +cache_simulator_t::get_l1d_metric(metric_name_t metric, unsigned int core) const +{ + if (core >= knobs_.num_cores) { + ERRMSG("Wrong core number: %u.\n", core); + return 0; + } + + const auto *stats = get_cache_stats(get_l1_dcache_name(core)); + + if (!stats) { + return 0; + } + + return stats->get_metric(metric); +} + +int_least64_t +cache_simulator_t::get_ll_metric(metric_name_t metric) const +{ + const auto *stats = get_cache_stats(ll_cache_name); + + if (!stats) { + return 0; + } + + return stats->get_metric(metric); +} + +unsigned int +cache_simulator_t::get_core_num() const { + return knobs_.num_cores; +} + +bool +cache_simulator_t::is_prefetcher_used() const +{ + return knobs_.data_prefetcher != PREFETCH_POLICY_NONE; +} + +bool +cache_simulator_t::is_cache_coherent() const +{ + return knobs_.model_coherence; +} + cache_t * cache_simulator_t::create_cache(const std::string &policy) { diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index b70825a1334..b63ef6ea665 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -60,12 +60,26 @@ class cache_simulator_t : public simulator_t { bool print_results() override; + int_least64_t + get_l1i_metric(metric_name_t metric, unsigned int core) const; + int_least64_t + get_l1d_metric(metric_name_t metric, unsigned int core) const; + int_least64_t + get_ll_metric(metric_name_t metric) const; + // Exposed to make it easy to test bool check_warmed_up(); uint64_t remaining_sim_refs() const; + unsigned int + get_core_num() const; + bool + is_prefetcher_used() const; + bool + is_cache_coherent() const; + protected: // Create a cache_t object with a specific replacement policy. virtual cache_t * @@ -92,6 +106,14 @@ class cache_simulator_t : public simulator_t { snoop_filter_t *snoop_filter_ = nullptr; private: + std::string + get_l1_icache_name(unsigned int core) const; + std::string + get_l1_dcache_name(unsigned int core) const; + const caching_device_stats_t * + get_cache_stats(const std::string& cache_type) const; + + const std::string ll_cache_name = "LL"; bool is_warmed_up_; }; diff --git a/clients/drcachesim/simulator/cache_stats.cpp b/clients/drcachesim/simulator/cache_stats.cpp index 3013aa7fbb3..1d0e4bc084a 100644 --- a/clients/drcachesim/simulator/cache_stats.cpp +++ b/clients/drcachesim/simulator/cache_stats.cpp @@ -41,6 +41,9 @@ cache_stats_t::cache_stats_t(const std::string &miss_file, bool warmup_enabled, , num_prefetch_hits_(0) , num_prefetch_misses_(0) { + stats_map_.emplace(FLUSHES, num_flushes_); + stats_map_.emplace(PREFETCH_HITS, num_prefetch_hits_); + stats_map_.emplace(PREFETCH_MISSES, num_prefetch_misses_); } void diff --git a/clients/drcachesim/simulator/caching_device_stats.cpp b/clients/drcachesim/simulator/caching_device_stats.cpp index 62df1967bb8..da2dddb6983 100644 --- a/clients/drcachesim/simulator/caching_device_stats.cpp +++ b/clients/drcachesim/simulator/caching_device_stats.cpp @@ -64,6 +64,15 @@ caching_device_stats_t::caching_device_stats_t(const std::string &miss_file, } else dump_misses_ = true; } + + stats_map_.emplace(HITS, num_hits_); + stats_map_.emplace(MISSES, num_misses_); + stats_map_.emplace(HITS_AT_RESET, num_hits_at_reset_); + stats_map_.emplace(MISSES_AT_RESET, num_misses_at_reset_); + stats_map_.emplace(CHILD_HITS_AT_RESET, num_child_hits_at_reset_); + stats_map_.emplace(CHILD_HITS, num_child_hits_); + stats_map_.emplace(INCLUSIVE_INVALIDATES, num_inclusive_invalidates_); + stats_map_.emplace(COHERENCE_INVALIDATES, num_coherence_invalidates_); } caching_device_stats_t::~caching_device_stats_t() diff --git a/clients/drcachesim/simulator/caching_device_stats.h b/clients/drcachesim/simulator/caching_device_stats.h index 52ea9758fb8..e69e8cb9247 100644 --- a/clients/drcachesim/simulator/caching_device_stats.h +++ b/clients/drcachesim/simulator/caching_device_stats.h @@ -38,6 +38,7 @@ #include "caching_device_block.h" #include +#include #include #ifdef HAS_ZLIB # include @@ -49,6 +50,20 @@ enum invalidation_type_t { INVALIDATION_COHERENCE, }; +enum metric_name_t { + HITS, + MISSES, + HITS_AT_RESET, + MISSES_AT_RESET, + CHILD_HITS, + CHILD_HITS_AT_RESET, + INCLUSIVE_INVALIDATES, + COHERENCE_INVALIDATES, + PREFETCH_HITS, + PREFETCH_MISSES, + FLUSHES +}; + class caching_device_stats_t { public: explicit caching_device_stats_t(const std::string &miss_file, @@ -81,6 +96,16 @@ class caching_device_stats_t { virtual void invalidate(invalidation_type_t invalidation_type); + int_least64_t + get_metric(metric_name_t metric) const { + if (stats_map_.find(metric) != stats_map_.end()) { + return stats_map_.at(metric); + } else { + ERRMSG("Wrong metric name.\n"); + return 0; + } + } + protected: bool success_; @@ -115,6 +140,10 @@ class caching_device_stats_t { // Print out write invalidations if cache is coherent. bool is_coherent_; + // References to the properties with statistics are held in the map with the + // statistic name as the key. Sample map element: {HITS, num_hits_} + std::map stats_map_; + // We provide a feature of dumping misses to a file. bool dump_misses_; #ifdef HAS_ZLIB From 334395abf4073113bdef89e71e0ac3b297aee3c9 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Tue, 20 Jul 2021 16:08:49 +0100 Subject: [PATCH 2/9] Fixing formatting errors This commit contains fixes for minor formatting errors which caused clang format check failure. --- clients/drcachesim/simulator/cache_simulator.cpp | 12 ++++++++---- clients/drcachesim/simulator/cache_simulator.h | 2 +- clients/drcachesim/simulator/caching_device_stats.h | 5 +++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 8ccae3a53a0..630f7347e5d 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -595,17 +595,20 @@ cache_simulator_t::print_results() } inline std::string -cache_simulator_t::get_l1_icache_name(unsigned int core) const { +cache_simulator_t::get_l1_icache_name(unsigned int core) const +{ return "L1_I_Cache_" + std::to_string(core); } inline std::string -cache_simulator_t::get_l1_dcache_name(unsigned int core) const { +cache_simulator_t::get_l1_dcache_name(unsigned int core) const +{ return "L1_D_Cache_" + std::to_string(core); } const caching_device_stats_t * -cache_simulator_t::get_cache_stats(const std::string& cache_type) const { +cache_simulator_t::get_cache_stats(const std::string& cache_type) const +{ auto pos = all_caches_.find(cache_type); if (pos != all_caches_.end()) { @@ -663,7 +666,8 @@ cache_simulator_t::get_ll_metric(metric_name_t metric) const } unsigned int -cache_simulator_t::get_core_num() const { +cache_simulator_t::get_core_num() const +{ return knobs_.num_cores; } diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index b63ef6ea665..51547a5ec3f 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -111,7 +111,7 @@ class cache_simulator_t : public simulator_t { std::string get_l1_dcache_name(unsigned int core) const; const caching_device_stats_t * - get_cache_stats(const std::string& cache_type) const; + get_cache_stats(const std::string &cache_type) const; const std::string ll_cache_name = "LL"; bool is_warmed_up_; diff --git a/clients/drcachesim/simulator/caching_device_stats.h b/clients/drcachesim/simulator/caching_device_stats.h index e69e8cb9247..950e2881c06 100644 --- a/clients/drcachesim/simulator/caching_device_stats.h +++ b/clients/drcachesim/simulator/caching_device_stats.h @@ -97,7 +97,8 @@ class caching_device_stats_t { invalidate(invalidation_type_t invalidation_type); int_least64_t - get_metric(metric_name_t metric) const { + get_metric(metric_name_t metric) const + { if (stats_map_.find(metric) != stats_map_.end()) { return stats_map_.at(metric); } else { @@ -142,7 +143,7 @@ class caching_device_stats_t { // References to the properties with statistics are held in the map with the // statistic name as the key. Sample map element: {HITS, num_hits_} - std::map stats_map_; + std::map stats_map_; // We provide a feature of dumping misses to a file. bool dump_misses_; From f5c83c69e9711219798bfce5e2c7eeae199f725e Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Tue, 20 Jul 2021 16:14:55 +0100 Subject: [PATCH 3/9] Fixing additional formatting error This commit contains fix for additional formatting error that wasn't spotted in the previous commit. --- clients/drcachesim/simulator/cache_simulator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 630f7347e5d..a8723fe3b7f 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -607,7 +607,7 @@ cache_simulator_t::get_l1_dcache_name(unsigned int core) const } const caching_device_stats_t * -cache_simulator_t::get_cache_stats(const std::string& cache_type) const +cache_simulator_t::get_cache_stats(const std::string &cache_type) const { auto pos = all_caches_.find(cache_type); From 343d5bf16a8b218fe4c8b4fe1bbcea2c421ef7c5 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Thu, 29 Jul 2021 16:19:50 +0100 Subject: [PATCH 4/9] Adding unit test and code structure improvements This patch contains implementation of the unit test and improvements to the code structure pointed out in the review comments. Change-Id: I19bc8a28c614219b2cb6d14f4445d23504a74815 --- .../drcachesim/simulator/cache_simulator.cpp | 90 +++++-------------- .../drcachesim/simulator/cache_simulator.h | 28 ++---- .../tests/drcachesim_unit_tests.cpp | 63 +++++++++++++ 3 files changed, 93 insertions(+), 88 deletions(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index a8723fe3b7f..301c63b1a32 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -594,93 +594,45 @@ cache_simulator_t::print_results() return true; } -inline std::string -cache_simulator_t::get_l1_icache_name(unsigned int core) const -{ - return "L1_I_Cache_" + std::to_string(core); -} - -inline std::string -cache_simulator_t::get_l1_dcache_name(unsigned int core) const -{ - return "L1_D_Cache_" + std::to_string(core); -} - -const caching_device_stats_t * -cache_simulator_t::get_cache_stats(const std::string &cache_type) const -{ - auto pos = all_caches_.find(cache_type); - - if (pos != all_caches_.end()) { - return pos->second->get_stats(); - } else { - ERRMSG("Couldn't get statistics. Wrong cache type.\n"); - return nullptr; - } -} - int_least64_t -cache_simulator_t::get_l1i_metric(metric_name_t metric, unsigned int core) const +cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, + unsigned core, cache_split_t split) const { - if (core >= knobs_.num_cores) { - ERRMSG("Wrong core number: %u.\n", core); - return 0; - } - - const auto *stats = get_cache_stats(get_l1_icache_name(core)); - - if (!stats) { - return 0; - } - - return stats->get_metric(metric); -} + caching_device_t *curr_cache; -int_least64_t -cache_simulator_t::get_l1d_metric(metric_name_t metric, unsigned int core) const -{ if (core >= knobs_.num_cores) { ERRMSG("Wrong core number: %u.\n", core); return 0; } - const auto *stats = get_cache_stats(get_l1_dcache_name(core)); - - if (!stats) { - return 0; + if (split == DATA) { + curr_cache = l1_dcaches_[core]; + } else { + curr_cache = l1_icaches_[core]; } - return stats->get_metric(metric); -} + for (size_t i = 1; i < level; i++) { + caching_device_t *parent = curr_cache->get_parent(); -int_least64_t -cache_simulator_t::get_ll_metric(metric_name_t metric) const -{ - const auto *stats = get_cache_stats(ll_cache_name); + if (parent == NULL) { + ERRMSG("Wrong cache level: %u.\n", core); + return 0; + } + curr_cache = parent; + } + caching_device_stats_t *stats = curr_cache->get_stats(); - if (!stats) { + if (stats == NULL) { + ERRMSG("Cache doesn't support statistics counting."); return 0; } return stats->get_metric(metric); } -unsigned int -cache_simulator_t::get_core_num() const -{ - return knobs_.num_cores; -} - -bool -cache_simulator_t::is_prefetcher_used() const -{ - return knobs_.data_prefetcher != PREFETCH_POLICY_NONE; -} - -bool -cache_simulator_t::is_cache_coherent() const -{ - return knobs_.model_coherence; +const cache_simulator_knobs_t & +cache_simulator_t::get_knobs() const { + return knobs_; } cache_t * diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index 51547a5ec3f..5ed7074fde8 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -43,6 +43,11 @@ #include "cache.h" #include "snoop_filter.h" +enum cache_split_t { + DATA, + INSTRUCTION +}; + class cache_simulator_t : public simulator_t { public: // This constructor is used when the cache hierarchy is configured @@ -61,11 +66,8 @@ class cache_simulator_t : public simulator_t { print_results() override; int_least64_t - get_l1i_metric(metric_name_t metric, unsigned int core) const; - int_least64_t - get_l1d_metric(metric_name_t metric, unsigned int core) const; - int_least64_t - get_ll_metric(metric_name_t metric) const; + get_cache_metric(unsigned level, metric_name_t metric, unsigned core = 0, + cache_split_t split = DATA) const; // Exposed to make it easy to test bool @@ -73,12 +75,8 @@ class cache_simulator_t : public simulator_t { uint64_t remaining_sim_refs() const; - unsigned int - get_core_num() const; - bool - is_prefetcher_used() const; - bool - is_cache_coherent() const; + const cache_simulator_knobs_t & + get_knobs() const; protected: // Create a cache_t object with a specific replacement policy. @@ -106,14 +104,6 @@ class cache_simulator_t : public simulator_t { snoop_filter_t *snoop_filter_ = nullptr; private: - std::string - get_l1_icache_name(unsigned int core) const; - std::string - get_l1_dcache_name(unsigned int core) const; - const caching_device_stats_t * - get_cache_stats(const std::string &cache_type) const; - - const std::string ll_cache_name = "LL"; bool is_warmed_up_; }; diff --git a/clients/drcachesim/tests/drcachesim_unit_tests.cpp b/clients/drcachesim/tests/drcachesim_unit_tests.cpp index bdd82a6e2d9..6d9c8c3c2d4 100644 --- a/clients/drcachesim/tests/drcachesim_unit_tests.cpp +++ b/clients/drcachesim/tests/drcachesim_unit_tests.cpp @@ -33,6 +33,7 @@ // Unit tests for drcachesim #include #include +#include #include "simulator/cache_simulator.h" #include "../common/memref.h" @@ -135,9 +136,71 @@ unit_test_sim_refs() } } +void +unit_test_metrics_API() +{ + cache_simulator_knobs_t knobs = make_test_knobs(); + cache_simulator_t cache_sim(knobs); + + memref_t ref; + ref.data.type = TRACE_TYPE_WRITE; + ref.data.addr = 0; + ref.data.size = 8; + + for (int i = 0; i < 4; i++) { + if (!cache_sim.process_memref(ref)) { + std::cerr << "drcachesim unit_test_metrics_API failed: " + << cache_sim.get_error_string() << "\n"; + exit(1); + } + } + assert(cache_sim.get_cache_metric(1, MISSES, 0, DATA) == 1); + assert(cache_sim.get_cache_metric(1, HITS, 0, DATA) == 3); + + ref.data.type = TRACE_TYPE_INSTR; + + for (int i = 0; i < 4; i++) { + if (!cache_sim.process_memref(ref)) { + std::cerr << "drcachesim unit_test_metrics_API failed: " + << cache_sim.get_error_string() << "\n"; + exit(1); + } + } + assert(cache_sim.get_cache_metric(1, MISSES, 0, INSTRUCTION) == 1); + assert(cache_sim.get_cache_metric(1, HITS, 0, INSTRUCTION) == 3); + + assert(cache_sim.get_cache_metric(2, MISSES) == 1); + assert(cache_sim.get_cache_metric(2, HITS) == 1); + + ref.data.type = TRACE_TYPE_PREFETCH; + ref.data.addr += 64; + + for (int i = 0; i < 4; i++) { + if (!cache_sim.process_memref(ref)) { + std::cerr << "drcachesim unit_test_metrics_API failed: " + << cache_sim.get_error_string() << "\n"; + exit(1); + } + } + assert(cache_sim.get_cache_metric(1, PREFETCH_MISSES, 0, DATA) == 1); + assert(cache_sim.get_cache_metric(1, PREFETCH_HITS, 0, DATA) == 3); + + ref.data.type = TRACE_TYPE_DATA_FLUSH; + + for (int i = 0; i < 4; i++) { + if (!cache_sim.process_memref(ref)) { + std::cerr << "drcachesim unit_test_metrics_API failed: " + << cache_sim.get_error_string() << "\n"; + exit(1); + } + } + assert(cache_sim.get_cache_metric(2, FLUSHES) == 4); +} + int main(int argc, const char *argv[]) { + unit_test_metrics_API(); unit_test_warmup_fraction(); unit_test_warmup_refs(); unit_test_sim_refs(); From 49b40309161012e07b715b84a240fc81ae3a2e58 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Mon, 2 Aug 2021 17:49:12 +0100 Subject: [PATCH 5/9] Fixing formatting errors This commit contains fixes for a few formatting errors which caused clang format test failure. --- clients/drcachesim/simulator/cache_simulator.cpp | 7 ++++--- clients/drcachesim/simulator/cache_simulator.h | 5 +---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 301c63b1a32..906c81efad1 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -595,8 +595,8 @@ cache_simulator_t::print_results() } int_least64_t -cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, - unsigned core, cache_split_t split) const +cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, unsigned core, + cache_split_t split) const { caching_device_t *curr_cache; @@ -631,7 +631,8 @@ cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, } const cache_simulator_knobs_t & -cache_simulator_t::get_knobs() const { +cache_simulator_t::get_knobs() const +{ return knobs_; } diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index 5ed7074fde8..9e13f38ae60 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -43,10 +43,7 @@ #include "cache.h" #include "snoop_filter.h" -enum cache_split_t { - DATA, - INSTRUCTION -}; +enum cache_split_t { DATA, INSTRUCTION }; class cache_simulator_t : public simulator_t { public: From a609f6744a8589ce84a3321212af52b659840d10 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Wed, 4 Aug 2021 12:11:58 +0100 Subject: [PATCH 6/9] Code style and functionality improvements This patch contains improvements to the functionality and style of the code. Enums were adjusted to prevent polluting global namespace and approach to report errors was changed. --- .../drcachesim/simulator/cache_simulator.cpp | 13 +++++-------- clients/drcachesim/simulator/cache_simulator.h | 18 +++++++++++++++--- clients/drcachesim/simulator/cache_stats.cpp | 6 +++--- .../simulator/caching_device_stats.cpp | 16 ++++++++-------- .../simulator/caching_device_stats.h | 2 +- .../drcachesim/tests/drcachesim_unit_tests.cpp | 3 +++ 6 files changed, 35 insertions(+), 23 deletions(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 906c81efad1..7e0c9cdd31e 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -595,17 +595,16 @@ cache_simulator_t::print_results() } int_least64_t -cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, unsigned core, +cache_simulator_t::get_cache_metric(metric_name_t metric, unsigned level, unsigned core, cache_split_t split) const { caching_device_t *curr_cache; if (core >= knobs_.num_cores) { - ERRMSG("Wrong core number: %u.\n", core); - return 0; + return STATS_ERROR_WRONG_CORE_NUMBER; } - if (split == DATA) { + if (split == cache_split_t::DATA) { curr_cache = l1_dcaches_[core]; } else { curr_cache = l1_icaches_[core]; @@ -615,16 +614,14 @@ cache_simulator_t::get_cache_metric(unsigned level, metric_name_t metric, unsign caching_device_t *parent = curr_cache->get_parent(); if (parent == NULL) { - ERRMSG("Wrong cache level: %u.\n", core); - return 0; + return STATS_ERROR_WRONG_CACHE_LEVEL; } curr_cache = parent; } caching_device_stats_t *stats = curr_cache->get_stats(); if (stats == NULL) { - ERRMSG("Cache doesn't support statistics counting."); - return 0; + return STATS_ERROR_NO_CACHE_STATS; } return stats->get_metric(metric); diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index 9e13f38ae60..464eb081d2d 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -42,8 +42,20 @@ #include "cache_stats.h" #include "cache.h" #include "snoop_filter.h" +#include -enum cache_split_t { DATA, INSTRUCTION }; +enum class cache_split_t { DATA, INSTRUCTION }; + +// Error codes returned when passing wrong parameters to the function for +// accessing cache statistics. +typedef enum { + // Core number is larger then congifured number of cores. + STATS_ERROR_WRONG_CORE_NUMBER = INT_MIN, + // Cache level is larger then configured number of levels. + STATS_ERROR_WRONG_CACHE_LEVEL, + // Given cache doesn't support counting statistics. + STATS_ERROR_NO_CACHE_STATS, +} stats_error_t; class cache_simulator_t : public simulator_t { public: @@ -63,8 +75,8 @@ class cache_simulator_t : public simulator_t { print_results() override; int_least64_t - get_cache_metric(unsigned level, metric_name_t metric, unsigned core = 0, - cache_split_t split = DATA) const; + get_cache_metric(metric_name_t metric, unsigned level, unsigned core = 0, + cache_split_t split = cache_split_t::DATA) const; // Exposed to make it easy to test bool diff --git a/clients/drcachesim/simulator/cache_stats.cpp b/clients/drcachesim/simulator/cache_stats.cpp index 1d0e4bc084a..bd72857227c 100644 --- a/clients/drcachesim/simulator/cache_stats.cpp +++ b/clients/drcachesim/simulator/cache_stats.cpp @@ -41,9 +41,9 @@ cache_stats_t::cache_stats_t(const std::string &miss_file, bool warmup_enabled, , num_prefetch_hits_(0) , num_prefetch_misses_(0) { - stats_map_.emplace(FLUSHES, num_flushes_); - stats_map_.emplace(PREFETCH_HITS, num_prefetch_hits_); - stats_map_.emplace(PREFETCH_MISSES, num_prefetch_misses_); + stats_map_.emplace(metric_name_t::FLUSHES, num_flushes_); + stats_map_.emplace(metric_name_t::PREFETCH_HITS, num_prefetch_hits_); + stats_map_.emplace(metric_name_t::PREFETCH_MISSES, num_prefetch_misses_); } void diff --git a/clients/drcachesim/simulator/caching_device_stats.cpp b/clients/drcachesim/simulator/caching_device_stats.cpp index da2dddb6983..aa11f1a56f3 100644 --- a/clients/drcachesim/simulator/caching_device_stats.cpp +++ b/clients/drcachesim/simulator/caching_device_stats.cpp @@ -65,14 +65,14 @@ caching_device_stats_t::caching_device_stats_t(const std::string &miss_file, dump_misses_ = true; } - stats_map_.emplace(HITS, num_hits_); - stats_map_.emplace(MISSES, num_misses_); - stats_map_.emplace(HITS_AT_RESET, num_hits_at_reset_); - stats_map_.emplace(MISSES_AT_RESET, num_misses_at_reset_); - stats_map_.emplace(CHILD_HITS_AT_RESET, num_child_hits_at_reset_); - stats_map_.emplace(CHILD_HITS, num_child_hits_); - stats_map_.emplace(INCLUSIVE_INVALIDATES, num_inclusive_invalidates_); - stats_map_.emplace(COHERENCE_INVALIDATES, num_coherence_invalidates_); + stats_map_.emplace(metric_name_t::HITS, num_hits_); + stats_map_.emplace(metric_name_t::MISSES, num_misses_); + stats_map_.emplace(metric_name_t::HITS_AT_RESET, num_hits_at_reset_); + stats_map_.emplace(metric_name_t::MISSES_AT_RESET, num_misses_at_reset_); + stats_map_.emplace(metric_name_t::CHILD_HITS_AT_RESET, num_child_hits_at_reset_); + stats_map_.emplace(metric_name_t::CHILD_HITS, num_child_hits_); + stats_map_.emplace(metric_name_t::INCLUSIVE_INVALIDATES, num_inclusive_invalidates_); + stats_map_.emplace(metric_name_t::COHERENCE_INVALIDATES, num_coherence_invalidates_); } caching_device_stats_t::~caching_device_stats_t() diff --git a/clients/drcachesim/simulator/caching_device_stats.h b/clients/drcachesim/simulator/caching_device_stats.h index 950e2881c06..885ad6c37f3 100644 --- a/clients/drcachesim/simulator/caching_device_stats.h +++ b/clients/drcachesim/simulator/caching_device_stats.h @@ -50,7 +50,7 @@ enum invalidation_type_t { INVALIDATION_COHERENCE, }; -enum metric_name_t { +enum class metric_name_t { HITS, MISSES, HITS_AT_RESET, diff --git a/clients/drcachesim/tests/drcachesim_unit_tests.cpp b/clients/drcachesim/tests/drcachesim_unit_tests.cpp index 6d9c8c3c2d4..4b04ff4b962 100644 --- a/clients/drcachesim/tests/drcachesim_unit_tests.cpp +++ b/clients/drcachesim/tests/drcachesim_unit_tests.cpp @@ -147,6 +147,9 @@ unit_test_metrics_API() ref.data.addr = 0; ref.data.size = 8; + // Currently invalidates are not counted properly in the configuration of + // cache_simulator_t with cache_simulator_knobs_t. + // TODO i#5031: Test invalidates metric when the issue is solved. for (int i = 0; i < 4; i++) { if (!cache_sim.process_memref(ref)) { std::cerr << "drcachesim unit_test_metrics_API failed: " From 38011c0c2336732d7ad714f3c3c9f3a6cd1288c7 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Wed, 4 Aug 2021 15:03:31 +0100 Subject: [PATCH 7/9] Fixing enum names in the unit test This patch fixes enum names in the metrics API unit tests. After changing enum types in the simulator code old names were left in the test code by mistake. --- .../tests/drcachesim_unit_tests.cpp | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/clients/drcachesim/tests/drcachesim_unit_tests.cpp b/clients/drcachesim/tests/drcachesim_unit_tests.cpp index 4b04ff4b962..ddebbbcc75d 100644 --- a/clients/drcachesim/tests/drcachesim_unit_tests.cpp +++ b/clients/drcachesim/tests/drcachesim_unit_tests.cpp @@ -157,8 +157,10 @@ unit_test_metrics_API() exit(1); } } - assert(cache_sim.get_cache_metric(1, MISSES, 0, DATA) == 1); - assert(cache_sim.get_cache_metric(1, HITS, 0, DATA) == 3); + assert(cache_sim.get_cache_metric(metric_name_t::MISSES, 1, 0, + cache_split_t::DATA) == 1); + assert(cache_sim.get_cache_metric(metric_name_t::HITS, 1, 0, + cache_split_t::DATA) == 3); ref.data.type = TRACE_TYPE_INSTR; @@ -169,11 +171,13 @@ unit_test_metrics_API() exit(1); } } - assert(cache_sim.get_cache_metric(1, MISSES, 0, INSTRUCTION) == 1); - assert(cache_sim.get_cache_metric(1, HITS, 0, INSTRUCTION) == 3); + assert(cache_sim.get_cache_metric(metric_name_t::MISSES, 1, 0, + cache_split_t::INSTRUCTION) == 1); + assert(cache_sim.get_cache_metric(metric_name_t::HITS, 1, 0, + cache_split_t::INSTRUCTION) == 3); - assert(cache_sim.get_cache_metric(2, MISSES) == 1); - assert(cache_sim.get_cache_metric(2, HITS) == 1); + assert(cache_sim.get_cache_metric(metric_name_t::MISSES, 2) == 1); + assert(cache_sim.get_cache_metric(metric_name_t::HITS, 2) == 1); ref.data.type = TRACE_TYPE_PREFETCH; ref.data.addr += 64; @@ -185,8 +189,10 @@ unit_test_metrics_API() exit(1); } } - assert(cache_sim.get_cache_metric(1, PREFETCH_MISSES, 0, DATA) == 1); - assert(cache_sim.get_cache_metric(1, PREFETCH_HITS, 0, DATA) == 3); + assert(cache_sim.get_cache_metric(metric_name_t::PREFETCH_MISSES, 1, 0, + cache_split_t::DATA) == 1); + assert(cache_sim.get_cache_metric(metric_name_t::PREFETCH_HITS, 1, 0, + cache_split_t::DATA) == 3); ref.data.type = TRACE_TYPE_DATA_FLUSH; @@ -197,7 +203,7 @@ unit_test_metrics_API() exit(1); } } - assert(cache_sim.get_cache_metric(2, FLUSHES) == 4); + assert(cache_sim.get_cache_metric(metric_name_t::FLUSHES, 2) == 4); } int From 333eec16a83ffbe457bfcc99789a4e3e3664f996 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Wed, 4 Aug 2021 15:19:10 +0100 Subject: [PATCH 8/9] Fixing formatting error in the unit test This patch fixes one formatting error in the unit test. Function call was split into two lines in the wrong way. --- clients/drcachesim/tests/drcachesim_unit_tests.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clients/drcachesim/tests/drcachesim_unit_tests.cpp b/clients/drcachesim/tests/drcachesim_unit_tests.cpp index ddebbbcc75d..031bda164d6 100644 --- a/clients/drcachesim/tests/drcachesim_unit_tests.cpp +++ b/clients/drcachesim/tests/drcachesim_unit_tests.cpp @@ -157,10 +157,10 @@ unit_test_metrics_API() exit(1); } } - assert(cache_sim.get_cache_metric(metric_name_t::MISSES, 1, 0, - cache_split_t::DATA) == 1); - assert(cache_sim.get_cache_metric(metric_name_t::HITS, 1, 0, - cache_split_t::DATA) == 3); + assert(cache_sim.get_cache_metric(metric_name_t::MISSES, 1, 0, cache_split_t::DATA) == + 1); + assert(cache_sim.get_cache_metric(metric_name_t::HITS, 1, 0, cache_split_t::DATA) == + 3); ref.data.type = TRACE_TYPE_INSTR; From 81db1b8d815cf8ef076ce8a303e30a49a6e275a6 Mon Sep 17 00:00:00 2001 From: Mateusz Kalinowski Date: Thu, 5 Aug 2021 12:03:58 +0100 Subject: [PATCH 9/9] Adding and improving comments This patch adds one comment and improves existing one. --- clients/drcachesim/simulator/cache_simulator.cpp | 2 ++ clients/drcachesim/simulator/cache_simulator.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/simulator/cache_simulator.cpp b/clients/drcachesim/simulator/cache_simulator.cpp index 7e0c9cdd31e..07c5ca2ef1a 100644 --- a/clients/drcachesim/simulator/cache_simulator.cpp +++ b/clients/drcachesim/simulator/cache_simulator.cpp @@ -594,6 +594,8 @@ cache_simulator_t::print_results() return true; } +// All valid metrics are returned as a positive number. +// Negative return value is an error and is of type stats_error_t. int_least64_t cache_simulator_t::get_cache_metric(metric_name_t metric, unsigned level, unsigned core, cache_split_t split) const diff --git a/clients/drcachesim/simulator/cache_simulator.h b/clients/drcachesim/simulator/cache_simulator.h index 464eb081d2d..61b7b5911f2 100644 --- a/clients/drcachesim/simulator/cache_simulator.h +++ b/clients/drcachesim/simulator/cache_simulator.h @@ -46,8 +46,8 @@ enum class cache_split_t { DATA, INSTRUCTION }; -// Error codes returned when passing wrong parameters to the function for -// accessing cache statistics. +// Error codes returned when passing wrong parameters to the +// get_cache_metric function. typedef enum { // Core number is larger then congifured number of cores. STATS_ERROR_WRONG_CORE_NUMBER = INT_MIN,