Kyle Roarty has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/33656 )
Change subject: gpu-compute,mem-ruby: WriteCompletePkts fixes
......................................................................
gpu-compute,mem-ruby: WriteCompletePkts fixes
There seems to be a flow of packets as so:
WriteResp -> WriteReq -> WriteCompleteResp
All of these packets share a senderState. The senderState was being
deleted in the writeResp packet. This patch fixes that, avoiding a
segfault
Additionally, the WriteCompleteResp packet was attempting to access
physical memory in hitCallback while not having any data, which
caused a crash. This can be resolved either by not allowing
WriteCompleteResp packets to access memory, or by copying the data
from the WriteReq packet. This patch denies WriteCompleteResp packets
memory access in hitCallback.
In VIPERCoalescer::writeCompleteCallback, there was a packet map, but
no packets were ever being removed. This patch removes packets that
match the address that was passed in to the function. There was an
error that occured if this change wasn't implemented, I forget what it
was at this point.
Now for the worst part of the change
in ComputeUnit::DataPort::recvTimingResp, when the packet is a
WriteCompleteResp packet, will call globalMemoryPipe.handleResponse.
That call happens when gpuDynInst->allLanesZero().
There's two issues with this.
1: The WriteResp packets (as mentioned above) share the same gpuDynInst
as the WriteCompleteResp packets. The WriteResp packets also decrement
the status vector in ComputeUnit::DataPort::processMemRespEvent. This
leads to issue 2
2: There are multiple WriteCompleteResp packets for one
gpuDynInst. Because the status vector is already decremented by the
WriteResp packets, the check for gpuDynInst->allLanesZero() returns
true for all those packets. This causes globalMemoryPipe.handleResponse
to be called multiple times, which causes a crash on an assert.
I simply replaced the assert with a return statement.
WIP
Change-Id: I9a064a0def2bf6c513f5295596c56b1b652b0ca4
---
M src/gpu-compute/compute_unit.cc
M src/gpu-compute/global_memory_pipeline.cc
M src/mem/ruby/system/RubyPort.cc
M src/mem/ruby/system/VIPERCoalescer.cc
4 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/src/gpu-compute/compute_unit.cc
b/src/gpu-compute/compute_unit.cc
index 920257d..2df3399 100644
--- a/src/gpu-compute/compute_unit.cc
+++ b/src/gpu-compute/compute_unit.cc
@@ -1376,7 +1376,10 @@
}
}
- delete pkt->senderState;
+ if (pkt->cmd != MemCmd::WriteResp) {
+ delete pkt->senderState;
+ }
+
delete pkt;
}
diff --git a/src/gpu-compute/global_memory_pipeline.cc
b/src/gpu-compute/global_memory_pipeline.cc
index 9fc515a..87bbe17 100644
--- a/src/gpu-compute/global_memory_pipeline.cc
+++ b/src/gpu-compute/global_memory_pipeline.cc
@@ -278,7 +278,9 @@
// if we are getting a response for this mem request,
// then it ought to already be in the ordered response
// buffer
- assert(mem_req != gmOrderedRespBuffer.end());
+ //assert(mem_req != gmOrderedRespBuffer.end());
+ if (mem_req == gmOrderedRespBuffer.end())
+ return;
mem_req->second.second = true;
}
diff --git a/src/mem/ruby/system/RubyPort.cc
b/src/mem/ruby/system/RubyPort.cc
index 4510e3a..574fab0 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -539,7 +539,8 @@
}
// Flush, acquire, release requests don't access physical memory
- if (pkt->isFlush() || pkt->cmd == MemCmd::MemSyncReq) {
+ if (pkt->isFlush() || pkt->cmd == MemCmd::MemSyncReq
+ || pkt->cmd == MemCmd::WriteCompleteResp) {
accessPhysMem = false;
}
diff --git a/src/mem/ruby/system/VIPERCoalescer.cc
b/src/mem/ruby/system/VIPERCoalescer.cc
index eafce6d..f6e7a4d 100644
--- a/src/mem/ruby/system/VIPERCoalescer.cc
+++ b/src/mem/ruby/system/VIPERCoalescer.cc
@@ -243,19 +243,28 @@
assert(m_writeCompletePktMap.count(key) == 1 &&
!m_writeCompletePktMap[key].empty());
- for (auto writeCompletePkt : m_writeCompletePktMap[key]) {
- if (makeLineAddress(writeCompletePkt->getAddr()) == addr) {
- RubyPort::SenderState *ss =
- safe_cast<RubyPort::SenderState *>
- (writeCompletePkt->senderState);
- MemSlavePort *port = ss->port;
- assert(port != NULL);
+ m_writeCompletePktMap[key].erase(
+ std::remove_if(
+ m_writeCompletePktMap[key].begin(),
+ m_writeCompletePktMap[key].end(),
+ [addr](PacketPtr writeCompletePkt) -> bool {
+ if (makeLineAddress(writeCompletePkt->getAddr()) == addr) {
+ RubyPort::SenderState *ss =
+ safe_cast<RubyPort::SenderState *>
+ (writeCompletePkt->senderState);
+ MemSlavePort *port = ss->port;
+ assert(port != NULL);
- writeCompletePkt->senderState = ss->predecessor;
- delete ss;
- port->hitCallback(writeCompletePkt);
- }
- }
+ writeCompletePkt->senderState = ss->predecessor;
+ delete ss;
+ port->hitCallback(writeCompletePkt);
+ return true;
+ }
+ return false;
+ }
+ ),
+ m_writeCompletePktMap[key].end()
+ );
trySendRetries();
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33656
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9a064a0def2bf6c513f5295596c56b1b652b0ca4
Gerrit-Change-Number: 33656
Gerrit-PatchSet: 1
Gerrit-Owner: Kyle Roarty <kyleroarty1...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
