changeset 1072b1381560 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=1072b1381560
description:
mem: Fix cache MSHR conflict determination
This patch fixes a rather subtle issue in the sending of MSHR requests
in the cache, where the logic previously did not check for conflicts
between the MSRH queue and the write queue when requests were not
ready. The correct thing to do is to always check, since not having a
ready MSHR does not guarantee that there is no conflict.
The underlying problem seems to have slipped past due to the symmetric
timings used for the write queue and MSHR queue. However, with the
recent timing changes the bug caused regressions to fail.
diffstat:
src/mem/cache/cache_impl.hh | 48 ++++++++++++++++++++------------------------
1 files changed, 22 insertions(+), 26 deletions(-)
diffs (72 lines):
diff -r b1d90d88420e -r 1072b1381560 src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh Mon Mar 02 04:00:52 2015 -0500
+++ b/src/mem/cache/cache_impl.hh Mon Mar 02 04:00:54 2015 -0500
@@ -1887,39 +1887,33 @@
MSHR *
Cache<TagStore>::getNextMSHR()
{
- // Check both MSHR queue and write buffer for potential requests
+ // Check both MSHR queue and write buffer for potential requests,
+ // note that null does not mean there is no request, it could
+ // simply be that it is not ready
MSHR *miss_mshr = mshrQueue.getNextMSHR();
MSHR *write_mshr = writeBuffer.getNextMSHR();
- // Now figure out which one to send... some cases are easy
- if (miss_mshr && !write_mshr) {
- return miss_mshr;
- }
- if (write_mshr && !miss_mshr) {
- return write_mshr;
- }
+ // If we got a write buffer request ready, first priority is a
+ // full write buffer, otherwhise we favour the miss requests
+ if (write_mshr &&
+ ((writeBuffer.isFull() && writeBuffer.inServiceEntries == 0) ||
+ !miss_mshr)) {
+ // need to search MSHR queue for conflicting earlier miss.
+ MSHR *conflict_mshr =
+ mshrQueue.findPending(write_mshr->addr, write_mshr->size,
+ write_mshr->isSecure);
- if (miss_mshr && write_mshr) {
- // We have one of each... normally we favor the miss request
- // unless the write buffer is full
- if (writeBuffer.isFull() && writeBuffer.inServiceEntries == 0) {
- // Write buffer is full, so we'd like to issue a write;
- // need to search MSHR queue for conflicting earlier miss.
- MSHR *conflict_mshr =
- mshrQueue.findPending(write_mshr->addr, write_mshr->size,
- write_mshr->isSecure);
+ if (conflict_mshr && conflict_mshr->order < write_mshr->order) {
+ // Service misses in order until conflict is cleared.
+ return conflict_mshr;
- if (conflict_mshr && conflict_mshr->order < write_mshr->order) {
- // Service misses in order until conflict is cleared.
- return conflict_mshr;
- }
-
- // No conflicts; issue write
- return write_mshr;
+ // @todo Note that we ignore the ready time of the conflict here
}
- // Write buffer isn't full, but need to check it for
- // conflicting earlier writeback
+ // No conflicts; issue write
+ return write_mshr;
+ } else if (miss_mshr) {
+ // need to check for conflicting earlier writeback
MSHR *conflict_mshr =
writeBuffer.findPending(miss_mshr->addr, miss_mshr->size,
miss_mshr->isSecure);
@@ -1937,6 +1931,8 @@
// have to flush writes in order? I don't think so... not
// for Alpha anyway. Maybe for x86?
return conflict_mshr;
+
+ // @todo Note that we ignore the ready time of the conflict here
}
// No conflicts; issue read
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
