mem-cache: stream add hysteresis#692
mem-cache: stream add hysteresis#692Maxpicca-Li wants to merge 1 commit intoOpenXiangShan:xs-devfrom
Conversation
📝 WalkthroughWalkthroughThe STREAM cache prefetcher now implements a hysteresis-based replacement policy. On cache hits, a hysteresis flag is set on the entry. On misses, the victim replacement logic conditionally either reinserts the entry with its previous state (if hysteresis is set) or initializes a new mapping, with the function returning Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[Generated by GEM5 Performance Robot] Standard PerformanceOverall Score
|
93844f9 to
ef72cc0
Compare
Change-Id: I710e7645a78dc48cf890e9223b76d92bc0a43d2a
|
[Generated by GEM5 Performance Robot] Standard PerformanceOverall Score
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/mem/cache/prefetch/xs_stream.cc (1)
112-112: Redundant assignment in else branch.Line 112 sets
entry->hysteresis = false;, but this is redundant when the else branch (line 106) is taken because the conditionif (entry->hysteresis)at line 103 already failed, meaninghysteresiswasfalse. While this defensive assignment doesn't hurt, it's unnecessary.Optional cleanup
entry->tag = regionHashTag(vaddr_tag_num); entry->decrMode = decr; entry->bitVec = 1UL << vaddr_offset; entry->cnt = 1; entry->active = in_active_page; - entry->hysteresis = false; stream_array.insertEntry(regionHashTag(vaddr_tag_num), secure, entry);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mem/cache/prefetch/xs_stream.ccsrc/mem/cache/prefetch/xs_stream.hh
🧰 Additional context used
🧬 Code graph analysis (1)
src/mem/cache/prefetch/xs_stream.cc (1)
src/mem/cache/prefetch/base.hh (1)
secure(161-164)
🔇 Additional comments (3)
src/mem/cache/prefetch/xs_stream.hh (1)
78-78: LGTM: Clean field addition.The
hysteresisfield is properly initialized with an in-class default and follows the same pattern as other boolean fields in the struct.src/mem/cache/prefetch/xs_stream.cc (2)
96-96: LGTM: Hysteresis flag set on hit.Setting the hysteresis flag on cache hits correctly marks the entry for deferred replacement, implementing a "second chance" eviction policy.
105-105: BothinsertEntrycalls are correct — no API signature mismatch.The
AssociativeSet::insertEntrymethod has a single signature with an optional fourth parameter:void insertEntry(Addr addr, bool is_secure, Entry* entry, bool with_reset = true);
- Line 105 explicitly passes
falseto skip replacement policy reset during reinsertion of an existing entry with hysteresis flag cleared.- Line 113 uses the default
truefor a fresh entry insertion with full initialization.Both usages are semantically correct and follow the intended API.
Change-Id: I710e7645a78dc48cf890e9223b76d92bc0a43d2a
Config: kmhv3, 0.8 coverage
Result: a little improvement, especially GemsFDTD and zeusmp
Data before rebasing
xs-dev:Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.