I'd actually like to see this changset go back in.  It seems that all
that needs to happen is that setPeer should delete the peer if it is a
defaultPort.  There are a couple of ways to do this:
1) Have setPeer call disconnect(), though I'm not clear on how the
deletePort() thing is supposed to work
2) in setPeer, dynamic_cast<DefaultPort *>(this) and delete it if it isn't null
3) create a new virtual function on Port that does nothing, and
override it to delete this in DefaultPort.  Call this function from
setPeer.

Anyway, Steve, since you're the most familiar with this code, I'd
prefer you put it back in, but I would be handy to have soon.

  Nate


> changeset 85c8d296c1cb in /z/repo/m5
> details: http://repo.m5sim.org/m5?cmd=changeset;node=85c8d296c1cb
> summary: Backed out changeset 94a7bb476fca: caused memory leak.
>
> changeset 0d9fac06c402 in /z/repo/m5
> details: http://repo.m5sim.org/m5?cmd=changeset;node=0d9fac06c402
> summary: Automated merge after backout.
>
> diffstat:
>
> 11 files changed, 7 insertions(+), 17 deletions(-)
> src/cpu/o3/fetch.hh          |    1 -
> src/cpu/o3/lsq.hh            |    1 -
> src/mem/bus.cc               |    1 -
> src/mem/cache/cache_impl.hh  |    4 ++--
> src/mem/mem_object.cc        |    1 -
> src/mem/mem_object.hh        |    3 ---
> src/mem/port.cc              |    7 -------
> src/mem/port.hh              |    2 ++
> src/python/swig/event.i      |    1 -
> src/sim/sim_object.hh        |    2 ++
> src/sim/sim_object_params.hh |    1 +
>
> diffs (truncated from 453 to 300 lines):
>
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/fetch.hh
> --- a/src/cpu/o3/fetch.hh       Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/o3/fetch.hh       Sat Jun 28 13:20:00 2008 -0400
> @@ -80,8 +80,8 @@ class DefaultFetch
>
>       public:
>         /** Default constructor. */
> -        IcachePort(DefaultFetch<Impl> *_fetch, O3CPU *_cpu)
> -            : Port(_fetch->name() + "-iport", _cpu), fetch(_fetch)
> +        IcachePort(DefaultFetch<Impl> *_fetch)
> +            : Port(_fetch->name() + "-iport"), fetch(_fetch)
>         { }
>
>         bool snoopRangeSent;
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/fetch_impl.hh
> --- a/src/cpu/o3/fetch_impl.hh  Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/o3/fetch_impl.hh  Sat Jun 28 13:20:00 2008 -0400
> @@ -167,7 +167,7 @@ DefaultFetch<Impl>::DefaultFetch(O3CPU *
>     instSize = sizeof(TheISA::MachInst);
>
>     // Name is finally available, so create the port.
> -    icachePort = new IcachePort(this, cpu);
> +    icachePort = new IcachePort(this);
>
>     icachePort->snoopRangeSent = false;
>
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/lsq.hh
> --- a/src/cpu/o3/lsq.hh Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/o3/lsq.hh Sat Jun 28 13:20:00 2008 -0400
> @@ -296,8 +296,8 @@ class LSQ {
>
>       public:
>         /** Default constructor. */
> -        DcachePort(LSQ *_lsq, O3CPU *_cpu)
> -            : Port(_lsq->name() + "-dport", _cpu), lsq(_lsq)
> +        DcachePort(LSQ *_lsq)
> +            : Port(_lsq->name() + "-dport"), lsq(_lsq)
>         { }
>
>         bool snoopRangeSent;
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/lsq_impl.hh
> --- a/src/cpu/o3/lsq_impl.hh    Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/o3/lsq_impl.hh    Sat Jun 28 13:20:00 2008 -0400
> @@ -112,7 +112,7 @@ LSQ<Impl>::DcachePort::recvRetry()
>
>  template <class Impl>
>  LSQ<Impl>::LSQ(O3CPU *cpu_ptr, IEW *iew_ptr, Params *params)
> -    : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this, cpu_ptr),
> +    : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this),
>       LQEntries(params->LQEntries),
>       SQEntries(params->SQEntries),
>       numThreads(params->numberOfThreads),
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/thread_context_impl.hh
> --- a/src/cpu/o3/thread_context_impl.hh Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/o3/thread_context_impl.hh Sat Jun 28 13:20:00 2008 -0400
> @@ -103,6 +103,7 @@ O3ThreadContext<Impl>::delVirtPort(Virtu
>  O3ThreadContext<Impl>::delVirtPort(VirtualPort *vp)
>  {
>     if (vp != thread->getVirtPort()) {
> +        vp->removeConn();
>         delete vp;
>     }
>  }
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/ozone/cpu_impl.hh
> --- a/src/cpu/ozone/cpu_impl.hh Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/ozone/cpu_impl.hh Sat Jun 28 13:20:00 2008 -0400
> @@ -747,6 +747,7 @@ void
>  void
>  OzoneCPU<Impl>::OzoneTC::delVirtPort(VirtualPort *vp)
>  {
> +    vp->removeConn();
>     delete vp;
>  }
>  #endif
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/simple_thread.cc
> --- a/src/cpu/simple_thread.cc  Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/simple_thread.cc  Sat Jun 28 13:20:00 2008 -0400
> @@ -302,6 +302,7 @@ SimpleThread::delVirtPort(VirtualPort *v
>  SimpleThread::delVirtPort(VirtualPort *vp)
>  {
>     if (vp != virtPort) {
> +        vp->removeConn();
>         delete vp;
>     }
>  }
> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/thread_state.cc
> --- a/src/cpu/thread_state.cc   Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/cpu/thread_state.cc   Sat Jun 28 13:20:00 2008 -0400
> @@ -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->disconnectFromPeer();
> +        physPort->removeConn();
>     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->disconnectFromPeer();
> +        virtPort->removeConn();
>     else
>         virtPort = new VirtualPort(csprintf("%s-%d-vport",
>                                         baseCpu->name(), tid));
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/bridge.cc
> --- a/src/mem/bridge.cc Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/bridge.cc Sat Jun 28 13:20:00 2008 -0400
> @@ -47,7 +47,7 @@ Bridge::BridgePort::BridgePort(const std
>                                int _delay, int _nack_delay, int _req_limit,
>                                int _resp_limit,
>                                std::vector<Range<Addr> > filter_ranges)
> -    : Port(_name, _bridge), bridge(_bridge), otherPort(_otherPort),
> +    : Port(_name), 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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/bus.cc
> --- a/src/mem/bus.cc    Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/bus.cc    Sat Jun 28 13:20:00 2008 -0400
> @@ -72,8 +72,8 @@ Bus::getPort(const std::string &if_name,
>     return bp;
>  }
>
> -bool
> -Bus::deletePort(Port *p)
> +void
> +Bus::deletePortRefs(Port *p)
>  {
>
>     BusPort *bp =  dynamic_cast<BusPort*>(p);
> @@ -81,11 +81,10 @@ Bus::deletePort(Port *p)
>         panic("Couldn't convert Port* to BusPort*\n");
>     // If this is our one functional port
>     if (funcPort == bp)
> -        return false;
> +        return;
>     interfaces.erase(bp->getId());
>     clearBusCache();
>     delete bp;
> -    return true;
>  }
>
>  /** Get the ranges of anyone other buses that we are connected to. */
> @@ -524,9 +523,12 @@ Bus::recvStatusChange(Port::Status statu
>         for (iter = ranges.begin(); iter != ranges.end(); iter++) {
>             DPRINTF(BusAddrRanges, "Adding range %#llx - %#llx for id %d\n",
>                     iter->start, iter->end, id);
> -            if (portMap.insert(*iter, id) == portMap.end())
> -                panic("Two devices with same range\n");
> -
> +            if (portMap.insert(*iter, id) == portMap.end()) {
> +                int conflict_id = portMap.find(*iter)->second;
> +                fatal("%s has two ports with same range:\n\t%s\n\t%s\n",
> +                      name(), interfaces[id]->getPeer()->name(),
> +                      interfaces[conflict_id]->getPeer()->name());
> +            }
>         }
>     }
>     DPRINTF(MMU, "port list has %d entries\n", portMap.size());
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/bus.hh
> --- a/src/mem/bus.hh    Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/bus.hh    Sat Jun 28 13:20:00 2008 -0400
> @@ -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 bool deletePort(Port *p);
> +    virtual void deletePortRefs(Port *p);
>
>     virtual void init();
>     virtual void startup();
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/cache/cache.hh
> --- a/src/mem/cache/cache.hh    Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/cache/cache.hh    Sat Jun 28 13:20:00 2008 -0400
> @@ -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 bool deletePort(Port *p);
> +    virtual void deletePortRefs(Port *p);
>
>     void regStats();
>
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/cache/cache_impl.hh
> --- a/src/mem/cache/cache_impl.hh       Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/cache/cache_impl.hh       Sat Jun 28 13:20:00 2008 -0400
> @@ -105,14 +105,13 @@ Cache<TagStore>::getPort(const std::stri
>  }
>
>  template<class TagStore>
> -bool
> -Cache<TagStore>::deletePort(Port *p)
> +void
> +Cache<TagStore>::deletePortRefs(Port *p)
>  {
>     if (cpuSidePort == p || memSidePort == p)
>         panic("Can only delete functional ports\n");
>
>     delete p;
> -    return true;
>  }
>
>
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/mem_object.cc
> --- a/src/mem/mem_object.cc     Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/mem_object.cc     Sat Jun 28 13:20:00 2008 -0400
> @@ -43,8 +43,8 @@ MemObject::makeParams(const std::string
>     return params;
>  }
>
> -bool
> -MemObject::deletePort(Port *p)
> +void
> +MemObject::deletePortRefs(Port *p)
>  {
>     panic("This object does not support port deletion\n");
>  }
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/mem_object.hh
> --- a/src/mem/mem_object.hh     Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/mem_object.hh     Sat Jun 28 13:20:00 2008 -0400
> @@ -64,13 +64,9 @@ 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 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);
> +    /** 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) ;
>  };
>
>  #endif //__MEM_MEM_OBJECT_HH__
> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/port.cc
> --- a/src/mem/port.cc   Sat Jun 21 01:04:43 2008 -0400
> +++ b/src/mem/port.cc   Sat Jun 28 13:20:00 2008 -0400
> @@ -39,25 +39,17 @@
>  #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()
>     {
> -        Port *peer = getPeer();
> -        fatal("unconnected port: %s", peer ? peer->name() : "<unknown>");
> +        fatal("%s: Unconnected port!", peer->name());
>     }
>
>   public:
> -    DefaultPeerPort(Port *_peer)
> -        : Port("default_port", NULL, _peer)
> +    DefaultPeerPort()
> +        : Port("default_port")
>     { }
>
>     bool recvTiming(PacketPtr)
> @@ -96,59 +88,36 @@ class DefaultPeerPort : public Port
>     bool isDefaultPort() const { return true; }
>  };
>
> +DefaultPeerPort defaultPeerPort;
>
> -Port::Port(const std::string &_name, MemObject *_owner, Port *_peer) :
> -    portName(_name),
> -    peer(_peer ? _peer : new DefaultPeerPort(this)),
> -    owner(_owner)
> +Port::Port()
> +    : peer(&defaultPeerPort), owner(NULL)
> +{
> +}
> +
> +Port::Port(const std::string &_name, MemObject *_owner)
> +    : portName(_name), peer(&defaultPeerPort), owner(_owner)
>  {
>  }
>
>  Port::~Port()
>  {
> -    disconnectFromPeer();
> -}
> -
> -void
> -Port::disconnectFromPeer()
> -{
> -    if (peer) {
> _______________________________________________
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
>
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to