diff options
| -rw-r--r-- | src/mem/packet.cc | 14 | ||||
| -rw-r--r-- | src/mem/packet.hh | 10 | ||||
| -rwxr-xr-x | src/mem/snoop_filter.cc | 36 |
3 files changed, 31 insertions, 29 deletions
diff --git a/src/mem/packet.cc b/src/mem/packet.cc index 289bc81bc..cacfc1d3b 100644 --- a/src/mem/packet.cc +++ b/src/mem/packet.cc @@ -83,7 +83,7 @@ MemCmd::commandInfo[] = { SET5(IsWrite, NeedsWritable, IsRequest, NeedsResponse, HasData), WriteResp, "WriteReq" }, /* WriteResp */ - { SET3(IsWrite, NeedsWritable, IsResponse), InvalidCmd, "WriteResp" }, + { SET2(IsWrite, IsResponse), InvalidCmd, "WriteResp" }, /* WritebackDirty */ { SET4(IsWrite, IsRequest, IsEviction, HasData), InvalidCmd, "WritebackDirty" }, @@ -117,7 +117,7 @@ MemCmd::commandInfo[] = IsRequest, NeedsResponse), UpgradeResp, "SCUpgradeReq" }, /* UpgradeResp */ - { SET3(NeedsWritable, IsUpgrade, IsResponse), + { SET2(IsUpgrade, IsResponse), InvalidCmd, "UpgradeResp" }, /* SCUpgradeFailReq: generates UpgradeFailResp but still gets the data */ { SET6(IsRead, NeedsWritable, IsInvalidate, @@ -125,7 +125,7 @@ MemCmd::commandInfo[] = UpgradeFailResp, "SCUpgradeFailReq" }, /* UpgradeFailResp - Behaves like a ReadExReq, but notifies an SC * that it has failed, acquires line as Dirty*/ - { SET4(IsRead, NeedsWritable, IsResponse, HasData), + { SET3(IsRead, IsResponse, HasData), InvalidCmd, "UpgradeFailResp" }, /* ReadExReq - Read issues by a cache, always cache-line aligned, * and the response is guaranteed to be writeable (exclusive or @@ -134,7 +134,7 @@ MemCmd::commandInfo[] = ReadExResp, "ReadExReq" }, /* ReadExResp - Response matching a read exclusive, as we check * the need for exclusive also on responses */ - { SET4(IsRead, NeedsWritable, IsResponse, HasData), + { SET3(IsRead, IsResponse, HasData), InvalidCmd, "ReadExResp" }, /* ReadCleanReq - Read issued by a cache, always cache-line * aligned, and the response is guaranteed to not contain dirty data @@ -157,13 +157,13 @@ MemCmd::commandInfo[] = IsRequest, NeedsResponse, HasData), StoreCondResp, "StoreCondFailReq" }, /* StoreCondResp */ - { SET4(IsWrite, NeedsWritable, IsLlsc, IsResponse), + { SET3(IsWrite, IsLlsc, IsResponse), InvalidCmd, "StoreCondResp" }, /* SwapReq -- for Swap ldstub type operations */ { SET6(IsRead, IsWrite, NeedsWritable, IsRequest, HasData, NeedsResponse), SwapResp, "SwapReq" }, /* SwapResp -- for Swap ldstub type operations */ - { SET5(IsRead, IsWrite, NeedsWritable, IsResponse, HasData), + { SET4(IsRead, IsWrite, IsResponse, HasData), InvalidCmd, "SwapResp" }, /* IntReq -- for interrupts */ { SET4(IsWrite, IsRequest, NeedsResponse, HasData), @@ -190,7 +190,7 @@ MemCmd::commandInfo[] = { SET4(IsInvalidate, IsRequest, NeedsWritable, NeedsResponse), InvalidateResp, "InvalidateReq" }, /* Invalidation Response */ - { SET3(IsInvalidate, IsResponse, NeedsWritable), + { SET2(IsInvalidate, IsResponse), InvalidCmd, "InvalidateResp" } }; diff --git a/src/mem/packet.hh b/src/mem/packet.hh index 94508f697..32c4cf631 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -502,7 +502,15 @@ class Packet : public Printable bool isUpgrade() const { return cmd.isUpgrade(); } bool isRequest() const { return cmd.isRequest(); } bool isResponse() const { return cmd.isResponse(); } - bool needsWritable() const { return cmd.needsWritable(); } + bool needsWritable() const + { + // we should never check if a response needsWritable, the + // request has this flag, and for a response we should rather + // look at the hasSharers flag (if not set, the response is to + // be considered writable) + assert(isRequest()); + return cmd.needsWritable(); + } bool needsResponse() const { return cmd.needsResponse(); } bool isInvalidate() const { return cmd.isInvalidate(); } bool isEviction() const { return cmd.isEviction(); } diff --git a/src/mem/snoop_filter.cc b/src/mem/snoop_filter.cc index b1ccc12c9..9d02ed249 100755 --- a/src/mem/snoop_filter.cc +++ b/src/mem/snoop_filter.cc @@ -257,17 +257,14 @@ SnoopFilter::updateSnoopResponse(const Packet* cpkt, panic_if(!(sf_item.requested & req_mask), "SF value %x.%x missing "\ "the original request\n", sf_item.requested, sf_item.holder); - // Update the residency of the cache line. - if (cpkt->needsWritable() || !cpkt->hasSharers()) { - DPRINTF(SnoopFilter, "%s: dropping %x because needs: %i writable: %i "\ - "SF val: %x.%x\n", __func__, rsp_mask, - cpkt->needsWritable(), !cpkt->hasSharers(), + // If the snoop response has no sharers the line is passed in + // Modified state, and we know that there are no other copies, or + // they will all be invalidated imminently + if (!cpkt->hasSharers()) { + DPRINTF(SnoopFilter, + "%s: dropping %x because non-shared snoop " + "response SF val: %x.%x\n", __func__, rsp_mask, sf_item.requested, sf_item.holder); - - sf_item.holder &= ~rsp_mask; - // The snoop filter does not see any ACKs from non-responding sharers - // that have been invalidated :( So below assert would be nice, but.. - //assert(sf_item.holder == 0); sf_item.holder = 0; } assert(!cpkt->isWriteback()); @@ -302,14 +299,10 @@ SnoopFilter::updateSnoopForward(const Packet* cpkt, DPRINTF(SnoopFilter, "%s: old SF value %x.%x\n", __func__, sf_item.requested, sf_item.holder); - // Remote (to this snoop filter) snoops update the filter - // already when they arrive from below, because we may not see - // any response. - if (cpkt->needsWritable()) { - // If the request to this snoop response hit an in-flight - // transaction, - // the holder was not reset -> no assertion & do that here, now! - //assert(sf_item.holder == 0); + // If the snoop response has no sharers the line is passed in + // Modified state, and we know that there are no other copies, or + // they will all be invalidated imminently + if (!cpkt->hasSharers()) { sf_item.holder = 0; } DPRINTF(SnoopFilter, "%s: new SF value %x.%x\n", @@ -343,9 +336,10 @@ SnoopFilter::updateResponse(const Packet* cpkt, const SlavePort& slave_port) panic_if(!(sf_item.requested & slave_mask), "SF value %x.%x missing "\ "request bit\n", sf_item.requested, sf_item.holder); - // Update the residency of the cache line. Here we assume that the - // line has been zapped in all caches that are not the responder. - if (cpkt->needsWritable() || !cpkt->hasSharers()) + // Update the residency of the cache line. If the response has no + // sharers we know that the line has been invalidated in all + // branches that are not where we are responding to. + if (!cpkt->hasSharers()) sf_item.holder = 0; sf_item.holder |= slave_mask; sf_item.requested &= ~slave_mask; |