[gem5-dev] Change in gem5/gem5[develop]: mem: Use the new unbound port reporting mechanism in the mem ports.

2020-06-16 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/30296 )


Change subject: mem: Use the new unbound port reporting mechanism in the  
mem ports.

..

mem: Use the new unbound port reporting mechanism in the mem ports.

There was an add-hoc check added to getAddrRanges, but the other methods
would just segfault if they tried to talk to their peers. This change
wraps all the calls in try blocks and catches the exception which the
peer will throw if it's the default and the port is not actually
connected to anything.

Change-Id: Ie46be0230f33f74305c599b251ca319a65ba008d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30296
Reviewed-by: Nikos Nikoleris 
Maintainer: Nikos Nikoleris 
Tested-by: kokoro 
---
M src/mem/port.cc
M src/mem/port.hh
2 files changed, 137 insertions(+), 32 deletions(-)

Approvals:
  Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/port.cc b/src/mem/port.cc
index 883b592..47e94f4 100644
--- a/src/mem/port.cc
+++ b/src/mem/port.cc
@@ -47,11 +47,72 @@
 #include "base/trace.hh"
 #include "sim/sim_object.hh"

+namespace
+{
+
+class DefaultMasterPort : public MasterPort
+{
+  protected:
+[[noreturn]] void
+blowUp() const
+{
+throw UnboundPortException();
+}
+
+  public:
+DefaultMasterPort() : MasterPort("default_master_port", nullptr) {}
+
+// Atomic protocol.
+Tick recvAtomicSnoop(PacketPtr) override { blowUp(); }
+
+// Timing protocol.
+bool recvTimingResp(PacketPtr) override { blowUp(); }
+void recvTimingSnoopReq(PacketPtr) override { blowUp(); }
+void recvReqRetry() override { blowUp(); }
+void recvRetrySnoopResp() override { blowUp(); }
+
+// Functional protocol.
+void recvFunctionalSnoop(PacketPtr) override { blowUp(); }
+};
+
+class DefaultSlavePort : public SlavePort
+{
+  protected:
+[[noreturn]] void
+blowUp() const
+{
+throw UnboundPortException();
+}
+
+  public:
+DefaultSlavePort() : SlavePort("default_slave_port", nullptr) {}
+
+// Atomic protocol.
+Tick recvAtomic(PacketPtr) override { blowUp(); }
+
+// Timing protocol.
+bool recvTimingReq(PacketPtr) override { blowUp(); }
+bool tryTiming(PacketPtr) override { blowUp(); }
+bool recvTimingSnoopResp(PacketPtr) override { blowUp(); }
+void recvRespRetry() override { blowUp(); }
+
+// Functional protocol.
+void recvFunctional(PacketPtr) override { blowUp(); }
+
+// General.
+AddrRangeList getAddrRanges() const override { return AddrRangeList();  
}

+};
+
+DefaultMasterPort defaultMasterPort;
+DefaultSlavePort defaultSlavePort;
+
+} // anonymous namespace
+
 /**
  * Master port
  */
 MasterPort::MasterPort(const std::string& name, SimObject* _owner, PortID  
_id)

-: Port(name, _id), _slavePort(NULL), owner(*_owner)
+: Port(name, _id), _slavePort(), owner(*_owner)
 {
 }

@@ -63,10 +124,8 @@
 MasterPort::bind(Port )
 {
 auto *slave_port = dynamic_cast();
-if (!slave_port) {
-fatal("Attempt to bind port %s to non-slave port %s.",
-name(), peer.name());
-}
+fatal_if(!slave_port, "Can't bind port %s to non-slave port %s.",
+ name(), peer.name());
 // master port keeps track of the slave port
 _slavePort = slave_port;
 Port::bind(peer);
@@ -77,11 +136,10 @@
 void
 MasterPort::unbind()
 {
-if (_slavePort == NULL)
-panic("Attempting to unbind master port %s that is not  
connected\n",

-  name());
+panic_if(!isConnected(), "Can't unbind master port %s which is not  
bound.",

+ name());
 _slavePort->slaveUnbind();
-_slavePort = nullptr;
+_slavePort = 
 Port::unbind();
 }

@@ -108,8 +166,8 @@
  * Slave port
  */
 SlavePort::SlavePort(const std::string& name, SimObject* _owner, PortID id)
-: Port(name, id), _masterPort(NULL), defaultBackdoorWarned(false),
-owner(*_owner)
+: Port(name, id), _masterPort(),
+defaultBackdoorWarned(false), owner(*_owner)
 {
 }

@@ -120,7 +178,7 @@
 void
 SlavePort::slaveUnbind()
 {
-_masterPort = NULL;
+_masterPort = 
 Port::unbind();
 }

diff --git a/src/mem/port.hh b/src/mem/port.hh
index d84e46f..eadf7f4 100644
--- a/src/mem/port.hh
+++ b/src/mem/port.hh
@@ -258,6 +258,7 @@

   private:
 MasterPort* _masterPort;
+
 bool defaultBackdoorWarned;

   protected:
@@ -278,13 +279,7 @@
 /**
  * Called by the owner to send a range change
  */
-void
-sendRangeChange() const
-{
-fatal_if(!_masterPort,
-"%s cannot sendRangeChange() without master port.",  
name());

-_masterPort->recvRangeChange();
-}
+void sendRangeChange() const { _masterPort->recvRangeChange(); }

 /**
  * Get a list of the non-overlapping address ranges the owner is
@@ -316,7 +311,11 

[gem5-dev] Change in gem5/gem5[develop]: mem: Use the new unbound port reporting mechanism in the mem ports.

2020-06-12 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/30296 )



Change subject: mem: Use the new unbound port reporting mechanism in the  
mem ports.

..

mem: Use the new unbound port reporting mechanism in the mem ports.

There was an add-hoc check added to getAddrRanges, but the other methods
would just segfault if they tried to talk to their peers. This change
wraps all the calls in try blocks and catches the exception which the
peer will throw if it's the default and the port is not actually
connected to anything.

Change-Id: Ie46be0230f33f74305c599b251ca319a65ba008d
---
M src/mem/port.cc
M src/mem/port.hh
2 files changed, 137 insertions(+), 32 deletions(-)



diff --git a/src/mem/port.cc b/src/mem/port.cc
index 883b592..47e94f4 100644
--- a/src/mem/port.cc
+++ b/src/mem/port.cc
@@ -47,11 +47,72 @@
 #include "base/trace.hh"
 #include "sim/sim_object.hh"

+namespace
+{
+
+class DefaultMasterPort : public MasterPort
+{
+  protected:
+[[noreturn]] void
+blowUp() const
+{
+throw UnboundPortException();
+}
+
+  public:
+DefaultMasterPort() : MasterPort("default_master_port", nullptr) {}
+
+// Atomic protocol.
+Tick recvAtomicSnoop(PacketPtr) override { blowUp(); }
+
+// Timing protocol.
+bool recvTimingResp(PacketPtr) override { blowUp(); }
+void recvTimingSnoopReq(PacketPtr) override { blowUp(); }
+void recvReqRetry() override { blowUp(); }
+void recvRetrySnoopResp() override { blowUp(); }
+
+// Functional protocol.
+void recvFunctionalSnoop(PacketPtr) override { blowUp(); }
+};
+
+class DefaultSlavePort : public SlavePort
+{
+  protected:
+[[noreturn]] void
+blowUp() const
+{
+throw UnboundPortException();
+}
+
+  public:
+DefaultSlavePort() : SlavePort("default_slave_port", nullptr) {}
+
+// Atomic protocol.
+Tick recvAtomic(PacketPtr) override { blowUp(); }
+
+// Timing protocol.
+bool recvTimingReq(PacketPtr) override { blowUp(); }
+bool tryTiming(PacketPtr) override { blowUp(); }
+bool recvTimingSnoopResp(PacketPtr) override { blowUp(); }
+void recvRespRetry() override { blowUp(); }
+
+// Functional protocol.
+void recvFunctional(PacketPtr) override { blowUp(); }
+
+// General.
+AddrRangeList getAddrRanges() const override { return AddrRangeList();  
}

+};
+
+DefaultMasterPort defaultMasterPort;
+DefaultSlavePort defaultSlavePort;
+
+} // anonymous namespace
+
 /**
  * Master port
  */
 MasterPort::MasterPort(const std::string& name, SimObject* _owner, PortID  
_id)

-: Port(name, _id), _slavePort(NULL), owner(*_owner)
+: Port(name, _id), _slavePort(), owner(*_owner)
 {
 }

@@ -63,10 +124,8 @@
 MasterPort::bind(Port )
 {
 auto *slave_port = dynamic_cast();
-if (!slave_port) {
-fatal("Attempt to bind port %s to non-slave port %s.",
-name(), peer.name());
-}
+fatal_if(!slave_port, "Can't bind port %s to non-slave port %s.",
+ name(), peer.name());
 // master port keeps track of the slave port
 _slavePort = slave_port;
 Port::bind(peer);
@@ -77,11 +136,10 @@
 void
 MasterPort::unbind()
 {
-if (_slavePort == NULL)
-panic("Attempting to unbind master port %s that is not  
connected\n",

-  name());
+panic_if(!isConnected(), "Can't unbind master port %s which is not  
bound.",

+ name());
 _slavePort->slaveUnbind();
-_slavePort = nullptr;
+_slavePort = 
 Port::unbind();
 }

@@ -108,8 +166,8 @@
  * Slave port
  */
 SlavePort::SlavePort(const std::string& name, SimObject* _owner, PortID id)
-: Port(name, id), _masterPort(NULL), defaultBackdoorWarned(false),
-owner(*_owner)
+: Port(name, id), _masterPort(),
+defaultBackdoorWarned(false), owner(*_owner)
 {
 }

@@ -120,7 +178,7 @@
 void
 SlavePort::slaveUnbind()
 {
-_masterPort = NULL;
+_masterPort = 
 Port::unbind();
 }

diff --git a/src/mem/port.hh b/src/mem/port.hh
index d84e46f..eadf7f4 100644
--- a/src/mem/port.hh
+++ b/src/mem/port.hh
@@ -258,6 +258,7 @@

   private:
 MasterPort* _masterPort;
+
 bool defaultBackdoorWarned;

   protected:
@@ -278,13 +279,7 @@
 /**
  * Called by the owner to send a range change
  */
-void
-sendRangeChange() const
-{
-fatal_if(!_masterPort,
-"%s cannot sendRangeChange() without master port.",  
name());

-_masterPort->recvRangeChange();
-}
+void sendRangeChange() const { _masterPort->recvRangeChange(); }

 /**
  * Get a list of the non-overlapping address ranges the owner is
@@ -316,7 +311,11 @@
 Tick
 sendAtomicSnoop(PacketPtr pkt)
 {
-return AtomicResponseProtocol::sendSnoop(_masterPort, pkt);
+try {
+return AtomicResponseProtocol::sendSnoop(_masterPort, pkt);
+} catch (UnboundPortException)