From a565c65e48772f31bb401a512cf8c810dffeb28e Mon Sep 17 00:00:00 2001 From: Jake Ke Date: Wed, 22 Jul 2026 13:24:50 -0700 Subject: [PATCH] memHierarchy: preserve same-address request order under MSHR pressure Cache::clockTick scans the per-cycle event buffer front-to-back, retrying any event it cannot accept. When an MSHR-full (or otherwise resource-blocked) request to a line is rejected mid-scan and a response later in the same scan frees the resource, a subsequent same-address request can be accepted ahead of the earlier one -- inverting two same-address requests issued by a single source. For back-to-back writes to the same line this silently drops an update: the older request lands last and overwrites the newer one, corrupting memory with no error raised. Fix: track addresses whose request was rejected this cycle in rejectedAddrsThisCycle_ and hold every later same-address request for the rest of that cycle's scan. On the next cycle the event buffer's FIFO order lets the earlier request claim the freed resource first. Responses are exempt so MSHR-draining events (GetXResp, WriteResp, AckPut, NACK, ...) always flow and forward progress is preserved. Observed under coherence_protocol=none with finite MSHRs, but the reorder is protocol-independent. Co-Authored-By: Claude Fable 5 --- .../elements/memHierarchy/cacheController.cc | 21 +++++++++++++++++++ .../elements/memHierarchy/cacheController.h | 1 + 2 files changed, 22 insertions(+) diff --git a/src/sst/elements/memHierarchy/cacheController.cc b/src/sst/elements/memHierarchy/cacheController.cc index 7841f588f3..2fb1943063 100644 --- a/src/sst/elements/memHierarchy/cacheController.cc +++ b/src/sst/elements/memHierarchy/cacheController.cc @@ -119,6 +119,7 @@ bool Cache::clockTick(Cycle_t time) { bankStatus_[bank] = false; addrsThisCycle_.clear(); + rejectedAddrsThisCycle_.clear(); // Handle events from each of the buffers // 1. Retry buffer -> Events that need to be retried, e.g., were stalled due to a pending action that is now resolved @@ -253,6 +254,22 @@ bool Cache::processEvent(MemEventBase* ev, bool retry) { Addr addr = event->getBaseAddr(); + /* Preserve program order among same-address requests within a cycle. + * The event buffer is scanned front-to-back each cycle; an event that + * cannot be accepted is skipped and retried, so a later event in the scan + * can be accepted ahead of an earlier one. That is harmless in general, + * but if an earlier request to a line was rejected (e.g. MSHR full) and a + * response frees the resource mid-scan, a later same-address request could + * be accepted out of order -- inverting two same-address requests from one + * source. For writes this silently drops an update. So: once any request + * to an address is rejected this cycle, hold all later same-address + * requests until the next cycle, where the buffer's FIFO order lets the + * earlier request claim the resource first. Responses are exempt so + * MSHR-draining events always flow and forward progress is preserved. */ + bool isRequest = CommandClassArr[(int)event->getCmd()] == CommandClass::Request; + if (isRequest && rejectedAddrsThisCycle_.find(addr) != rejectedAddrsThisCycle_.end()) + return false; + /* Arbitrate cache access - bank/link. Reject request on failure */ if (!arbitrateAccess(addr)) { // Disallow multiple requests to same line and/or bank in a single cycle if (mem_h_is_debug_addr(addr)) { @@ -262,6 +279,8 @@ bool Cache::processEvent(MemEventBase* ev, bool retry) { getCurrentSimCycle(), timestamp_, getName().c_str(), CommandString[(int)event->getCmd()], addr, id.str().c_str(), "", "", "Stall", "(bank busy)"); } + if (isRequest) + rejectedAddrsThisCycle_.insert(addr); return false; } @@ -369,6 +388,8 @@ bool Cache::processEvent(MemEventBase* ev, bool retry) { if (accepted) updateAccessStatus(addr); + else if (isRequest) + rejectedAddrsThisCycle_.insert(addr); // Preserve same-address order for the rest of this cycle's scan return accepted; } diff --git a/src/sst/elements/memHierarchy/cacheController.h b/src/sst/elements/memHierarchy/cacheController.h index 68a67e4c37..8bcdfa6b76 100644 --- a/src/sst/elements/memHierarchy/cacheController.h +++ b/src/sst/elements/memHierarchy/cacheController.h @@ -304,6 +304,7 @@ class Cache : public SST::Component { int requestsThisCycle_; std::vector bankStatus_; std::set addrsThisCycle_; + std::set rejectedAddrsThisCycle_; // Requests rejected this cycle; later same-address requests must also reject to preserve program order std::list retryBuffer_; std::list eventBuffer_; std::queue prefetchBuffer_;