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 | 11 +++++---- 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 | 10 +++++--- 11 files changed, 73 insertions(+), 41 deletions(-) diff --git a/src/mem/cache/prefetch/access_map_pattern_matching.hh b/src/mem/cache/prefetch/access_map_pattern_matching.hh index 397bc788a..6e2e194c7 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) - {} + AccessMapEntry(size_t num_entries) + : TaggedEntry(), states(num_entries, AM_INIT) + { + } - /** Reset the entries to their initial values */ - void reset() override + 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 e4e1b0428..6228fe4da 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 cc547e2dc..8acae6269 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 4dbd596a2..8f7041f2b 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 86cf9574c..e4c466c75 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 703105166..0fd72cc62 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 f177c5c06..07f3cb5ca 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 c97fde84d..fefebc5d2 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 3bf4dd293..d2464e27a 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 cf6144a64..2bcd2feea 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 34cf5d12a..725fa38db 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; -- 2.30.2