From 519808c02f2d5d780bf09589d2e29803a5af1ade Mon Sep 17 00:00:00 2001 From: "Daniel R. Carvalho" Date: Sat, 18 Jan 2020 16:19:53 +0100 Subject: [PATCH] mem-cache: Fix invalidation of prefetchers Add an invalidation function to the AssociativeSet, so that entries can be properly invalidated by also invalidating their replacement data. Both setInvalid and reset have been merged into invalidate to indicate users that they are using an incorrect approach by generating compilation errors, and to match CacheBlk's naming convention. Change-Id: I568076a3b5adda8b1311d9498b086c0dab457a14 Signed-off-by: Daniel R. Carvalho Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24529 Reviewed-by: Nikos Nikoleris Maintainer: Nikos Nikoleris Tested-by: kokoro --- .../prefetch/access_map_pattern_matching.hh | 13 +++++++---- src/mem/cache/prefetch/associative_set.hh | 23 +++++++++---------- .../cache/prefetch/associative_set_impl.hh | 10 +++++++- .../delta_correlating_prediction_tables.cc | 4 +++- .../delta_correlating_prediction_tables.hh | 10 ++++---- src/mem/cache/prefetch/indirect_memory.cc | 4 ++-- src/mem/cache/prefetch/indirect_memory.hh | 17 ++++++++++---- .../cache/prefetch/irregular_stream_buffer.hh | 13 +++++++---- src/mem/cache/prefetch/signature_path.hh | 10 +++++--- .../spatio_temporal_memory_streaming.cc | 2 +- .../spatio_temporal_memory_streaming.hh | 12 ++++++---- 11 files changed, 75 insertions(+), 43 deletions(-) diff --git a/src/mem/cache/prefetch/access_map_pattern_matching.hh b/src/mem/cache/prefetch/access_map_pattern_matching.hh index 397bc788a9..6e2e194c7c 100644 --- a/src/mem/cache/prefetch/access_map_pattern_matching.hh +++ b/src/mem/cache/prefetch/access_map_pattern_matching.hh @@ -89,12 +89,15 @@ class AccessMapPatternMatching : public ClockedObject /** vector containing the state of the cachelines in this zone */ std::vector states; - AccessMapEntry(size_t num_entries) : states(num_entries, AM_INIT) - {} - - /** Reset the entries to their initial values */ - void reset() override + AccessMapEntry(size_t num_entries) + : TaggedEntry(), states(num_entries, AM_INIT) { + } + + void + invalidate() override + { + TaggedEntry::invalidate(); for (auto &entry : states) { entry = AM_INIT; } diff --git a/src/mem/cache/prefetch/associative_set.hh b/src/mem/cache/prefetch/associative_set.hh index e4e1b04282..6228fe4da2 100644 --- a/src/mem/cache/prefetch/associative_set.hh +++ b/src/mem/cache/prefetch/associative_set.hh @@ -66,10 +66,10 @@ class TaggedEntry : public ReplaceableEntry { valid = true; } - /** - * Sets the entry to invalid - */ - void setInvalid() { + /** Invalidates the entry. */ + virtual void + invalidate() + { valid = false; } @@ -108,14 +108,6 @@ class TaggedEntry : public ReplaceableEntry { { secure = s; } - - /** - * Resets the entry, this is called when an entry is evicted to allocate - * a new one. Types inheriting this class should provide its own - * implementation - */ - virtual void reset () { - } }; /** @@ -196,6 +188,13 @@ class AssociativeSet { */ void insertEntry(Addr addr, bool is_secure, Entry* entry); + /** + * Invalidate an entry and its respective replacement data. + * + * @param entry Entry to be invalidated. + */ + void invalidate(Entry* entry); + /** Iterator types */ using const_iterator = typename std::vector::const_iterator; using iterator = typename std::vector::iterator; diff --git a/src/mem/cache/prefetch/associative_set_impl.hh b/src/mem/cache/prefetch/associative_set_impl.hh index cc547e2dcd..8acae62690 100644 --- a/src/mem/cache/prefetch/associative_set_impl.hh +++ b/src/mem/cache/prefetch/associative_set_impl.hh @@ -87,7 +87,7 @@ AssociativeSet::findVictim(Addr addr) Entry* victim = static_cast(replacementPolicy->getVictim( selected_entries)); // There is only one eviction for this replacement - victim->reset(); + invalidate(victim); return victim; } @@ -117,4 +117,12 @@ AssociativeSet::insertEntry(Addr addr, bool is_secure, Entry* entry) replacementPolicy->reset(entry->replacementData); } +template +void +AssociativeSet::invalidate(Entry* entry) +{ + entry->invalidate(); + replacementPolicy->invalidate(entry->replacementData); +} + #endif//__CACHE_PREFETCH_ASSOCIATIVE_SET_IMPL_HH__ diff --git a/src/mem/cache/prefetch/delta_correlating_prediction_tables.cc b/src/mem/cache/prefetch/delta_correlating_prediction_tables.cc index 4dbd596a2b..8f7041f2b9 100644 --- a/src/mem/cache/prefetch/delta_correlating_prediction_tables.cc +++ b/src/mem/cache/prefetch/delta_correlating_prediction_tables.cc @@ -44,8 +44,10 @@ DeltaCorrelatingPredictionTables::DeltaCorrelatingPredictionTables( } void -DeltaCorrelatingPredictionTables::DCPTEntry::reset() +DeltaCorrelatingPredictionTables::DCPTEntry::invalidate() { + TaggedEntry::invalidate(); + for (auto &delta : deltas) { delta = 0; } diff --git a/src/mem/cache/prefetch/delta_correlating_prediction_tables.hh b/src/mem/cache/prefetch/delta_correlating_prediction_tables.hh index 86cf9574cc..e4c466c754 100644 --- a/src/mem/cache/prefetch/delta_correlating_prediction_tables.hh +++ b/src/mem/cache/prefetch/delta_correlating_prediction_tables.hh @@ -75,12 +75,12 @@ class DeltaCorrelatingPredictionTables : public SimObject * Constructor * @param num_deltas number of deltas stored in the entry */ - DCPTEntry(unsigned int num_deltas) : lastAddress(0), deltaPointer(0), - deltas(num_deltas) - {} + DCPTEntry(unsigned int num_deltas) + : TaggedEntry(), lastAddress(0), deltaPointer(0), deltas(num_deltas) + { + } - /** Reset callback called when invalidating the entry */ - void reset() override; + void invalidate() override; /** * Adds an address to the entry, if the entry already existed, a delta diff --git a/src/mem/cache/prefetch/indirect_memory.cc b/src/mem/cache/prefetch/indirect_memory.cc index 703105166e..0fd72cc62a 100644 --- a/src/mem/cache/prefetch/indirect_memory.cc +++ b/src/mem/cache/prefetch/indirect_memory.cc @@ -183,7 +183,7 @@ IndirectMemoryPrefetcher::allocateOrUpdateIPDEntry( } else { // Third access! no pattern has been found so far, // release the IPD entry - ipd_entry->reset(); + ipd.invalidate(ipd_entry); ipdEntryTrackingMisses = nullptr; } } else { @@ -237,7 +237,7 @@ IndirectMemoryPrefetcher::trackMissIndex2(Addr miss_addr) pt_entry->enabled = true; pt_entry->indirectCounter.reset(); // Release the current IPD Entry - entry->reset(); + ipd.invalidate(entry); // Do not track more misses ipdEntryTrackingMisses = nullptr; return; diff --git a/src/mem/cache/prefetch/indirect_memory.hh b/src/mem/cache/prefetch/indirect_memory.hh index f177c5c06f..07f3cb5cae 100644 --- a/src/mem/cache/prefetch/indirect_memory.hh +++ b/src/mem/cache/prefetch/indirect_memory.hh @@ -101,7 +101,10 @@ class IndirectMemoryPrefetcher : public QueuedPrefetcher increasedIndirectCounter(false) {} - void reset() override { + void + invalidate() override + { + TaggedEntry::invalidate(); address = 0; secure = false; streamCounter = 0; @@ -136,16 +139,20 @@ class IndirectMemoryPrefetcher : public QueuedPrefetcher IndirectPatternDetectorEntry(unsigned int num_addresses, unsigned int num_shifts) - : idx1(0), idx2(0), secondIndexSet(false), numMisses(0), + : TaggedEntry(), idx1(0), idx2(0), secondIndexSet(false), + numMisses(0), baseAddr(num_addresses, std::vector(num_shifts)) - {} + { + } - void reset() override { + void + invalidate() override + { + TaggedEntry::invalidate(); idx1 = 0; idx2 = 0; secondIndexSet = false; numMisses = 0; - setInvalid(); } }; /** Indirect Pattern Detector (IPD) table */ diff --git a/src/mem/cache/prefetch/irregular_stream_buffer.hh b/src/mem/cache/prefetch/irregular_stream_buffer.hh index c97fde84db..fefebc5d29 100644 --- a/src/mem/cache/prefetch/irregular_stream_buffer.hh +++ b/src/mem/cache/prefetch/irregular_stream_buffer.hh @@ -79,13 +79,18 @@ class IrregularStreamBufferPrefetcher : public QueuedPrefetcher * Maps a set of contiguous addresses to another set of (not necessarily * contiguos) addresses, with their corresponding confidence counters */ - struct AddressMappingEntry : public TaggedEntry { + struct AddressMappingEntry : public TaggedEntry + { std::vector mappings; AddressMappingEntry(size_t num_mappings, unsigned counter_bits) - : mappings(num_mappings, counter_bits) - {} - void reset() override + : TaggedEntry(), mappings(num_mappings, counter_bits) { + } + + void + invalidate() override + { + TaggedEntry::invalidate(); for (auto &entry : mappings) { entry.address = 0; entry.counter.reset(); diff --git a/src/mem/cache/prefetch/signature_path.hh b/src/mem/cache/prefetch/signature_path.hh index 3bf4dd2931..d2464e27a2 100644 --- a/src/mem/cache/prefetch/signature_path.hh +++ b/src/mem/cache/prefetch/signature_path.hh @@ -99,12 +99,16 @@ class SignaturePathPrefetcher : public QueuedPrefetcher /** use counter, used by SPPv2 */ SatCounter counter; PatternEntry(size_t num_strides, unsigned counter_bits) - : strideEntries(num_strides, counter_bits), counter(counter_bits) - {} + : TaggedEntry(), strideEntries(num_strides, counter_bits), + counter(counter_bits) + { + } /** Reset the entries to their initial values */ - void reset() override + void + invalidate() override { + TaggedEntry::invalidate(); for (auto &entry : strideEntries) { entry.counter.reset(); entry.stride = 0; diff --git a/src/mem/cache/prefetch/spatio_temporal_memory_streaming.cc b/src/mem/cache/prefetch/spatio_temporal_memory_streaming.cc index cf6144a64f..2bcd2feea7 100644 --- a/src/mem/cache/prefetch/spatio_temporal_memory_streaming.cc +++ b/src/mem/cache/prefetch/spatio_temporal_memory_streaming.cc @@ -96,7 +96,7 @@ STeMSPrefetcher::checkForActiveGenerationsEnd() { // this also sets the values of the entry pst_entry->update(agt_entry); // Free the AGT entry - agt_entry.setInvalid(); + activeGenerationTable.invalidate(&agt_entry); } } } diff --git a/src/mem/cache/prefetch/spatio_temporal_memory_streaming.hh b/src/mem/cache/prefetch/spatio_temporal_memory_streaming.hh index 34cf5d12a1..725fa38dbf 100644 --- a/src/mem/cache/prefetch/spatio_temporal_memory_streaming.hh +++ b/src/mem/cache/prefetch/spatio_temporal_memory_streaming.hh @@ -86,12 +86,16 @@ class STeMSPrefetcher : public QueuedPrefetcher /** Sequence of accesses */ std::vector sequence; - ActiveGenerationTableEntry(int num_positions) : paddress(0), pc(0), + ActiveGenerationTableEntry(int num_positions) + : TaggedEntry(), paddress(0), pc(0), seqCounter(0), sequence(num_positions) - {} - - void reset() override { + } + + void + invalidate() override + { + TaggedEntry::invalidate(); paddress = 0; pc = 0; seqCounter = 0;