From 599d2c91f88ba74098a917eb0b7f901add49276a Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 22 Mar 2019 14:23:21 -0700 Subject: [PATCH] mem: Clean up the xbars a little. Get rid of comments which just restate the code, get rid of redundant "virtual" keywords, add "override"s, fix style, and get rid of xbar::init which was empty and hiding the parent class init. Change-Id: I8ce20abee340baa88084d142f2fb8c633ee54ba9 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/17592 Reviewed-by: Gabe Black Maintainer: Gabe Black --- src/mem/coherent_xbar.cc | 8 +- src/mem/coherent_xbar.hh | 161 +++++++++++++++--------------------- src/mem/noncoherent_xbar.hh | 99 +++++++++------------- src/mem/xbar.cc | 29 +++---- src/mem/xbar.hh | 55 ++++++------ 5 files changed, 149 insertions(+), 203 deletions(-) diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc index 2bee5bc9d..3e994dd71 100644 --- a/src/mem/coherent_xbar.cc +++ b/src/mem/coherent_xbar.cc @@ -71,8 +71,8 @@ CoherentXBar::CoherentXBar(const CoherentXBarParams *p) masterPorts.push_back(bp); reqLayers.push_back(new ReqLayer(*bp, *this, csprintf(".reqLayer%d", i))); - snoopLayers.push_back(new SnoopRespLayer(*bp, *this, - csprintf(".snoopLayer%d", i))); + snoopLayers.push_back( + new SnoopRespLayer(*bp, *this, csprintf(".snoopLayer%d", i))); } // see if we have a default slave device connected and if so add @@ -81,10 +81,10 @@ CoherentXBar::CoherentXBar(const CoherentXBarParams *p) defaultPortID = masterPorts.size(); std::string portName = name() + ".default"; MasterPort* bp = new CoherentXBarMasterPort(portName, *this, - defaultPortID); + defaultPortID); masterPorts.push_back(bp); reqLayers.push_back(new ReqLayer(*bp, *this, csprintf(".reqLayer%d", - defaultPortID))); + defaultPortID))); snoopLayers.push_back(new SnoopRespLayer(*bp, *this, csprintf(".snoopLayer%d", defaultPortID))); diff --git a/src/mem/coherent_xbar.hh b/src/mem/coherent_xbar.hh index 79777b998..2bb5db281 100644 --- a/src/mem/coherent_xbar.hh +++ b/src/mem/coherent_xbar.hh @@ -108,35 +108,35 @@ class CoherentXBar : public BaseXBar protected: - /** - * When receiving a timing request, pass it to the crossbar. - */ - virtual bool recvTimingReq(PacketPtr pkt) - { return xbar.recvTimingReq(pkt, id); } + bool + recvTimingReq(PacketPtr pkt) override + { + return xbar.recvTimingReq(pkt, id); + } - /** - * When receiving a timing snoop response, pass it to the crossbar. - */ - virtual bool recvTimingSnoopResp(PacketPtr pkt) - { return xbar.recvTimingSnoopResp(pkt, id); } + bool + recvTimingSnoopResp(PacketPtr pkt) override + { + return xbar.recvTimingSnoopResp(pkt, id); + } - /** - * When receiving an atomic request, pass it to the crossbar. - */ - virtual Tick recvAtomic(PacketPtr pkt) - { return xbar.recvAtomic(pkt, id); } + Tick + recvAtomic(PacketPtr pkt) override + { + return xbar.recvAtomic(pkt, id); + } - /** - * When receiving a functional request, pass it to the crossbar. - */ - virtual void recvFunctional(PacketPtr pkt) - { xbar.recvFunctional(pkt, id); } + void + recvFunctional(PacketPtr pkt) override + { + xbar.recvFunctional(pkt, id); + } - /** - * Return the union of all adress ranges seen by this crossbar. - */ - virtual AddrRangeList getAddrRanges() const - { return xbar.getAddrRanges(); } + AddrRangeList + getAddrRanges() const override + { + return xbar.getAddrRanges(); + } }; @@ -166,42 +166,34 @@ class CoherentXBar : public BaseXBar * * @return a boolean that is true if this port is snooping */ - virtual bool isSnooping() const - { return true; } + bool isSnooping() const override { return true; } - /** - * When receiving a timing response, pass it to the crossbar. - */ - virtual bool recvTimingResp(PacketPtr pkt) - { return xbar.recvTimingResp(pkt, id); } - - /** - * When receiving a timing snoop request, pass it to the crossbar. - */ - virtual void recvTimingSnoopReq(PacketPtr pkt) - { return xbar.recvTimingSnoopReq(pkt, id); } + bool + recvTimingResp(PacketPtr pkt) override + { + return xbar.recvTimingResp(pkt, id); + } - /** - * When receiving an atomic snoop request, pass it to the crossbar. - */ - virtual Tick recvAtomicSnoop(PacketPtr pkt) - { return xbar.recvAtomicSnoop(pkt, id); } + void + recvTimingSnoopReq(PacketPtr pkt) override + { + return xbar.recvTimingSnoopReq(pkt, id); + } - /** - * When receiving a functional snoop request, pass it to the crossbar. - */ - virtual void recvFunctionalSnoop(PacketPtr pkt) - { xbar.recvFunctionalSnoop(pkt, id); } + Tick + recvAtomicSnoop(PacketPtr pkt) override + { + return xbar.recvAtomicSnoop(pkt, id); + } - /** When reciving a range change from the peer port (at id), - pass it to the crossbar. */ - virtual void recvRangeChange() - { xbar.recvRangeChange(id); } + void + recvFunctionalSnoop(PacketPtr pkt) override + { + xbar.recvFunctionalSnoop(pkt, id); + } - /** When reciving a retry from the peer port (at id), - pass it to the crossbar. */ - virtual void recvReqRetry() - { xbar.recvReqRetry(id); } + void recvRangeChange() override { xbar.recvRangeChange(id); } + void recvReqRetry() override { xbar.recvReqRetry(id); } }; @@ -231,23 +223,23 @@ class CoherentXBar : public BaseXBar * Override the sending of retries and pass them on through * the mirrored slave port. */ - void sendRetryResp() { + void + sendRetryResp() override + { // forward it as a snoop response retry slavePort.sendRetrySnoopResp(); } - /** - * Provided as necessary. - */ - void recvReqRetry() { panic("SnoopRespPort should never see retry\n"); } + void + recvReqRetry() override + { + panic("SnoopRespPort should never see retry"); + } - /** - * Provided as necessary. - */ - bool recvTimingResp(PacketPtr pkt) + bool + recvTimingResp(PacketPtr pkt) override { - panic("SnoopRespPort should never see timing response\n"); - return false; + panic("SnoopRespPort should never see timing response"); } }; @@ -280,13 +272,8 @@ class CoherentXBar : public BaseXBar * broadcast needed for probes. NULL denotes an absent filter. */ SnoopFilter *snoopFilter; - /** Cycles of snoop response latency.*/ const Cycles snoopResponseLatency; - - /** Is this crossbar the point of coherency? **/ const bool pointOfCoherency; - - /** Is this crossbar the point of unification? **/ const bool pointOfUnification; /** @@ -295,24 +282,10 @@ class CoherentXBar : public BaseXBar */ std::unique_ptr pendingDelete; - /** Function called by the port when the crossbar is recieving a Timing - request packet.*/ bool recvTimingReq(PacketPtr pkt, PortID slave_port_id); - - /** Function called by the port when the crossbar is recieving a Timing - response packet.*/ bool recvTimingResp(PacketPtr pkt, PortID master_port_id); - - /** Function called by the port when the crossbar is recieving a timing - snoop request.*/ void recvTimingSnoopReq(PacketPtr pkt, PortID master_port_id); - - /** Function called by the port when the crossbar is recieving a timing - snoop response.*/ bool recvTimingSnoopResp(PacketPtr pkt, PortID slave_port_id); - - /** Timing function called by port when it is once again able to process - * requests. */ void recvReqRetry(PortID master_port_id); /** @@ -323,7 +296,9 @@ class CoherentXBar : public BaseXBar * @param pkt Packet to forward * @param exclude_slave_port_id Id of slave port to exclude */ - void forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id) { + void + forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id) + { forwardTiming(pkt, exclude_slave_port_id, snoopPorts); } @@ -339,12 +314,7 @@ class CoherentXBar : public BaseXBar void forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id, const std::vector& dests); - /** Function called by the port when the crossbar is recieving a Atomic - transaction.*/ Tick recvAtomic(PacketPtr pkt, PortID slave_port_id); - - /** Function called by the port when the crossbar is recieving an - atomic snoop transaction.*/ Tick recvAtomicSnoop(PacketPtr pkt, PortID master_port_id); /** @@ -357,8 +327,8 @@ class CoherentXBar : public BaseXBar * * @return a pair containing the snoop response and snoop latency */ - std::pair forwardAtomic(PacketPtr pkt, - PortID exclude_slave_port_id) + std::pair + forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id) { return forwardAtomic(pkt, exclude_slave_port_id, InvalidPortID, snoopPorts); @@ -423,7 +393,8 @@ class CoherentXBar : public BaseXBar * * @return Whether the memory below is the destination for the packet */ - bool isDestination(const PacketPtr pkt) const + bool + isDestination(const PacketPtr pkt) const { return (pkt->req->isToPOC() && pointOfCoherency) || (pkt->req->isToPOU() && pointOfUnification); diff --git a/src/mem/noncoherent_xbar.hh b/src/mem/noncoherent_xbar.hh index 4ff1ec4fc..9d4efbee2 100644 --- a/src/mem/noncoherent_xbar.hh +++ b/src/mem/noncoherent_xbar.hh @@ -104,30 +104,29 @@ class NoncoherentXBar : public BaseXBar protected: - /** - * When receiving a timing request, pass it to the crossbar. - */ - virtual bool recvTimingReq(PacketPtr pkt) - { return xbar.recvTimingReq(pkt, id); } - - /** - * When receiving an atomic request, pass it to the crossbar. - */ - virtual Tick recvAtomic(PacketPtr pkt) - { return xbar.recvAtomic(pkt, id); } - - /** - * When receiving a functional request, pass it to the crossbar. - */ - virtual void recvFunctional(PacketPtr pkt) - { xbar.recvFunctional(pkt, id); } - - /** - * Return the union of all adress ranges seen by this crossbar. - */ - virtual AddrRangeList getAddrRanges() const - { return xbar.getAddrRanges(); } - + bool + recvTimingReq(PacketPtr pkt) override + { + return xbar.recvTimingReq(pkt, id); + } + + Tick + recvAtomic(PacketPtr pkt) override + { + return xbar.recvAtomic(pkt, id); + } + + void + recvFunctional(PacketPtr pkt) override + { + xbar.recvFunctional(pkt, id); + } + + AddrRangeList + getAddrRanges() const override + { + return xbar.getAddrRanges(); + } }; /** @@ -151,42 +150,29 @@ class NoncoherentXBar : public BaseXBar protected: - /** - * When receiving a timing response, pass it to the crossbar. - */ - virtual bool recvTimingResp(PacketPtr pkt) - { return xbar.recvTimingResp(pkt, id); } - - /** When reciving a range change from the peer port (at id), - pass it to the crossbar. */ - virtual void recvRangeChange() - { xbar.recvRangeChange(id); } - - /** When reciving a retry from the peer port (at id), - pass it to the crossbar. */ - virtual void recvReqRetry() - { xbar.recvReqRetry(id); } - + bool + recvTimingResp(PacketPtr pkt) override + { + return xbar.recvTimingResp(pkt, id); + } + + void + recvRangeChange() override + { + xbar.recvRangeChange(id); + } + + void + recvReqRetry() override + { + xbar.recvReqRetry(id); + } }; - /** Function called by the port when the crossbar is recieving a Timing - request packet.*/ virtual bool recvTimingReq(PacketPtr pkt, PortID slave_port_id); - - /** Function called by the port when the crossbar is recieving a Timing - response packet.*/ virtual bool recvTimingResp(PacketPtr pkt, PortID master_port_id); - - /** Timing function called by port when it is once again able to process - * requests. */ void recvReqRetry(PortID master_port_id); - - /** Function called by the port when the crossbar is recieving a Atomic - transaction.*/ Tick recvAtomic(PacketPtr pkt, PortID slave_port_id); - - /** Function called by the port when the crossbar is recieving a Functional - transaction.*/ void recvFunctional(PacketPtr pkt, PortID slave_port_id); public: @@ -195,10 +181,7 @@ class NoncoherentXBar : public BaseXBar virtual ~NoncoherentXBar(); - /** - * stats - */ - virtual void regStats(); + void regStats() override; Stats::Scalar totPktSize; }; diff --git a/src/mem/xbar.cc b/src/mem/xbar.cc index 247024eff..9328c2990 100644 --- a/src/mem/xbar.cc +++ b/src/mem/xbar.cc @@ -76,11 +76,6 @@ BaseXBar::~BaseXBar() delete s; } -void -BaseXBar::init() -{ -} - Port & BaseXBar::getPort(const std::string &if_name, PortID idx) { @@ -136,7 +131,7 @@ BaseXBar::calcPacketTiming(PacketPtr pkt, Tick header_delay) } template -BaseXBar::Layer::Layer(DstType& _port, BaseXBar& _xbar, +BaseXBar::Layer::Layer(DstType& _port, BaseXBar& _xbar, const std::string& _name) : port(_port), xbar(_xbar), _name(_name), state(IDLE), waitingForPeer(NULL), releaseEvent([this]{ releaseLayer(); }, name()) @@ -144,7 +139,7 @@ BaseXBar::Layer::Layer(DstType& _port, BaseXBar& _xbar, } template -void BaseXBar::Layer::occupyLayer(Tick until) +void BaseXBar::Layer::occupyLayer(Tick until) { // ensure the state is busy at this point, as the layer should // transition from idle as soon as it has decided to forward the @@ -165,7 +160,7 @@ void BaseXBar::Layer::occupyLayer(Tick until) template bool -BaseXBar::Layer::tryTiming(SrcType* src_port) +BaseXBar::Layer::tryTiming(SrcType* src_port) { // if we are in the retry state, we will not see anything but the // retrying port (or in the case of the snoop ports the snoop @@ -196,7 +191,7 @@ BaseXBar::Layer::tryTiming(SrcType* src_port) template void -BaseXBar::Layer::succeededTiming(Tick busy_time) +BaseXBar::Layer::succeededTiming(Tick busy_time) { // we should have gone from idle or retry to busy in the tryTiming // test @@ -208,7 +203,7 @@ BaseXBar::Layer::succeededTiming(Tick busy_time) template void -BaseXBar::Layer::failedTiming(SrcType* src_port, +BaseXBar::Layer::failedTiming(SrcType* src_port, Tick busy_time) { // ensure no one got in between and tried to send something to @@ -230,7 +225,7 @@ BaseXBar::Layer::failedTiming(SrcType* src_port, template void -BaseXBar::Layer::releaseLayer() +BaseXBar::Layer::releaseLayer() { // releasing the bus means we should now be idle assert(state == BUSY); @@ -254,7 +249,7 @@ BaseXBar::Layer::releaseLayer() template void -BaseXBar::Layer::retryWaiting() +BaseXBar::Layer::retryWaiting() { // this should never be called with no one waiting assert(!waitingForLayer.empty()); @@ -289,7 +284,7 @@ BaseXBar::Layer::retryWaiting() template void -BaseXBar::Layer::recvRetry() +BaseXBar::Layer::recvRetry() { // we should never get a retry without having failed to forward // something to this port @@ -573,7 +568,7 @@ BaseXBar::regStats() template DrainState -BaseXBar::Layer::drain() +BaseXBar::Layer::drain() { //We should check that we're not "doing" anything, and that noone is //waiting. We might be idle but have someone waiting if the device we @@ -588,7 +583,7 @@ BaseXBar::Layer::drain() template void -BaseXBar::Layer::regStats() +BaseXBar::Layer::regStats() { using namespace Stats; @@ -611,5 +606,5 @@ BaseXBar::Layer::regStats() * file, but since there are only two given options (MasterPort and * SlavePort) it seems a bit excessive at this point. */ -template class BaseXBar::Layer; -template class BaseXBar::Layer; +template class BaseXBar::Layer; +template class BaseXBar::Layer; diff --git a/src/mem/xbar.hh b/src/mem/xbar.hh index 674515001..fbd17e50b 100644 --- a/src/mem/xbar.hh +++ b/src/mem/xbar.hh @@ -116,9 +116,6 @@ class BaseXBar : public MemObject */ DrainState drain() override; - /** - * Get the crossbar layer's name - */ const std::string name() const { return xbar.name() + _name; } @@ -154,7 +151,6 @@ class BaseXBar : public MemObject */ void failedTiming(SrcType* src_port, Tick busy_time); - /** Occupy the layer until until */ void occupyLayer(Tick until); /** @@ -170,9 +166,6 @@ class BaseXBar : public MemObject */ void recvRetry(); - /** - * Register stats for the layer - */ void regStats(); protected: @@ -193,7 +186,6 @@ class BaseXBar : public MemObject /** The crossbar this layer is a part of. */ BaseXBar& xbar; - /** A name for this layer. */ std::string _name; /** @@ -214,7 +206,6 @@ class BaseXBar : public MemObject */ enum State { IDLE, BUSY, RETRY }; - /** track the state of the layer */ State state; /** @@ -235,8 +226,6 @@ class BaseXBar : public MemObject * potential waiting port, or drain if asked to do so. */ void releaseLayer(); - - /** event used to schedule a release of the layer */ EventFunctionWrapper releaseEvent; /** @@ -249,7 +238,7 @@ class BaseXBar : public MemObject }; - class ReqLayer : public Layer + class ReqLayer : public Layer { public: /** @@ -260,15 +249,18 @@ class BaseXBar : public MemObject * @param _name the layer's name */ ReqLayer(MasterPort& _port, BaseXBar& _xbar, const std::string& _name) : - Layer(_port, _xbar, _name) {} + Layer(_port, _xbar, _name) + {} protected: - - void sendRetry(SlavePort* retry_port) - { retry_port->sendRetryReq(); } + void + sendRetry(SlavePort* retry_port) override + { + retry_port->sendRetryReq(); + } }; - class RespLayer : public Layer + class RespLayer : public Layer { public: /** @@ -278,16 +270,20 @@ class BaseXBar : public MemObject * @param _xbar the crossbar this layer belongs to * @param _name the layer's name */ - RespLayer(SlavePort& _port, BaseXBar& _xbar, const std::string& _name) : - Layer(_port, _xbar, _name) {} + RespLayer(SlavePort& _port, BaseXBar& _xbar, + const std::string& _name) : + Layer(_port, _xbar, _name) + {} protected: - - void sendRetry(MasterPort* retry_port) - { retry_port->sendRetryResp(); } + void + sendRetry(MasterPort* retry_port) override + { + retry_port->sendRetryResp(); + } }; - class SnoopRespLayer : public Layer + class SnoopRespLayer : public Layer { public: /** @@ -299,12 +295,16 @@ class BaseXBar : public MemObject */ SnoopRespLayer(MasterPort& _port, BaseXBar& _xbar, const std::string& _name) : - Layer(_port, _xbar, _name) {} + Layer(_port, _xbar, _name) + {} protected: - void sendRetry(SlavePort* retry_port) - { retry_port->sendRetrySnoopResp(); } + void + sendRetry(SlavePort* retry_port) override + { + retry_port->sendRetrySnoopResp(); + } }; /** @@ -312,9 +312,7 @@ class BaseXBar : public MemObject * and to decode the address. */ const Cycles frontendLatency; - /** Cycles of forward latency */ const Cycles forwardLatency; - /** Cycles of response latency */ const Cycles responseLatency; /** the width of the xbar in bytes */ const uint32_t width; @@ -417,7 +415,6 @@ class BaseXBar : public MemObject PortID idx=InvalidPortID) override; void regStats() override; - }; #endif //__MEM_XBAR_HH__ -- 2.30.2