From 6b45238316052f458ba9ebc9d24a91cfa9e41cf1 Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Sat, 21 Jun 2008 01:04:43 -0400 Subject: [PATCH] Generate more useful error messages for unconnected ports. Force all non-default ports to provide a name and an owner in the constructor. --- src/cpu/o3/fetch.hh | 4 +- src/cpu/o3/fetch_impl.hh | 2 +- src/cpu/o3/lsq.hh | 4 +- src/cpu/o3/lsq_impl.hh | 2 +- src/cpu/o3/thread_context_impl.hh | 1 - src/cpu/ozone/cpu_impl.hh | 1 - src/cpu/simple_thread.cc | 1 - src/cpu/thread_state.cc | 4 +- src/mem/bridge.cc | 2 +- src/mem/bus.cc | 7 ++-- src/mem/bus.hh | 2 +- src/mem/cache/cache.hh | 2 +- src/mem/cache/cache_impl.hh | 5 ++- src/mem/mem_object.cc | 4 +- src/mem/mem_object.hh | 10 +++-- src/mem/port.cc | 65 +++++++++++++++++++++++-------- src/mem/port.hh | 22 +++++------ 17 files changed, 84 insertions(+), 54 deletions(-) diff --git a/src/cpu/o3/fetch.hh b/src/cpu/o3/fetch.hh index d954bd1e7..3ada0328f 100644 --- a/src/cpu/o3/fetch.hh +++ b/src/cpu/o3/fetch.hh @@ -80,8 +80,8 @@ class DefaultFetch public: /** Default constructor. */ - IcachePort(DefaultFetch *_fetch) - : Port(_fetch->name() + "-iport"), fetch(_fetch) + IcachePort(DefaultFetch *_fetch, O3CPU *_cpu) + : Port(_fetch->name() + "-iport", _cpu), fetch(_fetch) { } bool snoopRangeSent; diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh index 7d344fa33..ecfbacd98 100644 --- a/src/cpu/o3/fetch_impl.hh +++ b/src/cpu/o3/fetch_impl.hh @@ -167,7 +167,7 @@ DefaultFetch::DefaultFetch(O3CPU *_cpu, Params *params) instSize = sizeof(TheISA::MachInst); // Name is finally available, so create the port. - icachePort = new IcachePort(this); + icachePort = new IcachePort(this, cpu); icachePort->snoopRangeSent = false; diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh index 06de608e0..82d73c6ee 100644 --- a/src/cpu/o3/lsq.hh +++ b/src/cpu/o3/lsq.hh @@ -296,8 +296,8 @@ class LSQ { public: /** Default constructor. */ - DcachePort(LSQ *_lsq) - : Port(_lsq->name() + "-dport"), lsq(_lsq) + DcachePort(LSQ *_lsq, O3CPU *_cpu) + : Port(_lsq->name() + "-dport", _cpu), lsq(_lsq) { } bool snoopRangeSent; diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh index 8ed6f7f54..41ab66dd0 100644 --- a/src/cpu/o3/lsq_impl.hh +++ b/src/cpu/o3/lsq_impl.hh @@ -112,7 +112,7 @@ LSQ::DcachePort::recvRetry() template LSQ::LSQ(O3CPU *cpu_ptr, IEW *iew_ptr, Params *params) - : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this), + : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this, cpu_ptr), LQEntries(params->LQEntries), SQEntries(params->SQEntries), numThreads(params->numberOfThreads), diff --git a/src/cpu/o3/thread_context_impl.hh b/src/cpu/o3/thread_context_impl.hh index 865d58635..514d3de64 100755 --- a/src/cpu/o3/thread_context_impl.hh +++ b/src/cpu/o3/thread_context_impl.hh @@ -103,7 +103,6 @@ void O3ThreadContext::delVirtPort(VirtualPort *vp) { if (vp != thread->getVirtPort()) { - vp->removeConn(); delete vp; } } diff --git a/src/cpu/ozone/cpu_impl.hh b/src/cpu/ozone/cpu_impl.hh index 0c7105382..4cb55fa23 100644 --- a/src/cpu/ozone/cpu_impl.hh +++ b/src/cpu/ozone/cpu_impl.hh @@ -747,7 +747,6 @@ template void OzoneCPU::OzoneTC::delVirtPort(VirtualPort *vp) { - vp->removeConn(); delete vp; } #endif diff --git a/src/cpu/simple_thread.cc b/src/cpu/simple_thread.cc index 5a5444de4..47b69d05f 100644 --- a/src/cpu/simple_thread.cc +++ b/src/cpu/simple_thread.cc @@ -302,7 +302,6 @@ void SimpleThread::delVirtPort(VirtualPort *vp) { if (vp != virtPort) { - vp->removeConn(); delete vp; } } diff --git a/src/cpu/thread_state.cc b/src/cpu/thread_state.cc index bcfc9c924..56839ca7f 100644 --- a/src/cpu/thread_state.cc +++ b/src/cpu/thread_state.cc @@ -126,7 +126,7 @@ ThreadState::connectPhysPort() // already existed. Fix this memory leak once the bus port IDs // for functional ports is resolved. if (physPort) - physPort->removeConn(); + physPort->disconnectFromPeer(); else physPort = new FunctionalPort(csprintf("%s-%d-funcport", baseCpu->name(), tid)); @@ -140,7 +140,7 @@ ThreadState::connectVirtPort() // already existed. Fix this memory leak once the bus port IDs // for functional ports is resolved. if (virtPort) - virtPort->removeConn(); + virtPort->disconnectFromPeer(); else virtPort = new VirtualPort(csprintf("%s-%d-vport", baseCpu->name(), tid)); diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc index c09cacc00..25c6e5d19 100644 --- a/src/mem/bridge.cc +++ b/src/mem/bridge.cc @@ -47,7 +47,7 @@ Bridge::BridgePort::BridgePort(const std::string &_name, int _delay, int _nack_delay, int _req_limit, int _resp_limit, std::vector > filter_ranges) - : Port(_name), bridge(_bridge), otherPort(_otherPort), + : Port(_name, _bridge), bridge(_bridge), otherPort(_otherPort), delay(_delay), nackDelay(_nack_delay), filterRanges(filter_ranges), outstandingResponses(0), queuedRequests(0), inRetry(false), reqQueueLimit(_req_limit), respQueueLimit(_resp_limit), sendEvent(this) diff --git a/src/mem/bus.cc b/src/mem/bus.cc index 3bf1c6cfc..9dd3353fd 100644 --- a/src/mem/bus.cc +++ b/src/mem/bus.cc @@ -72,8 +72,8 @@ Bus::getPort(const std::string &if_name, int idx) return bp; } -void -Bus::deletePortRefs(Port *p) +bool +Bus::deletePort(Port *p) { BusPort *bp = dynamic_cast(p); @@ -81,10 +81,11 @@ Bus::deletePortRefs(Port *p) panic("Couldn't convert Port* to BusPort*\n"); // If this is our one functional port if (funcPort == bp) - return; + return false; interfaces.erase(bp->getId()); clearBusCache(); delete bp; + return true; } /** Get the ranges of anyone other buses that we are connected to. */ diff --git a/src/mem/bus.hh b/src/mem/bus.hh index 74901d626..7d476bb65 100644 --- a/src/mem/bus.hh +++ b/src/mem/bus.hh @@ -364,7 +364,7 @@ class Bus : public MemObject /** A function used to return the port associated with this bus object. */ virtual Port *getPort(const std::string &if_name, int idx = -1); - virtual void deletePortRefs(Port *p); + virtual bool deletePort(Port *p); virtual void init(); virtual void startup(); diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index 88c9deb7b..56ae811a3 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -217,7 +217,7 @@ class Cache : public BaseCache Cache(const Params *p, TagStore *tags, BasePrefetcher *prefetcher); virtual Port *getPort(const std::string &if_name, int idx = -1); - virtual void deletePortRefs(Port *p); + virtual bool deletePort(Port *p); void regStats(); diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 3b56c0a2e..a3dae7b2a 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -105,13 +105,14 @@ Cache::getPort(const std::string &if_name, int idx) } template -void -Cache::deletePortRefs(Port *p) +bool +Cache::deletePort(Port *p) { if (cpuSidePort == p || memSidePort == p) panic("Can only delete functional ports\n"); delete p; + return true; } diff --git a/src/mem/mem_object.cc b/src/mem/mem_object.cc index ce2a1107e..7d27db763 100644 --- a/src/mem/mem_object.cc +++ b/src/mem/mem_object.cc @@ -43,8 +43,8 @@ MemObject::makeParams(const std::string &name) return params; } -void -MemObject::deletePortRefs(Port *p) +bool +MemObject::deletePort(Port *p) { panic("This object does not support port deletion\n"); } diff --git a/src/mem/mem_object.hh b/src/mem/mem_object.hh index 33b56dfd4..a91ea5ac4 100644 --- a/src/mem/mem_object.hh +++ b/src/mem/mem_object.hh @@ -64,9 +64,13 @@ class MemObject : public SimObject /** Additional function to return the Port of a memory object. */ virtual Port *getPort(const std::string &if_name, int idx = -1) = 0; - /** Tell object that this port is about to disappear, so it should remove it - * from any structures that it's keeping it in. */ - virtual void deletePortRefs(Port *p) ; + /** Tell MemObject that this port is no longer in use, so it + * should remove it from any structures that it's keeping it in. + * If the port was allocated dynamically for this connection, it + * should be deleted here. + * @return True if the port was deleted, false if it still exists. + */ + virtual bool deletePort(Port *p); }; #endif //__MEM_MEM_OBJECT_HH__ diff --git a/src/mem/port.cc b/src/mem/port.cc index 0e03194c9..1e6989750 100644 --- a/src/mem/port.cc +++ b/src/mem/port.cc @@ -39,17 +39,25 @@ #include "mem/mem_object.hh" #include "mem/port.hh" +/** + * Special class for port objects that are used as peers for + * unconnected ports. Assigning instances of this class to newly + * allocated ports allows us to guarantee that every port has a peer + * object (so there's no need to check for null peer pointers), while + * catching uses of unconnected ports. + */ class DefaultPeerPort : public Port { protected: void blowUp() { - fatal("%s: Unconnected port!", peer->name()); + Port *peer = getPeer(); + fatal("unconnected port: %s", peer ? peer->name() : ""); } public: - DefaultPeerPort() - : Port("default_port") + DefaultPeerPort(Port *_peer) + : Port("default_port", NULL, _peer) { } bool recvTiming(PacketPtr) @@ -88,36 +96,59 @@ class DefaultPeerPort : public Port bool isDefaultPort() const { return true; } }; -DefaultPeerPort defaultPeerPort; -Port::Port() - : peer(&defaultPeerPort), owner(NULL) +Port::Port(const std::string &_name, MemObject *_owner, Port *_peer) : + portName(_name), + peer(_peer ? _peer : new DefaultPeerPort(this)), + owner(_owner) { } -Port::Port(const std::string &_name, MemObject *_owner) - : portName(_name), peer(&defaultPeerPort), owner(_owner) +Port::~Port() { + disconnectFromPeer(); } -Port::~Port() +void +Port::disconnectFromPeer() { + if (peer) { + assert(peer->getPeer() == this); + peer->disconnect(); + } } void -Port::setPeer(Port *port) +Port::disconnect() { - DPRINTF(Config, "setting peer to %s\n", port->name()); - - peer = port; + // This notification should come only from our peer, so we must + // have one, + assert(peer != NULL); + // We must clear 'peer' here, else if owner->deletePort() calls + // delete on us then we'll recurse infinitely through the Port + // destructor. + peer = NULL; + // If owner->deletePort() returns true, then we've been deleted, + // so don't do anything but get out of here. If not, reset peer + // pointer to a DefaultPeerPort. + if (!(owner && owner->deletePort(this))) + peer = new DefaultPeerPort(this); } void -Port::removeConn() +Port::setPeer(Port *port) { - if (peer->getOwner()) - peer->getOwner()->deletePortRefs(peer); - peer = NULL; + DPRINTF(Config, "setting peer to %s, old peer %s\n", + port->name(), peer ? peer->name() : ""); + + // You'd think we'd want to disconnect from the previous peer + // here, but it turns out that with some functional ports the old + // peer keeps using the connection, and it works because + // functional ports are unidirectional. + // + // disconnectFromPeer(); + + peer = port; } void diff --git a/src/mem/port.hh b/src/mem/port.hh index 15fda2164..4e0d91e75 100644 --- a/src/mem/port.hh +++ b/src/mem/port.hh @@ -87,17 +87,14 @@ class Port public: - Port(); - /** * Constructor. * * @param _name Port name for DPRINTF output. Should include name * of memory system object to which the port belongs. * @param _owner Pointer to the MemObject that owns this port. - * Will not necessarily be set. */ - Port(const std::string &_name, MemObject *_owner = NULL); + Port(const std::string &_name, MemObject *_owner, Port *_peer = NULL); /** Return port name (for DPRINTF). */ const std::string &name() const { return portName; } @@ -111,24 +108,18 @@ class Port RangeChange }; - void setName(const std::string &name) - { portName = name; } - /** Function to set the pointer for the peer port. */ virtual void setPeer(Port *port); /** Function to get the pointer to the peer port. */ Port *getPeer() { return peer; } - /** Function to set the owner of this port. */ - void setOwner(MemObject *_owner) { owner = _owner; } - /** Function to return the owner of this port. */ MemObject *getOwner() { return owner; } - /** Inform the peer port to delete itself and notify it's owner about it's - * demise. */ - void removeConn(); + /** Notify my peer port that I'm disconnecting (by calling its + * disconnect() method) so it can clean up. */ + void disconnectFromPeer(); virtual bool isDefaultPort() const { return false; } @@ -254,6 +245,11 @@ class Port /** Internal helper function for read/writeBlob(). */ void blobHelper(Addr addr, uint8_t *p, int size, MemCmd cmd); + + /** Receive notification that my peer is disconnecting and clean + * up (potentially deleting myself in the process). Should be + * called only from peer's disconnectFromPeer(). */ + void disconnect(); }; /** A simple functional port that is only meant for one way communication to -- 2.30.2