ruby: more flexible ruby tester support

This patch allows the ruby random tester to use ruby ports that may only
support instr or data requests.  This patch is similar to a previous changeset
(8932:1b2c17565ac8) that was unfortunately broken by subsequent changesets.
This current patch implements the support in a more straight-forward way.
Since retries are now tested when running the ruby random tester, this patch
splits up the retry and drain check behavior so that RubyPort children, such
as the GPUCoalescer, can perform those operations correctly without having to
duplicate code.  Finally, the patch also includes better DPRINTFs for
debugging the tester.

5 files changed, 88 insertions, 31 deletions

diff --git a/src/cpu/testers/rubytest/Check.cc b/src/cpu/testers/rubytest/Check.cc
index 4cdaf9b2f..c8e7816c3 100644
--- a/src/cpu/testers/rubytest/Check.cc
+++ b/src/cpu/testers/rubytest/Check.cc
@@ -94,7 +94,9 @@ Check::initiatePrefetch()
         cmd = MemCmd::ReadReq;
 
         // if necessary, make the request an instruction fetch
-        if (m_tester_ptr->isInstReadableCpuPort(index)) {
+        if (m_tester_ptr->isInstOnlyCpuPort(index) ||
+            (m_tester_ptr->isInstDataCpuPort(index) &&
+             (random_mt.random(0, 0x1)))) {
             flags.set(Request::INST_FETCH);
         }
     } else {
@@ -193,7 +195,7 @@ Check::initiateAction()
     *writeData = m_value + m_store_count;
     pkt->dataDynamic(writeData);
 
-    DPRINTF(RubyTest, "data 0x%x check 0x%x\n",
+    DPRINTF(RubyTest, "Seq write: index %d data 0x%x check 0x%x\n", index,
             *(pkt->getConstPtr<uint8_t>()), *writeData);
 
     // push the subblock onto the sender state.  The sequencer will
@@ -205,6 +207,7 @@ Check::initiateAction()
         DPRINTF(RubyTest, "status before action update: %s\n",
                 (TesterStatus_to_string(m_status)).c_str());
         m_status = TesterStatus_Action_Pending;
+        DPRINTF(RubyTest, "Check %s, State=Action_Pending\n", m_address);
     } else {
         // If the packet did not issue, must delete
         // Note: No need to delete the data, the packet destructor
@@ -232,7 +235,9 @@ Check::initiateCheck()
     Request::Flags flags;
 
     // If necessary, make the request an instruction fetch
-    if (m_tester_ptr->isInstReadableCpuPort(index)) {
+    if (m_tester_ptr->isInstOnlyCpuPort(index) ||
+        (m_tester_ptr->isInstDataCpuPort(index) &&
+         (random_mt.random(0, 0x1)))) {
         flags.set(Request::INST_FETCH);
     }
 
@@ -245,6 +250,8 @@ Check::initiateCheck()
     uint8_t *dataArray = new uint8_t[CHECK_SIZE];
     pkt->dataDynamic(dataArray);
 
+    DPRINTF(RubyTest, "Seq read: index %d\n", index);
+
     // push the subblock onto the sender state.  The sequencer will
     // update the subblock on the return
     pkt->senderState = new SenderState(m_address, req->getSize());
@@ -254,6 +261,7 @@ Check::initiateCheck()
         DPRINTF(RubyTest, "status before check update: %s\n",
                 TesterStatus_to_string(m_status).c_str());
         m_status = TesterStatus_Check_Pending;
+        DPRINTF(RubyTest, "Check %s, State=Check_Pending\n", m_address);
     } else {
         // If the packet did not issue, must delete
         // Note: No need to delete the data, the packet destructor
@@ -291,8 +299,11 @@ Check::performCallback(NodeID proc, SubBlock* data, Cycles curTime)
         m_store_count++;
         if (m_store_count == CHECK_SIZE) {
             m_status = TesterStatus_Ready;
+            DPRINTF(RubyTest, "Check %s, State=Ready\n", m_address);
         } else {
             m_status = TesterStatus_Idle;
+            DPRINTF(RubyTest, "Check %s, State=Idle store_count: %d\n",
+                    m_address, m_store_count);
         }
         DPRINTF(RubyTest, "Action callback return data now %d\n",
                 data->getByte(0));
@@ -316,6 +327,7 @@ Check::performCallback(NodeID proc, SubBlock* data, Cycles curTime)
         m_tester_ptr->incrementCheckCompletions();
 
         m_status = TesterStatus_Idle;
+        DPRINTF(RubyTest, "Check %s, State=Idle\n", m_address);
         pickValue();
 
     } else {
@@ -335,6 +347,7 @@ Check::changeAddress(Addr address)
     assert(m_status == TesterStatus_Idle || m_status == TesterStatus_Ready);
     m_status = TesterStatus_Idle;
     m_address = address;
+    DPRINTF(RubyTest, "Check %s, State=Idle\n", m_address);
     m_store_count = 0;
 }
 
@@ -342,7 +355,6 @@ void
 Check::pickValue()
 {
     assert(m_status == TesterStatus_Idle);
-    m_status = TesterStatus_Idle;
     m_value = random_mt.random(0, 0xff); // One byte
     m_store_count = 0;
 }
@@ -353,7 +365,8 @@ Check::pickInitiatingNode()
     assert(m_status == TesterStatus_Idle || m_status == TesterStatus_Ready);
     m_status = TesterStatus_Idle;
     m_initiatingNode = (random_mt.random(0, m_num_writers - 1));
-    DPRINTF(RubyTest, "picked initiating node %d\n", m_initiatingNode);
+    DPRINTF(RubyTest, "Check %s, State=Idle, picked initiating node %d\n",
+            m_address, m_initiatingNode);
     m_store_count = 0;
 }
 
diff --git a/src/cpu/testers/rubytest/CheckTable.cc b/src/cpu/testers/rubytest/CheckTable.cc
index b75fd0a52..3bdd73f27 100644
--- a/src/cpu/testers/rubytest/CheckTable.cc
+++ b/src/cpu/testers/rubytest/CheckTable.cc
@@ -42,6 +42,7 @@ CheckTable::CheckTable(int _num_writers, int _num_readers, RubyTester* _tester)
     const int size1 = 32;
     const int size2 = 100;
 
+    DPRINTF(RubyTest, "Adding false sharing checks\n");
     // The first set is to get some false sharing
     physical = 1000;
     for (int i = 0; i < size1; i++) {
@@ -50,6 +51,7 @@ CheckTable::CheckTable(int _num_writers, int _num_readers, RubyTester* _tester)
         physical += CHECK_SIZE;
     }
 
+    DPRINTF(RubyTest, "Adding cache conflict checks\n");
     // The next two sets are to get some limited false sharing and
     // cache conflicts
     physical = 1000;
@@ -59,6 +61,7 @@ CheckTable::CheckTable(int _num_writers, int _num_readers, RubyTester* _tester)
         physical += 256;
     }
 
+    DPRINTF(RubyTest, "Adding cache conflict checks2\n");
     physical = 1000 + CHECK_SIZE;
     for (int i = 0; i < size2; i++) {
         // Setup linear addresses
@@ -91,6 +94,8 @@ CheckTable::addCheck(Addr address)
         }
     }
 
+    DPRINTF(RubyTest, "Adding check for address: %s\n", address);
+
     Check* check_ptr = new Check(address, 100 + m_check_vector.size(),
                                  m_num_writers, m_num_readers, m_tester_ptr);
     for (int i = 0; i < CHECK_SIZE; i++) {
@@ -110,7 +115,7 @@ CheckTable::getRandomCheck()
 Check*
 CheckTable::getCheck(const Addr address)
 {
-    DPRINTF(RubyTest, "Looking for check by address: %s", address);
+    DPRINTF(RubyTest, "Looking for check by address: %s\n", address);
 
     auto i = m_lookup_map.find(address);
 
diff --git a/src/cpu/testers/rubytest/RubyTester.cc b/src/cpu/testers/rubytest/RubyTester.cc
index e0f30f552..5ed6d7f66 100644
--- a/src/cpu/testers/rubytest/RubyTester.cc
+++ b/src/cpu/testers/rubytest/RubyTester.cc
@@ -58,7 +58,8 @@ RubyTester::RubyTester(const Params *p)
     m_num_readers(0),
     m_wakeup_frequency(p->wakeup_frequency),
     m_check_flush(p->check_flush),
-    m_num_inst_ports(p->port_cpuInstPort_connection_count)
+    m_num_inst_only_ports(p->port_cpuInstPort_connection_count),
+    m_num_inst_data_ports(p->port_cpuInstDataPort_connection_count)
 {
     m_checks_completed = 0;
 
@@ -73,15 +74,25 @@ RubyTester::RubyTester(const Params *p)
     // Note: the inst ports are the lowest elements of the readPort vector,
     // then the data ports are added to the readPort vector
     //
+    int idx = 0;
     for (int i = 0; i < p->port_cpuInstPort_connection_count; ++i) {
         readPorts.push_back(new CpuPort(csprintf("%s-instPort%d", name(), i),
-                                        this, i));
+                                        this, i, idx));
+        idx++;
+    }
+    for (int i = 0; i < p->port_cpuInstDataPort_connection_count; ++i) {
+        CpuPort *port = new CpuPort(csprintf("%s-instDataPort%d", name(), i),
+                                    this, i, idx);
+        readPorts.push_back(port);
+        writePorts.push_back(port);
+        idx++;
     }
     for (int i = 0; i < p->port_cpuDataPort_connection_count; ++i) {
         CpuPort *port = new CpuPort(csprintf("%s-dataPort%d", name(), i),
-                                    this, i);
+                                    this, i, idx);
         readPorts.push_back(port);
         writePorts.push_back(port);
+        idx++;
     }
 
     // add the check start event to the event queue
@@ -108,6 +119,7 @@ RubyTester::init()
 
     m_num_writers = writePorts.size();
     m_num_readers = readPorts.size();
+    assert(m_num_readers == m_num_cpus);
 
     m_checkTable_ptr = new CheckTable(m_num_writers, m_num_readers, this);
 }
@@ -115,32 +127,45 @@ RubyTester::init()
 BaseMasterPort &
 RubyTester::getMasterPort(const std::string &if_name, PortID idx)
 {
-    if (if_name != "cpuInstPort" && if_name != "cpuDataPort") {
+    if (if_name != "cpuInstPort" && if_name != "cpuInstDataPort" &&
+        if_name != "cpuDataPort") {
         // pass it along to our super class
         return MemObject::getMasterPort(if_name, idx);
     } else {
         if (if_name == "cpuInstPort") {
-            if (idx > m_num_inst_ports) {
-                panic("RubyTester::getMasterPort: unknown inst port idx %d\n",
+            if (idx > m_num_inst_only_ports) {
+                panic("RubyTester::getMasterPort: unknown inst port %d\n",
                       idx);
             }
             //
-            // inst ports directly map to the lowest readPort elements
+            // inst ports map to the lowest readPort elements
             //
             return *readPorts[idx];
+        } else if (if_name == "cpuInstDataPort") {
+            if (idx > m_num_inst_data_ports) {
+                panic("RubyTester::getMasterPort: unknown inst+data port %d\n",
+                      idx);
+            }
+            int read_idx = idx + m_num_inst_only_ports;
+            //
+            // inst+data ports map to the next readPort elements
+            //
+            return *readPorts[read_idx];
         } else {
             assert(if_name == "cpuDataPort");
             //
-            // add the inst port offset to translate to the correct read port
-            // index
+            // data only ports map to the final readPort elements
             //
-            int read_idx = idx + m_num_inst_ports;
-            if (read_idx >= static_cast<PortID>(readPorts.size())) {
-                panic("RubyTester::getMasterPort: unknown data port idx %d\n",
+            if (idx > (static_cast<int>(readPorts.size()) -
+                       (m_num_inst_only_ports + m_num_inst_data_ports))) {
+                panic("RubyTester::getMasterPort: unknown data port %d\n",
                       idx);
             }
+            int read_idx = idx + m_num_inst_only_ports + m_num_inst_data_ports;
             return *readPorts[read_idx];
         }
+        // Note: currently the Ruby Tester does not support write only ports
+        // but that could easily be added here
     }
 }
 
@@ -152,7 +177,7 @@ RubyTester::CpuPort::recvTimingResp(PacketPtr pkt)
         safe_cast<RubyTester::SenderState*>(pkt->senderState);
     SubBlock& subblock = senderState->subBlock;
 
-    tester->hitCallback(id, &subblock);
+    tester->hitCallback(globalIdx, &subblock);
 
     // Now that the tester has completed, delete the senderState
     // (includes sublock) and the packet, then return
@@ -163,9 +188,16 @@ RubyTester::CpuPort::recvTimingResp(PacketPtr pkt)
 }
 
 bool
-RubyTester::isInstReadableCpuPort(int idx)
+RubyTester::isInstOnlyCpuPort(int idx)
+{
+    return idx < m_num_inst_only_ports;
+}
+
+bool
+RubyTester::isInstDataCpuPort(int idx)
 {
-    return idx < m_num_inst_ports;
+    return ((idx >= m_num_inst_only_ports) &&
+            (idx < (m_num_inst_only_ports + m_num_inst_data_ports)));
 }
 
 MasterPort*
@@ -190,13 +222,13 @@ RubyTester::hitCallback(NodeID proc, SubBlock* data)
     // Mark that we made progress
     m_last_progress_vector[proc] = curCycle();
 
-    DPRINTF(RubyTest, "completed request for proc: %d\n", proc);
-    DPRINTF(RubyTest, "addr: 0x%x, size: %d, data: ",
+    DPRINTF(RubyTest, "completed request for proc: %d", proc);
+    DPRINTFR(RubyTest, " addr: 0x%x, size: %d, data: ",
             data->getAddress(), data->getSize());
     for (int byte = 0; byte < data->getSize(); byte++) {
-        DPRINTF(RubyTest, "%d", data->getByte(byte));
+        DPRINTFR(RubyTest, "%d ", data->getByte(byte));
     }
-    DPRINTF(RubyTest, "\n");
+    DPRINTFR(RubyTest, "\n");
 
     // This tells us our store has 'completed' or for a load gives us
     // back the data to make the check
diff --git a/src/cpu/testers/rubytest/RubyTester.hh b/src/cpu/testers/rubytest/RubyTester.hh
index 94a982e32..39e6d78a3 100644
--- a/src/cpu/testers/rubytest/RubyTester.hh
+++ b/src/cpu/testers/rubytest/RubyTester.hh
@@ -60,6 +60,8 @@ class RubyTester : public MemObject
     {
       private:
         RubyTester *tester;
+        // index for m_last_progress_vector and hitCallback
+        PortID globalIdx;
 
       public:
         //
@@ -68,8 +70,10 @@ class RubyTester : public MemObject
         // RubyPorts that support both types of requests, separate InstOnly
         // and DataOnly CpuPorts will map to that RubyPort
 
-        CpuPort(const std::string &_name, RubyTester *_tester, PortID _id)
-            : MasterPort(_name, _tester, _id), tester(_tester)
+        CpuPort(const std::string &_name, RubyTester *_tester, PortID _id,
+                PortID _index)
+            : MasterPort(_name, _tester, _id), tester(_tester),
+              globalIdx(_index)
         {}
 
       protected:
@@ -93,7 +97,8 @@ class RubyTester : public MemObject
     virtual BaseMasterPort &getMasterPort(const std::string &if_name,
                                           PortID idx = InvalidPortID);
 
-    bool isInstReadableCpuPort(int idx);
+    bool isInstOnlyCpuPort(int idx);
+    bool isInstDataCpuPort(int idx);
 
     MasterPort* getReadableCpuPort(int idx);
     MasterPort* getWritableCpuPort(int idx);
@@ -152,7 +157,8 @@ class RubyTester : public MemObject
     int m_num_readers;
     int m_wakeup_frequency;
     bool m_check_flush;
-    int m_num_inst_ports;
+    int m_num_inst_only_ports;
+    int m_num_inst_data_ports;
 };
 
 inline std::ostream&
diff --git a/src/cpu/testers/rubytest/RubyTester.py b/src/cpu/testers/rubytest/RubyTester.py
index 7af70cae0..f12485566 100644
--- a/src/cpu/testers/rubytest/RubyTester.py
+++ b/src/cpu/testers/rubytest/RubyTester.py
@@ -34,8 +34,9 @@ class RubyTester(MemObject):
     type = 'RubyTester'
     cxx_header = "cpu/testers/rubytest/RubyTester.hh"
     num_cpus = Param.Int("number of cpus / RubyPorts")
-    cpuDataPort = VectorMasterPort("the cpu data cache ports")
-    cpuInstPort = VectorMasterPort("the cpu inst cache ports")
+    cpuInstDataPort = VectorMasterPort("cpu combo ports to inst & data caches")
+    cpuInstPort = VectorMasterPort("cpu ports to only inst caches")
+    cpuDataPort = VectorMasterPort("cpu ports to only data caches")
     checks_to_complete = Param.Int(100, "checks to complete")
     deadlock_threshold = Param.Int(50000, "how often to check for deadlock")
     wakeup_frequency = Param.Int(10, "number of cycles between wakeups")