[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: MESI_Three_Level LL/SC improvements

2020-05-02 Thread Pouya Fotouhi (Gerrit) via gem5-dev
Pouya Fotouhi has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/28328 )


Change subject: mem-ruby: MESI_Three_Level LL/SC improvements
..

mem-ruby: MESI_Three_Level LL/SC improvements

This patch fixes the MESI_Three_Level protocols so that it correctly
informers the Ruby sequencer when a line eviction occurs. Furthermore,
the patch allows the protocol to recognize the 'Store_Conditional'
RubyRequestType and shortcuts this operation if the monitored line
has been cleared from the address monitor. This prevents certain
livelock behaviour in which a line could ping-pong between competing
cores.

The patch establishes a new C/C++ preprocessor definition which allows
the Sequencer to send the 'Store_Conditional' RubyRequestType to
MESI_Three_Level instead of 'ST'. This is a temporary measure until
the other protocols explicitely recognize 'Store_Conditional'.

Change-Id: I27ae041ab0e015a4f54f20df666f9c4873c7583d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/28328
Reviewed-by: Daniel Carvalho 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
M src/mem/ruby/system/SConscript
M src/mem/ruby/system/Sequencer.cc
3 files changed, 77 insertions(+), 23 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm  
b/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm

index b74a727..14fb07a 100644
--- a/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
+++ b/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
@@ -121,6 +121,8 @@
 Ack,desc="Ack for processor";

 WB_Ack,desc="Ack for replacement";
+
+Failed_SC,desc="Store conditional request that will fail";
   }

   // TYPES
@@ -257,7 +259,8 @@
   return Event:Load;
 } else if (type == RubyRequestType:IFETCH) {
   return Event:Ifetch;
-} else if ((type == RubyRequestType:ST) || (type ==  
RubyRequestType:ATOMIC)) {
+} else if ((type == RubyRequestType:ST) || (type ==  
RubyRequestType:ATOMIC)

+   || (type == RubyRequestType:Store_Conditional)) {
   return Event:Store;
 } else {
   error("Invalid RubyRequestType");
@@ -349,36 +352,51 @@
 }
   }
 } else {
-
   // *** DATA ACCESS ***
   Entry Dcache_entry := getDCacheEntry(in_msg.LineAddress);
+
+  // early out for failed store conditionals
+
+  if (in_msg.Type == RubyRequestType:Store_Conditional) {
+  if (!sequencer.llscCheckMonitor(in_msg.LineAddress)) {
+trigger(Event:Failed_SC, in_msg.LineAddress,
+Dcache_entry, TBEs[in_msg.LineAddress]);
+}
+  }
+
   if (is_valid(Dcache_entry)) {
 // The tag matches for the L0, so the L0 ask the L1 for it
 trigger(mandatory_request_type_to_event(in_msg.Type),  
in_msg.LineAddress,

 Dcache_entry, TBEs[in_msg.LineAddress]);
   } else {
-
-// Check to see if it is in the OTHER L0
-Entry Icache_entry := getICacheEntry(in_msg.LineAddress);
-if (is_valid(Icache_entry)) {
-  // The block is in the wrong L0, put the request on the  
queue to the private L1

-  trigger(Event:L0_Replacement, in_msg.LineAddress,
-  Icache_entry, TBEs[in_msg.LineAddress]);
-}
-
-if (Dcache.cacheAvail(in_msg.LineAddress)) {
-  // L1 does't have the line, but we have space for it
-  // in the L0 let's see if the L1 has it
-  trigger(mandatory_request_type_to_event(in_msg.Type),  
in_msg.LineAddress,

-  Dcache_entry, TBEs[in_msg.LineAddress]);
+// if the request is not valid, the store conditional will fail
+if (in_msg.Type == RubyRequestType:Store_Conditional) {
+// if the line is not valid, it can't be locked
+trigger(Event:Failed_SC, in_msg.LineAddress,
+Dcache_entry, TBEs[in_msg.LineAddress]);
 } else {
-  // No room in the L1, so we need to make room in the L0
-  // Check if the line we want to evict is not locked
-  Addr addr := Dcache.cacheProbe(in_msg.LineAddress);
-  check_on_cache_probe(mandatoryQueue_in, addr);
-  trigger(Event:L0_Replacement, addr,
-  getDCacheEntry(addr),
-  TBEs[addr]);
+  // Check to see if it is in the OTHER L0
+  Entry Icache_entry := getICacheEntry(in_msg.LineAddress);
+  if (is_valid(Icache_entry)) {
+// The block is in the wrong L0, put the request on the  
queue to the private L1

+ 

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: MESI_Three_Level LL/SC improvements

2020-04-30 Thread Giacomo Travaglini (Gerrit) via gem5-dev

Hello Timothy Hayes,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/c/public/gem5/+/28328

to review the following change.


Change subject: mem-ruby: MESI_Three_Level LL/SC improvements
..

mem-ruby: MESI_Three_Level LL/SC improvements

This patch fixes the MESI_Three_Level protocols so that it correctly
informers the Ruby sequencer when a line eviction occurs. Furthermore,
the patch allows the protocol to recognize the 'Store_Conditional'
RubyRequestType and shortcuts this operation if the monitored line
has been cleared from the address monitor. This prevents certain
livelock behaviour in which a line could ping-pong between competing
cores.

The patch establishes a new C/C++ preprocessor definition which allows
the Sequencer to send the 'Store_Conditional' RubyRequestType to
MESI_Three_Level instead of 'ST'. This is a temporary measure until
the other protocols explicitely recognize 'Store_Conditional'.

Change-Id: I27ae041ab0e015a4f54f20df666f9c4873c7583d
---
M src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
M src/mem/ruby/system/SConscript
M src/mem/ruby/system/Sequencer.cc
3 files changed, 77 insertions(+), 23 deletions(-)



diff --git a/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm  
b/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm

index b74a727..14fb07a 100644
--- a/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
+++ b/src/mem/ruby/protocol/MESI_Three_Level-L0cache.sm
@@ -121,6 +121,8 @@
 Ack,desc="Ack for processor";

 WB_Ack,desc="Ack for replacement";
+
+Failed_SC,desc="Store conditional request that will fail";
   }

   // TYPES
@@ -257,7 +259,8 @@
   return Event:Load;
 } else if (type == RubyRequestType:IFETCH) {
   return Event:Ifetch;
-} else if ((type == RubyRequestType:ST) || (type ==  
RubyRequestType:ATOMIC)) {
+} else if ((type == RubyRequestType:ST) || (type ==  
RubyRequestType:ATOMIC)

+   || (type == RubyRequestType:Store_Conditional)) {
   return Event:Store;
 } else {
   error("Invalid RubyRequestType");
@@ -349,36 +352,51 @@
 }
   }
 } else {
-
   // *** DATA ACCESS ***
   Entry Dcache_entry := getDCacheEntry(in_msg.LineAddress);
+
+  // early out for failed store conditionals
+
+  if (in_msg.Type == RubyRequestType:Store_Conditional) {
+  if (!sequencer.llscCheckMonitor(in_msg.LineAddress)) {
+trigger(Event:Failed_SC, in_msg.LineAddress,
+Dcache_entry, TBEs[in_msg.LineAddress]);
+}
+  }
+
   if (is_valid(Dcache_entry)) {
 // The tag matches for the L0, so the L0 ask the L1 for it
 trigger(mandatory_request_type_to_event(in_msg.Type),  
in_msg.LineAddress,

 Dcache_entry, TBEs[in_msg.LineAddress]);
   } else {
-
-// Check to see if it is in the OTHER L0
-Entry Icache_entry := getICacheEntry(in_msg.LineAddress);
-if (is_valid(Icache_entry)) {
-  // The block is in the wrong L0, put the request on the  
queue to the private L1

-  trigger(Event:L0_Replacement, in_msg.LineAddress,
-  Icache_entry, TBEs[in_msg.LineAddress]);
-}
-
-if (Dcache.cacheAvail(in_msg.LineAddress)) {
-  // L1 does't have the line, but we have space for it
-  // in the L0 let's see if the L1 has it
-  trigger(mandatory_request_type_to_event(in_msg.Type),  
in_msg.LineAddress,

-  Dcache_entry, TBEs[in_msg.LineAddress]);
+// if the request is not valid, the store conditional will fail
+if (in_msg.Type == RubyRequestType:Store_Conditional) {
+// if the line is not valid, it can't be locked
+trigger(Event:Failed_SC, in_msg.LineAddress,
+Dcache_entry, TBEs[in_msg.LineAddress]);
 } else {
-  // No room in the L1, so we need to make room in the L0
-  // Check if the line we want to evict is not locked
-  Addr addr := Dcache.cacheProbe(in_msg.LineAddress);
-  check_on_cache_probe(mandatoryQueue_in, addr);
-  trigger(Event:L0_Replacement, addr,
-  getDCacheEntry(addr),
-  TBEs[addr]);
+  // Check to see if it is in the OTHER L0
+  Entry Icache_entry := getICacheEntry(in_msg.LineAddress);
+  if (is_valid(Icache_entry)) {
+// The block is in the wrong L0, put the request on the  
queue to the private L1

+trigger(Event:L0_Replacement, in_msg.LineAddress,
+Icache_entry, TBEs[in_msg.LineAddress]);
+  }
+
+  if (Dcache.cacheAvail(in_msg.LineAddress)) {
+//