From e0de180ee4eb3316d2dd0d20320f2a1b310101bc Mon Sep 17 00:00:00 2001 From: Nikos Nikoleris Date: Tue, 25 Jun 2019 13:55:42 +0100 Subject: [PATCH] mem-cache: Fix MSHR whole line write tracking The MSHR keeps track of outstanding writes and services them as a whole line write whenever possible. To do this the outstanding writes have to be compatible (e.g., not strictly ordered). Prior to this change, due to this tracking mechanism, the MSHR would not service a WriteLineReq with flags that do not allow merging as a full line write even if it was the first target triggering an assertion. This changeset fixes this bug. Change-Id: I2cbf5ece0c108c1fcfe6855e8f194408d5ab8ce2 Signed-off-by: Nikos Nikoleris Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/22126 Reviewed-by: Daniel Carvalho Tested-by: kokoro --- src/mem/cache/mshr.hh | 75 ++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh index 3e7b79ea0..40eb970c0 100644 --- a/src/mem/cache/mshr.hh +++ b/src/mem/cache/mshr.hh @@ -203,7 +203,7 @@ class MSHR : public QueueEntry, public Printable } void resetFlags() { - onlyWrites = true; + canMergeWrites = true; std::fill(writesBitmap.begin(), writesBitmap.end(), false); needsWritable = false; @@ -227,27 +227,37 @@ class MSHR : public QueueEntry, public Printable * * @param pkt Packet considered for adding */ - void updateWriteFlags(PacketPtr pkt) { - const Request::FlagsType noMergeFlags = - Request::UNCACHEABLE | - Request::STRICT_ORDER | Request::MMAPPED_IPR | - Request::PRIVILEGED | Request::LLSC | - Request::MEM_SWAP | Request::MEM_SWAP_COND | - Request::SECURE; - - // if we have already seen writes for the full block stop - // here, this might be a full line write followed by - // other compatible requests (e.g., reads) - if (!isWholeLineWrite()) { - bool can_merge_write = pkt->isWrite() && - ((pkt->req->getFlags() & noMergeFlags) == 0); - onlyWrites &= can_merge_write; - if (onlyWrites) { - auto offset = pkt->getOffset(blkSize); - auto begin = writesBitmap.begin() + offset; - std::fill(begin, begin + pkt->getSize(), true); - } - } + void + updateWriteFlags(PacketPtr pkt) + { + // if we have already seen writes for the full block stop + // here, this might be a full line write followed by + // other compatible requests (e.g., reads) + if (!isWholeLineWrite()) { + // Avoid merging requests with special flags (e.g., + // strictly ordered) + const Request::FlagsType no_merge_flags = + Request::UNCACHEABLE | Request::STRICT_ORDER | + Request::MMAPPED_IPR | Request::PRIVILEGED | + Request::LLSC | Request::MEM_SWAP | + Request::MEM_SWAP_COND | Request::SECURE; + const auto &req_flags = pkt->req->getFlags(); + bool compat_write = pkt->isWrite() && + !req_flags.isSet(no_merge_flags); + canMergeWrites &= compat_write; + + // if this request is the first target in this list + // and additionally a whole-line write, we need to + // service it as a whole-line even if we won't allow + // any further merging (e.g., SECURE whole line + // write). + bool first_write = pkt->isWrite() && (size() == 0); + if (first_write || compat_write) { + auto offset = pkt->getOffset(blkSize); + auto begin = writesBitmap.begin() + offset; + std::fill(begin, begin + pkt->getSize(), true); + } + } } /** @@ -258,7 +268,7 @@ class MSHR : public QueueEntry, public Printable */ bool isReset() const { return !needsWritable && !hasUpgrade && !allocOnFill && - !hasFromCache && onlyWrites; + !hasFromCache && canMergeWrites; } /** @@ -289,17 +299,16 @@ class MSHR : public QueueEntry, public Printable const std::string &prefix) const; /** - * Check if this list contains only compatible writes, and if they - * span the entire cache line. This is used as part of the - * miss-packet creation. Note that new requests may arrive after a - * miss-packet has been created, and for the fill we therefore use - * the wasWholeLineWrite field. + * Check if this list contains writes that cover an entire + * cache line. This is used as part of the miss-packet + * creation. Note that new requests may arrive after a + * miss-packet has been created, and for the corresponding + * fill we use the wasWholeLineWrite field. */ bool isWholeLineWrite() const { - return onlyWrites && - std::all_of(writesBitmap.begin(), - writesBitmap.end(), [](bool i) { return i; }); + return std::all_of(writesBitmap.begin(), writesBitmap.end(), + [](bool i) { return i; }); } private: @@ -309,8 +318,8 @@ class MSHR : public QueueEntry, public Printable /** Size of the cache block. */ Addr blkSize; - /** Are we only dealing with writes. */ - bool onlyWrites; + /** Indicates whether we can merge incoming write requests */ + bool canMergeWrites; // NOTE: std::vector might not meet satisfy the // ForwardIterator requirement and therefore cannot be used -- 2.30.2