summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAndreas Hansson <andreas.hansson@arm.com>2015-03-02 04:00:54 -0500
committerAndreas Hansson <andreas.hansson@arm.com>2015-03-02 04:00:54 -0500
commit88e2963951860966dd850ef874e5fed99fe78b88 (patch)
tree5c8ebbe7b69ec49ebbd762fd84c5bbf73a864bb3 /src
parent407737614ed2431d75314eba813edfec40e95bcc (diff)
downloadgem5-88e2963951860966dd850ef874e5fed99fe78b88.tar.xz
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 (limited to 'src')
-rw-r--r--src/mem/cache/cache_impl.hh48
1 files changed, 22 insertions, 26 deletions
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index ec438bc50..745fe38bf 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -1887,39 +1887,33 @@ template<class TagStore>
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 (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 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 (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 @@ Cache<TagStore>::getNextMSHR()
// 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