From aff2cad4bed219a6827c26fd632c8cb3904752e6 Mon Sep 17 00:00:00 2001 From: Jing Qu Date: Thu, 19 Sep 2019 17:41:57 -0500 Subject: [PATCH] mem-ruby: prevent cacheProbe being called multiple times The cacheProbe() function will return the victim entry, and it gets called for multiple times in trigger function in a single miss. This will cause a problem when we try to add a new replacement policy to the Ruby system. Certain policy, like RRIP, will modify the block information every time the getVictim() function gets called. To prevent future problems, we need to store the victim entry, so that we only call it once in one miss. Change-Id: Ic5ca05f789d9bbfb963b8e993ef707020f243702 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/21099 Reviewed-by: Jason Lowe-Power Reviewed-by: Pouya Fotouhi Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/mem/ruby/protocol/MESI_Three_Level-L1cache.sm | 12 +++++------- src/mem/ruby/protocol/MESI_Two_Level-L1cache.sm | 10 ++++------ src/mem/ruby/protocol/MESI_Two_Level-L2cache.sm | 10 +++++----- src/mem/ruby/protocol/MOESI_CMP_directory-L2cache.sm | 12 ++++++------ src/mem/ruby/protocol/MOESI_CMP_token-L1cache.sm | 11 +++++------ src/mem/ruby/protocol/MOESI_CMP_token-L2cache.sm | 10 ++++------ src/mem/ruby/protocol/MOESI_hammer-dir.sm | 6 +++--- 7 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/mem/ruby/protocol/MESI_Three_Level-L1cache.sm b/src/mem/ruby/protocol/MESI_Three_Level-L1cache.sm index 6db35ceeb..79d554d39 100644 --- a/src/mem/ruby/protocol/MESI_Three_Level-L1cache.sm +++ b/src/mem/ruby/protocol/MESI_Three_Level-L1cache.sm @@ -365,18 +365,16 @@ machine(MachineType:L1Cache, "MESI Directory L1 Cache CMP") in_msg.addr, cache_entry, tbe); } else { // No room in the L1, so we need to make room in the L1 - Entry victim_entry := - getCacheEntry(cache.cacheProbe(in_msg.addr)); - TBE victim_tbe := TBEs[cache.cacheProbe(in_msg.addr)]; + Addr victim := cache.cacheProbe(in_msg.addr); + Entry victim_entry := getCacheEntry(victim); + TBE victim_tbe := TBEs[victim]; if (is_valid(victim_entry) && inL0Cache(victim_entry.CacheState)) { trigger(Event:L0_Invalidate_Own, - cache.cacheProbe(in_msg.addr), - victim_entry, victim_tbe); + victim, victim_entry, victim_tbe); } else { trigger(Event:L1_Replacement, - cache.cacheProbe(in_msg.addr), - victim_entry, victim_tbe); + victim, victim_entry, victim_tbe); } } } diff --git a/src/mem/ruby/protocol/MESI_Two_Level-L1cache.sm b/src/mem/ruby/protocol/MESI_Two_Level-L1cache.sm index 51d3f621c..7c83478db 100644 --- a/src/mem/ruby/protocol/MESI_Two_Level-L1cache.sm +++ b/src/mem/ruby/protocol/MESI_Two_Level-L1cache.sm @@ -330,10 +330,9 @@ machine(MachineType:L1Cache, "MESI Directory L1 Cache CMP") L1Icache_entry, TBEs[in_msg.LineAddress]); } else { // No room in the L1, so we need to make room in the L1 + Addr victim := L1Icache.cacheProbe(in_msg.LineAddress); trigger(Event:PF_L1_Replacement, - L1Icache.cacheProbe(in_msg.LineAddress), - getL1ICacheEntry(L1Icache.cacheProbe(in_msg.LineAddress)), - TBEs[L1Icache.cacheProbe(in_msg.LineAddress)]); + victim, getL1ICacheEntry(victim), TBEs[victim]); } } else { // Data prefetch @@ -364,10 +363,9 @@ machine(MachineType:L1Cache, "MESI Directory L1 Cache CMP") L1Dcache_entry, TBEs[in_msg.LineAddress]); } else { // No room in the L1, so we need to make room in the L1 + Addr victim := L1Dcache.cacheProbe(in_msg.LineAddress); trigger(Event:PF_L1_Replacement, - L1Dcache.cacheProbe(in_msg.LineAddress), - getL1DCacheEntry(L1Dcache.cacheProbe(in_msg.LineAddress)), - TBEs[L1Dcache.cacheProbe(in_msg.LineAddress)]); + victim, getL1DCacheEntry(victim), TBEs[victim]); } } } diff --git a/src/mem/ruby/protocol/MESI_Two_Level-L2cache.sm b/src/mem/ruby/protocol/MESI_Two_Level-L2cache.sm index 5a8cfae6d..988cfd2f7 100644 --- a/src/mem/ruby/protocol/MESI_Two_Level-L2cache.sm +++ b/src/mem/ruby/protocol/MESI_Two_Level-L2cache.sm @@ -378,13 +378,13 @@ machine(MachineType:L2Cache, "MESI Directory L2 Cache CMP") in_msg.addr, cache_entry, tbe); } else { // No room in the L2, so we need to make room before handling the request - Entry L2cache_entry := getCacheEntry(L2cache.cacheProbe(in_msg.addr)); + Addr victim := L2cache.cacheProbe(in_msg.addr); + Entry L2cache_entry := getCacheEntry(victim); if (isDirty(L2cache_entry)) { - trigger(Event:L2_Replacement, L2cache.cacheProbe(in_msg.addr), - L2cache_entry, TBEs[L2cache.cacheProbe(in_msg.addr)]); + trigger(Event:L2_Replacement, victim, L2cache_entry, TBEs[victim]); } else { - trigger(Event:L2_Replacement_clean, L2cache.cacheProbe(in_msg.addr), - L2cache_entry, TBEs[L2cache.cacheProbe(in_msg.addr)]); + trigger(Event:L2_Replacement_clean, + victim, L2cache_entry, TBEs[victim]); } } } diff --git a/src/mem/ruby/protocol/MOESI_CMP_directory-L2cache.sm b/src/mem/ruby/protocol/MOESI_CMP_directory-L2cache.sm index faea79fec..f20085fd5 100644 --- a/src/mem/ruby/protocol/MOESI_CMP_directory-L2cache.sm +++ b/src/mem/ruby/protocol/MOESI_CMP_directory-L2cache.sm @@ -714,9 +714,9 @@ machine(MachineType:L2Cache, "Token protocol") Entry cache_entry := getCacheEntry(in_msg.addr); if (is_invalid(cache_entry) && L2cache.cacheAvail(in_msg.addr) == false) { - trigger(Event:L2_Replacement, L2cache.cacheProbe(in_msg.addr), - getCacheEntry(L2cache.cacheProbe(in_msg.addr)), - TBEs[L2cache.cacheProbe(in_msg.addr)]); + Addr victim := L2cache.cacheProbe(in_msg.addr); + trigger(Event:L2_Replacement, + victim, getCacheEntry(victim), TBEs[victim]); } else { trigger(Event:L1_WBDIRTYDATA, in_msg.addr, @@ -726,9 +726,9 @@ machine(MachineType:L2Cache, "Token protocol") Entry cache_entry := getCacheEntry(in_msg.addr); if (is_invalid(cache_entry) && L2cache.cacheAvail(in_msg.addr) == false) { - trigger(Event:L2_Replacement, L2cache.cacheProbe(in_msg.addr), - getCacheEntry(L2cache.cacheProbe(in_msg.addr)), - TBEs[L2cache.cacheProbe(in_msg.addr)]); + Addr victim := L2cache.cacheProbe(in_msg.addr); + trigger(Event:L2_Replacement, + victim, getCacheEntry(victim), TBEs[victim]); } else { trigger(Event:L1_WBCLEANDATA, in_msg.addr, diff --git a/src/mem/ruby/protocol/MOESI_CMP_token-L1cache.sm b/src/mem/ruby/protocol/MOESI_CMP_token-L1cache.sm index db06fb591..960afda33 100644 --- a/src/mem/ruby/protocol/MOESI_CMP_token-L1cache.sm +++ b/src/mem/ruby/protocol/MOESI_CMP_token-L1cache.sm @@ -696,9 +696,9 @@ machine(MachineType:L1Cache, "Token protocol") } else { // No room in the L1, so we need to make room trigger(Event:L1_Replacement, - L1Icache.cacheProbe(in_msg.LineAddress), - getL1ICacheEntry(L1Icache.cacheProbe(in_msg.LineAddress)), - L1_TBEs[L1Icache.cacheProbe(in_msg.LineAddress)]); + Addr victim := L1Icache.cacheProbe(in_msg.LineAddress); + trigger(Event:L1_Replacement, + victim, getL1ICacheEntry(victim), L1_TBEs[victim]); } } } else { @@ -726,10 +726,9 @@ machine(MachineType:L1Cache, "Token protocol") in_msg.LineAddress, L1Dcache_entry, tbe); } else { // No room in the L1, so we need to make room + Addr victim := L1Dcache.cacheProbe(in_msg.LineAddress); trigger(Event:L1_Replacement, - L1Dcache.cacheProbe(in_msg.LineAddress), - getL1DCacheEntry(L1Dcache.cacheProbe(in_msg.LineAddress)), - L1_TBEs[L1Dcache.cacheProbe(in_msg.LineAddress)]); + victim, getL1DCacheEntry(victim), L1_TBEs[victim]); } } } diff --git a/src/mem/ruby/protocol/MOESI_CMP_token-L2cache.sm b/src/mem/ruby/protocol/MOESI_CMP_token-L2cache.sm index 7911179c2..948a2d95f 100644 --- a/src/mem/ruby/protocol/MOESI_CMP_token-L2cache.sm +++ b/src/mem/ruby/protocol/MOESI_CMP_token-L2cache.sm @@ -449,9 +449,8 @@ machine(MachineType:L2Cache, "Token protocol") } } else { - trigger(Event:L2_Replacement, - L2cache.cacheProbe(in_msg.addr), - getCacheEntry(L2cache.cacheProbe(in_msg.addr))); + Addr victim := L2cache.cacheProbe(in_msg.addr); + trigger(Event:L2_Replacement, victim, getCacheEntry(victim)); } } else if (in_msg.Type == CoherenceResponseType:INV) { trigger(Event:L1_INV, in_msg.addr, cache_entry); @@ -486,9 +485,8 @@ machine(MachineType:L2Cache, "Token protocol") } } else { - trigger(Event:L2_Replacement, - L2cache.cacheProbe(in_msg.addr), - getCacheEntry(L2cache.cacheProbe(in_msg.addr))); + Addr victim := L2cache.cacheProbe(in_msg.addr); + trigger(Event:L2_Replacement, victim, getCacheEntry(victim)); } } else if (in_msg.Type == CoherenceResponseType:INV) { trigger(Event:L1_INV, in_msg.addr, cache_entry); diff --git a/src/mem/ruby/protocol/MOESI_hammer-dir.sm b/src/mem/ruby/protocol/MOESI_hammer-dir.sm index 42522c727..d85ff1929 100644 --- a/src/mem/ruby/protocol/MOESI_hammer-dir.sm +++ b/src/mem/ruby/protocol/MOESI_hammer-dir.sm @@ -427,9 +427,9 @@ machine(MachineType:Directory, "AMD Hammer-like protocol") pf_entry, tbe); } else { trigger(Event:Pf_Replacement, - probeFilter.cacheProbe(in_msg.addr), - getProbeFilterEntry(probeFilter.cacheProbe(in_msg.addr)), - TBEs[probeFilter.cacheProbe(in_msg.addr)]); + Addr victim := probeFilter.cacheProbe(in_msg.addr); + trigger(Event:Pf_Replacement, + victim, getProbeFilterEntry(victim), TBEs[victim]); } } } else { -- 2.30.2