changeset b99fdc295c34 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b99fdc295c34
description:
mem: Remove null-check bypassing in Packet::getPtr
This patch removes the parameter that enables bypassing the null check
in the Packet::getPtr method. A number of call sites assume the value
to be non-null.
The one odd case is the RubyTester, which issues zero-sized
prefetches(!), and despite being reads they had no valid data
pointer. This is now fixed, but the size oddity remains (unless anyone
object or has any good suggestions).
Finally, in the Ruby Sequencer, appropriate checks are made for flush
packets as they have no valid data pointer.
diffstat:
src/cpu/testers/rubytest/Check.cc | 5 +++++
src/mem/packet.hh | 4 ++--
src/mem/ruby/slicc_interface/RubyRequest.cc | 2 +-
src/mem/ruby/slicc_interface/RubySlicc_Util.hh | 4 ++--
src/mem/ruby/system/DMASequencer.cc | 2 +-
src/mem/ruby/system/Sequencer.cc | 20 +++++++++-----------
6 files changed, 20 insertions(+), 17 deletions(-)
diffs (126 lines):
diff -r e1a853349529 -r b99fdc295c34 src/cpu/testers/rubytest/Check.cc
--- a/src/cpu/testers/rubytest/Check.cc Tue Dec 02 06:07:32 2014 -0500
+++ b/src/cpu/testers/rubytest/Check.cc Tue Dec 02 06:07:34 2014 -0500
@@ -110,6 +110,11 @@
req->setThreadContext(index, 0);
PacketPtr pkt = new Packet(req, cmd);
+ // despite the oddity of the 0 size (questionable if this should
+ // even be allowed), a prefetch is still a read and as such needs
+ // a place to store the result
+ uint8_t *data = new uint8_t;
+ pkt->dataDynamic(data);
// push the subblock onto the sender state. The sequencer will
// update the subblock on the return
diff -r e1a853349529 -r b99fdc295c34 src/mem/packet.hh
--- a/src/mem/packet.hh Tue Dec 02 06:07:32 2014 -0500
+++ b/src/mem/packet.hh Tue Dec 02 06:07:34 2014 -0500
@@ -846,9 +846,9 @@
*/
template <typename T>
T*
- getPtr(bool null_ok = false)
+ getPtr()
{
- assert(null_ok || flags.isSet(STATIC_DATA|DYNAMIC_DATA));
+ assert(flags.isSet(STATIC_DATA|DYNAMIC_DATA));
return (T*)data;
}
diff -r e1a853349529 -r b99fdc295c34 src/mem/ruby/slicc_interface/RubyRequest.cc
--- a/src/mem/ruby/slicc_interface/RubyRequest.cc Tue Dec 02 06:07:32
2014 -0500
+++ b/src/mem/ruby/slicc_interface/RubyRequest.cc Tue Dec 02 06:07:34
2014 -0500
@@ -72,7 +72,7 @@
Addr mBase = m_PhysicalAddress.getAddress();
Addr mTail = mBase + m_Size;
- uint8_t * pktData = pkt->getPtr<uint8_t>(true);
+ uint8_t * pktData = pkt->getPtr<uint8_t>();
Addr cBase = std::max(wBase, mBase);
Addr cTail = std::min(wTail, mTail);
diff -r e1a853349529 -r b99fdc295c34
src/mem/ruby/slicc_interface/RubySlicc_Util.hh
--- a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh Tue Dec 02 06:07:32
2014 -0500
+++ b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh Tue Dec 02 06:07:34
2014 -0500
@@ -107,7 +107,7 @@
lineAddr.makeLineAddress();
if (pktLineAddr == lineAddr) {
- uint8_t *data = pkt->getPtr<uint8_t>(true);
+ uint8_t *data = pkt->getPtr<uint8_t>();
unsigned int size_in_bytes = pkt->getSize();
unsigned startByte = pkt->getAddr() - lineAddr.getAddress();
@@ -135,7 +135,7 @@
lineAddr.makeLineAddress();
if (pktLineAddr == lineAddr) {
- uint8_t *data = pkt->getPtr<uint8_t>(true);
+ uint8_t *data = pkt->getPtr<uint8_t>();
unsigned int size_in_bytes = pkt->getSize();
unsigned startByte = pkt->getAddr() - lineAddr.getAddress();
diff -r e1a853349529 -r b99fdc295c34 src/mem/ruby/system/DMASequencer.cc
--- a/src/mem/ruby/system/DMASequencer.cc Tue Dec 02 06:07:32 2014 -0500
+++ b/src/mem/ruby/system/DMASequencer.cc Tue Dec 02 06:07:34 2014 -0500
@@ -235,7 +235,7 @@
}
uint64_t paddr = pkt->getAddr();
- uint8_t* data = pkt->getPtr<uint8_t>(true);
+ uint8_t* data = pkt->getPtr<uint8_t>();
int len = pkt->getSize();
bool write = pkt->isWrite();
diff -r e1a853349529 -r b99fdc295c34 src/mem/ruby/system/Sequencer.cc
--- a/src/mem/ruby/system/Sequencer.cc Tue Dec 02 06:07:32 2014 -0500
+++ b/src/mem/ruby/system/Sequencer.cc Tue Dec 02 06:07:34 2014 -0500
@@ -524,28 +524,23 @@
llscSuccess ? "Done" : "SC_Failed", "", "",
request_address, total_latency);
- // update the data
+ // update the data unless it is a non-data-carrying flush
if (g_system_ptr->m_warmup_enabled) {
- assert(pkt->getPtr<uint8_t>(false) != NULL);
- data.setData(pkt->getPtr<uint8_t>(false),
+ data.setData(pkt->getPtr<uint8_t>(),
request_address.getOffset(), pkt->getSize());
- } else if (pkt->getPtr<uint8_t>(true) != NULL) {
+ } else if (!pkt->isFlush()) {
if ((type == RubyRequestType_LD) ||
(type == RubyRequestType_IFETCH) ||
(type == RubyRequestType_RMW_Read) ||
(type == RubyRequestType_Locked_RMW_Read) ||
(type == RubyRequestType_Load_Linked)) {
- memcpy(pkt->getPtr<uint8_t>(true),
+ memcpy(pkt->getPtr<uint8_t>(),
data.getData(request_address.getOffset(), pkt->getSize()),
pkt->getSize());
} else {
- data.setData(pkt->getPtr<uint8_t>(true),
+ data.setData(pkt->getPtr<uint8_t>(),
request_address.getOffset(), pkt->getSize());
}
- } else {
- DPRINTF(MemoryAccess,
- "WARNING. Data not transfered from Ruby to M5 for type %s\n",
- RubyRequestType_to_string(type));
}
// If using the RubyTester, update the RubyTester sender state's
@@ -679,9 +674,12 @@
pc = pkt->req->getPC();
}
+ // check if the packet has data as for example prefetch and flush
+ // requests do not
std::shared_ptr<RubyRequest> msg =
std::make_shared<RubyRequest>(clockEdge(), pkt->getAddr(),
- pkt->getPtr<uint8_t>(true),
+ pkt->isFlush() ?
+ nullptr : pkt->getPtr<uint8_t>(),
pkt->getSize(), pc, secondary_type,
RubyAccessMode_Supervisor, pkt,
PrefetchBit_No, proc_id);
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
