From df833c27c1ec348c49c89e2ebb36d84aa956960c Mon Sep 17 00:00:00 2001 From: Ayaz Akram Date: Mon, 13 Feb 2023 12:52:31 -0800 Subject: [PATCH 1/5] cpu: Handle split memory requests --- .../testers/dr_trace_player/trace_player.cc | 77 ++++++++++++++++--- .../testers/dr_trace_player/trace_player.hh | 6 +- 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/cpu/testers/dr_trace_player/trace_player.cc b/src/cpu/testers/dr_trace_player/trace_player.cc index 8ab0856c8a..1de355fba9 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.cc +++ b/src/cpu/testers/dr_trace_player/trace_player.cc @@ -167,24 +167,53 @@ DRTracePlayer::executeMemInst(DRTraceReader::TraceRef &mem_ref) bool DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) { - PacketPtr pkt = getPacket(mem_ref); + PacketPtr pkt, split_pkt; + + std::tie(pkt, split_pkt) = getPacket(mem_ref); + + if (!retrySplitPkt) { + // we should not be sending the first pkt again if + // we are retrying only on the second part of the + // previously stalled request + DPRINTF(DRTrace, "Trying to send %s\n", pkt->print()); + + if (!port.sendTimingReq(pkt)) { + DPRINTF(DRTrace, "Failed to send pkt\n"); + if (stats.memStallStart == 0) { + stats.memStallStart = curTick(); + } + delete pkt; + if (!split_pkt) { + // if this is not a split pkt req, + // we can return back here + return true; + } + } else { + stats.latencyTracker[pkt] = curTick(); + return false; + } + } - DPRINTF(DRTrace, "Trying to send %s\n", pkt->print()); + DPRINTF(DRTrace, "Trying to send split %s\n", split_pkt->print()); - if (!port.sendTimingReq(pkt)) { - DPRINTF(DRTrace, "Failed to send pkt\n"); + if (!port.sendTimingReq(split_pkt)) { + DPRINTF(DRTrace, "Failed to send pkt (split pkt) \n"); if (stats.memStallStart == 0) { stats.memStallStart = curTick(); } - delete pkt; + retrySplitPkt = false; + delete split_pkt; return true; } else { - stats.latencyTracker[pkt] = curTick(); + stats.latencyTracker[split_pkt] = curTick(); + // also remember that we only need to retry on second part of + // the split pkt + retrySplitPkt = true; return false; } } -PacketPtr +std::pair DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) { Request::Flags flags = Request::PHYSICAL; @@ -198,11 +227,13 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) addr %= compressAddressRange.size(); } + bool split_req = false; unsigned size = mem_ref.size; Addr split_addr = roundDown(addr + size - 1, cacheLineSize); if (split_addr > addr) { - warn("Ignoring split packet that crosses cache line boundary."); + DPRINTF(DRTrace, "Split pkt (crosses cache line boundary) created\n"); size = split_addr - addr; + split_req = true; } // Create new request @@ -230,9 +261,35 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) } } - return pkt; -} + PacketPtr split_pkt = NULL; + + // In case of split packets when we want to send two requests. + // For the second request, the starting address + // will be split_addr and the size will be cacheLineSize - size + + if (split_req) { + + // Create the split request + RequestPtr split_req = std::make_shared(split_addr, + cacheLineSize - size, flags, requestorId); + split_req->setPC(curPC); + // Embed it in a packet + split_pkt = new Packet(split_req, cmd); + + if (params().send_data) { + uint8_t* split_pkt_data = new uint8_t[split_req->getSize()]; + split_pkt->dataDynamic(split_pkt_data); + + if (cmd.isWrite()) { + std::fill_n(split_pkt_data, split_req->getSize(), + (uint8_t)requestorId); + } + } + } + + return std::make_pair(pkt, split_pkt); +} Port & DRTracePlayer::getPort(const std::string &if_name, PortID idx) diff --git a/src/cpu/testers/dr_trace_player/trace_player.hh b/src/cpu/testers/dr_trace_player/trace_player.hh index 9ba38a4a0d..9a2598db72 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.hh +++ b/src/cpu/testers/dr_trace_player/trace_player.hh @@ -71,6 +71,9 @@ class DRTracePlayer : public ClockedObject AddrRange compressAddressRange; int cacheLineSize; + // variable to keep track of retries for split pkts + bool retrySplitPkt = false; + // State bool stalled = false; Addr curPC = 0; @@ -129,7 +132,8 @@ class DRTracePlayer : public ClockedObject bool recvTimingResp(PacketPtr pkt); - PacketPtr getPacket(DRTraceReader::TraceRef &mem_ref); + std::pair + getPacket(DRTraceReader::TraceRef &mem_ref); /** * @brief Send a request to memory based on the trace From 41acdd477093fd2e9586f3e450e82ad4d5569802 Mon Sep 17 00:00:00 2001 From: Ayaz Akram Date: Fri, 24 Feb 2023 23:44:46 -0800 Subject: [PATCH 2/5] cpu: some fixes for split pkt handling --- .../testers/dr_trace_player/trace_player.cc | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/cpu/testers/dr_trace_player/trace_player.cc b/src/cpu/testers/dr_trace_player/trace_player.cc index 1de355fba9..78423cbd20 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.cc +++ b/src/cpu/testers/dr_trace_player/trace_player.cc @@ -155,8 +155,6 @@ DRTracePlayer::executeMemInst(DRTraceReader::TraceRef &mem_ref) } if (!trySendMemRef(mem_ref)) { - numOutstandingMemReqs++; - stats.outstandingMemReqs.sample(numOutstandingMemReqs); stats.numMemInsts++; return false; } else { @@ -182,8 +180,11 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) if (stats.memStallStart == 0) { stats.memStallStart = curTick(); } + numOutstandingMemReqs++; + stats.outstandingMemReqs.sample(numOutstandingMemReqs); + delete pkt; - if (!split_pkt) { + if (split_pkt == nullptr) { // if this is not a split pkt req, // we can return back here return true; @@ -201,14 +202,17 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) if (stats.memStallStart == 0) { stats.memStallStart = curTick(); } - retrySplitPkt = false; + numOutstandingMemReqs++; + stats.outstandingMemReqs.sample(numOutstandingMemReqs); + delete split_pkt; - return true; - } else { - stats.latencyTracker[split_pkt] = curTick(); // also remember that we only need to retry on second part of // the split pkt retrySplitPkt = true; + return true; + } else { + stats.latencyTracker[split_pkt] = curTick(); + retrySplitPkt = false; return false; } } @@ -261,7 +265,7 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) } } - PacketPtr split_pkt = NULL; + PacketPtr split_pkt = nullptr; // In case of split packets when we want to send two requests. // For the second request, the starting address From 10c25263f612e3470d0447f0028eb1712c17f927 Mon Sep 17 00:00:00 2001 From: Ayaz Akram Date: Sun, 26 Feb 2023 23:49:07 -0800 Subject: [PATCH 3/5] cpu: more fixes --- .../testers/dr_trace_player/trace_player.cc | 33 ++++++++++--------- .../testers/dr_trace_player/trace_player.hh | 2 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/cpu/testers/dr_trace_player/trace_player.cc b/src/cpu/testers/dr_trace_player/trace_player.cc index 78423cbd20..d5bcc0f471 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.cc +++ b/src/cpu/testers/dr_trace_player/trace_player.cc @@ -165,9 +165,7 @@ DRTracePlayer::executeMemInst(DRTraceReader::TraceRef &mem_ref) bool DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) { - PacketPtr pkt, split_pkt; - - std::tie(pkt, split_pkt) = getPacket(mem_ref); + auto [pkt, split_pkt] = getPacket(mem_ref); if (!retrySplitPkt) { // we should not be sending the first pkt again if @@ -180,18 +178,20 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) if (stats.memStallStart == 0) { stats.memStallStart = curTick(); } - numOutstandingMemReqs++; - stats.outstandingMemReqs.sample(numOutstandingMemReqs); - delete pkt; - if (split_pkt == nullptr) { - // if this is not a split pkt req, - // we can return back here + + // return true if we have to stall on the first pkt + // irrespective of if this is a split req return true; - } } else { + numOutstandingMemReqs++; + stats.outstandingMemReqs.sample(numOutstandingMemReqs); stats.latencyTracker[pkt] = curTick(); - return false; + if (split_pkt == nullptr) { + // if this is not a split req, we can + // return here + return false; + } } } @@ -202,22 +202,23 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) if (stats.memStallStart == 0) { stats.memStallStart = curTick(); } - numOutstandingMemReqs++; - stats.outstandingMemReqs.sample(numOutstandingMemReqs); - delete split_pkt; // also remember that we only need to retry on second part of // the split pkt retrySplitPkt = true; return true; } else { + numOutstandingMemReqs++; + stats.outstandingMemReqs.sample(numOutstandingMemReqs); stats.latencyTracker[split_pkt] = curTick(); retrySplitPkt = false; + // At this point, we are sure that both pkts of the split req + // are received by the port return false; } } -std::pair +std::tuple DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) { Request::Flags flags = Request::PHYSICAL; @@ -292,7 +293,7 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref) } } - return std::make_pair(pkt, split_pkt); + return {pkt, split_pkt}; } Port & diff --git a/src/cpu/testers/dr_trace_player/trace_player.hh b/src/cpu/testers/dr_trace_player/trace_player.hh index 9a2598db72..8f614054b6 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.hh +++ b/src/cpu/testers/dr_trace_player/trace_player.hh @@ -132,7 +132,7 @@ class DRTracePlayer : public ClockedObject bool recvTimingResp(PacketPtr pkt); - std::pair + std::tuple getPacket(DRTraceReader::TraceRef &mem_ref); /** From 98127914b417de013b67083ad422809bfb31d2e0 Mon Sep 17 00:00:00 2001 From: Ayaz Akram Date: Tue, 28 Feb 2023 02:31:25 -0800 Subject: [PATCH 4/5] cpu: delete pkt fix --- src/cpu/testers/dr_trace_player/trace_player.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cpu/testers/dr_trace_player/trace_player.cc b/src/cpu/testers/dr_trace_player/trace_player.cc index d5bcc0f471..8243fb09d1 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.cc +++ b/src/cpu/testers/dr_trace_player/trace_player.cc @@ -179,6 +179,7 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) stats.memStallStart = curTick(); } delete pkt; + delete split_pkt; // return true if we have to stall on the first pkt // irrespective of if this is a split req @@ -193,6 +194,11 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) return false; } } + } else { + // we should delete the first pkt here + // if we are only trying to resend the + // second pkt + delete pkt; } DPRINTF(DRTrace, "Trying to send split %s\n", split_pkt->print()); From b72ccb26b74f07fc1cf3d7d05b15cbd31b012078 Mon Sep 17 00:00:00 2001 From: Ayaz Akram Date: Tue, 28 Feb 2023 21:54:59 -0800 Subject: [PATCH 5/5] cpu: add/fix comments --- src/cpu/testers/dr_trace_player/trace_player.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cpu/testers/dr_trace_player/trace_player.cc b/src/cpu/testers/dr_trace_player/trace_player.cc index 8243fb09d1..4ee814f94f 100644 --- a/src/cpu/testers/dr_trace_player/trace_player.cc +++ b/src/cpu/testers/dr_trace_player/trace_player.cc @@ -165,12 +165,16 @@ DRTracePlayer::executeMemInst(DRTraceReader::TraceRef &mem_ref) bool DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) { + // split_pkt will be nullptr if not a split req auto [pkt, split_pkt] = getPacket(mem_ref); + // ensure that currently we are not in process of + // retrying to send the second part of a previously + // stalled request to avoid duplicate first pkt in + // the memory system. + // Also, the assumption is that we cannot start a + // new memory request in parallel. if (!retrySplitPkt) { - // we should not be sending the first pkt again if - // we are retrying only on the second part of the - // previously stalled request DPRINTF(DRTrace, "Trying to send %s\n", pkt->print()); if (!port.sendTimingReq(pkt)) { @@ -203,6 +207,9 @@ DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref) DPRINTF(DRTrace, "Trying to send split %s\n", split_pkt->print()); + // if the first pkt is sent out, and the current + // request is a split request, try to send out + // the second pkt if (!port.sendTimingReq(split_pkt)) { DPRINTF(DRTrace, "Failed to send pkt (split pkt) \n"); if (stats.memStallStart == 0) {