changeset dcb908e40547 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=dcb908e40547
description:
mem: Cleanup Packet::checkFunctional and hasData usage
This patch cleans up the use of hasData and checkFunctional in the
packet. The hasData function is unfortunately suggesting that it
checks if the packet has a valid data pointer, when it does in fact
only check if the specific packet type is specified to have a data
payload. The confusion led to a bug in checkFunctional. The latter
function is also tidied up to avoid name overloading.
diffstat:
src/mem/cache/cache_impl.hh | 8 ++++++-
src/mem/packet.cc | 20 +++++++++++-------
src/mem/packet.hh | 47 +++++++++++++++++++++++++++++---------------
3 files changed, 50 insertions(+), 25 deletions(-)
diffs (166 lines):
diff -r ffd46545b284 -r dcb908e40547 src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh Tue Dec 02 06:07:50 2014 -0500
+++ b/src/mem/cache/cache_impl.hh Tue Dec 02 06:07:52 2014 -0500
@@ -1481,6 +1481,10 @@
if (blk == NULL) {
// better have read new data...
assert(pkt->hasData());
+
+ // only read reaponses have data
+ assert(pkt->isRead());
+
// need to do a replacement
blk = allocateBlock(addr, is_secure, writebacks);
if (blk == NULL) {
@@ -1538,8 +1542,10 @@
DPRINTF(Cache, "Block addr %x (%s) moving from state %x to %s\n",
addr, is_secure ? "s" : "ns", old_state, blk->print());
- // if we got new data, copy it in
+ // if we got new data, copy it in (checking for a read response
+ // and a response that has data is the same in the end)
if (pkt->isRead()) {
+ assert(pkt->hasData());
std::memcpy(blk->data, pkt->getConstPtr<uint8_t>(), blkSize);
}
diff -r ffd46545b284 -r dcb908e40547 src/mem/packet.cc
--- a/src/mem/packet.cc Tue Dec 02 06:07:50 2014 -0500
+++ b/src/mem/packet.cc Tue Dec 02 06:07:52 2014 -0500
@@ -174,7 +174,7 @@
bool
Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size,
- uint8_t *data)
+ uint8_t *_data)
{
Addr func_start = getAddr();
Addr func_end = getAddr() + getSize() - 1;
@@ -189,12 +189,14 @@
// check print first since it doesn't require data
if (isPrint()) {
+ assert(!_data);
safe_cast<PrintReqState*>(senderState)->printObj(obj);
return false;
}
- // if there's no data, there's no need to look further
- if (!data) {
+ // we allow the caller to pass NULL to signify the other packet
+ // has no data
+ if (!_data) {
return false;
}
@@ -204,7 +206,9 @@
if (isRead()) {
if (func_start >= val_start && func_end <= val_end) {
- memcpy(getPtr<uint8_t>(), data + offset, getSize());
+ memcpy(getPtr<uint8_t>(), _data + offset, getSize());
+ // complete overlap, and as the current packet is a read
+ // we are done
return true;
} else {
// Offsets and sizes to copy in case of partial overlap
@@ -271,7 +275,7 @@
// copy the first half
uint8_t *dest = getPtr<uint8_t>() + func_offset;
- uint8_t *src = data + val_offset;
+ uint8_t *src = _data + val_offset;
memcpy(dest, src, (bytesValidStart - func_offset));
// re-calc the offsets and indices to do the copy
@@ -293,7 +297,7 @@
// copy partial data into the packet's data array
uint8_t *dest = getPtr<uint8_t>() + func_offset;
- uint8_t *src = data + val_offset;
+ uint8_t *src = _data + val_offset;
memcpy(dest, src, overlap_size);
// check if we're done filling the functional access
@@ -302,11 +306,11 @@
}
} else if (isWrite()) {
if (offset >= 0) {
- memcpy(data + offset, getConstPtr<uint8_t>(),
+ memcpy(_data + offset, getConstPtr<uint8_t>(),
(min(func_end, val_end) - func_start) + 1);
} else {
// val_start > func_start
- memcpy(data, getConstPtr<uint8_t>() - offset,
+ memcpy(_data, getConstPtr<uint8_t>() - offset,
(min(func_end, val_end) - val_start) + 1);
}
} else {
diff -r ffd46545b284 -r dcb908e40547 src/mem/packet.hh
--- a/src/mem/packet.hh Tue Dec 02 06:07:50 2014 -0500
+++ b/src/mem/packet.hh Tue Dec 02 06:07:52 2014 -0500
@@ -185,6 +185,12 @@
bool needsExclusive() const { return testCmdAttrib(NeedsExclusive); }
bool needsResponse() const { return testCmdAttrib(NeedsResponse); }
bool isInvalidate() const { return testCmdAttrib(IsInvalidate); }
+
+ /**
+ * Check if this particular packet type carries payload data. Note
+ * that this does not reflect if the data pointer of the packet is
+ * valid or not.
+ */
bool hasData() const { return testCmdAttrib(HasData); }
bool isLLSC() const { return testCmdAttrib(IsLlsc); }
bool isSWPrefetch() const { return testCmdAttrib(IsSWPrefetch); }
@@ -918,26 +924,35 @@
}
/**
+ * Check a functional request against a memory value stored in
+ * another packet (i.e. an in-transit request or
+ * response). Returns true if the current packet is a read, and
+ * the other packet provides the data, which is then copied to the
+ * current packet. If the current packet is a write, and the other
+ * packet intersects this one, then we update the data
+ * accordingly.
+ */
+ bool
+ checkFunctional(PacketPtr other)
+ {
+ // all packets that are carrying a payload should have a valid
+ // data pointer
+ return checkFunctional(other, other->getAddr(), other->isSecure(),
+ other->getSize(),
+ other->hasData() ?
+ other->getPtr<uint8_t>() : NULL);
+ }
+
+ /**
* Check a functional request against a memory value represented
- * by a base/size pair and an associated data array. If the
- * functional request is a read, it may be satisfied by the memory
- * value. If the functional request is a write, it may update the
+ * by a base/size pair and an associated data array. If the
+ * current packet is a read, it may be satisfied by the memory
+ * value. If the current packet is a write, it may update the
* memory value.
*/
- bool checkFunctional(Printable *obj, Addr base, bool is_secure, int size,
- uint8_t *data);
-
- /**
- * Check a functional request against a memory value stored in
- * another packet (i.e. an in-transit request or response).
- */
bool
- checkFunctional(PacketPtr other)
- {
- uint8_t *data = other->hasData() ? other->getPtr<uint8_t>() : NULL;
- return checkFunctional(other, other->getAddr(), other->isSecure(),
- other->getSize(), data);
- }
+ checkFunctional(Printable *obj, Addr base, bool is_secure, int size,
+ uint8_t *_data);
/**
* Push label for PrintReq (safe to call unconditionally).
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
