Packet: Unify the use of PortID in packet and port

This patch removes the Packet::NodeID typedef and unifies it with the
Port::PortId. The src and dest fields in the packet are used to hold a
port id (e.g. in the bus), and thus the two should actually be the
same.

The typedef PortID is now global (in base/types.hh) and aligned with
the ThreadID in terms of capitalisation and naming of the
InvalidPortID constant.

Before this patch, two flags were used for valid destination and
source, rather than relying on a named value (InvalidPortID), and
this is now redundant, as the src and dest field themselves are
sufficient to tell whether the current value is a valid port
identifier or not. Consequently, the VALID_SRC and VALID_DST are
removed.

As part of the cleaning up, a number of int parameters and local
variables are updated to use PortID.

Note that Ruby still has its own NodeID typedef. Furthermore, the
MemObject getMaster/SlavePort still has an int idx parameter with a
default value of -1 which should eventually change to PortID idx =
InvalidPortID.
This commit is contained in:
Andreas Hansson 2012-05-30 05:29:42 -04:00
parent 6a54f7fc5f
commit cad802761a
10 changed files with 84 additions and 89 deletions

View file

@ -97,6 +97,12 @@ const Addr MaxAddr = (Addr)-1;
typedef int16_t ThreadID;
const ThreadID InvalidThreadID = (ThreadID)-1;
/**
* Port index/ID type, and a symbolic name for an invalid port id.
*/
typedef int16_t PortID;
const PortID InvalidPortID = (PortID)-1;
class FaultBase;
template <class T> class RefCountingPtr;
typedef RefCountingPtr<FaultBase> Fault;

View file

@ -54,7 +54,7 @@ class RubyDirectedTester : public MemObject
public:
CpuPort(const std::string &_name, RubyDirectedTester *_tester,
Port::PortId _id)
PortID _id)
: MasterPort(_name, _tester, _id), tester(_tester)
{}

View file

@ -57,7 +57,7 @@ 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)
CpuPort(const std::string &_name, RubyTester *_tester, PortID _id)
: MasterPort(_name, _tester, _id), tester(_tester)
{}

View file

@ -91,7 +91,7 @@ class Bridge : public MemObject
public:
Packet::SenderState *origSenderState;
Packet::NodeID origSrc;
PortID origSrc;
RequestState(PacketPtr _pkt)
: origSenderState(_pkt->senderState),

View file

@ -57,7 +57,7 @@ Bus::Bus(const BusParams *p)
: MemObject(p), clock(p->clock),
headerCycles(p->header_cycles), width(p->width), tickNextIdle(0),
drainEvent(NULL), busIdleEvent(this), inRetry(false),
defaultPortId(Port::INVALID_PORT_ID),
defaultPortID(InvalidPortID),
useDefaultRange(p->use_default_range),
defaultBlockSize(p->block_size),
cachedBlockSize(0), cachedBlockSizeValid(false)
@ -82,9 +82,9 @@ Bus::Bus(const BusParams *p)
// see if we have a default slave device connected and if so add
// our corresponding master port
if (p->port_default_connection_count) {
defaultPortId = masterPorts.size();
defaultPortID = masterPorts.size();
std::string portName = csprintf("%s-default", name());
MasterPort* bp = new BusMasterPort(portName, this, defaultPortId);
MasterPort* bp = new BusMasterPort(portName, this, defaultPortID);
masterPorts.push_back(bp);
}
@ -105,7 +105,7 @@ Bus::getMasterPort(const std::string &if_name, int idx)
// the master port index translates directly to the vector position
return *masterPorts[idx];
} else if (if_name == "default") {
return *masterPorts[defaultPortId];
return *masterPorts[defaultPortID];
} else {
return MemObject::getMasterPort(if_name, idx);
}
@ -318,7 +318,7 @@ Bus::recvTimingSnoopReq(PacketPtr pkt)
assert(pkt->isExpressSnoop());
// forward to all snoopers
forwardTiming(pkt, Port::INVALID_PORT_ID);
forwardTiming(pkt, InvalidPortID);
// a snoop request came from a connected slave device (one of
// our master ports), and if it is not coming from the slave
@ -347,7 +347,7 @@ Bus::recvTimingSnoopResp(PacketPtr pkt)
src_port->name(), pkt->cmdString(), pkt->getAddr());
// get the destination from the packet
Packet::NodeID dest = pkt->getDest();
PortID dest = pkt->getDest();
// responses are never express snoops
assert(!pkt->isExpressSnoop());
@ -410,7 +410,7 @@ Bus::succeededTiming(Tick busy_time)
}
void
Bus::forwardTiming(PacketPtr pkt, int exclude_slave_port_id)
Bus::forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id)
{
for (SlavePortIter s = snoopPorts.begin(); s != snoopPorts.end(); ++s) {
SlavePort *p = *s;
@ -418,7 +418,7 @@ Bus::forwardTiming(PacketPtr pkt, int exclude_slave_port_id)
// (corresponding to our own slave port that is also in
// snoopPorts) and should not send it back to where it came
// from
if (exclude_slave_port_id == Port::INVALID_PORT_ID ||
if (exclude_slave_port_id == InvalidPortID ||
p->getId() != exclude_slave_port_id) {
// cache is not allowed to refuse snoop
p->sendTimingSnoopReq(pkt);
@ -481,7 +481,7 @@ Bus::retryWaiting()
}
void
Bus::recvRetry(Port::PortId id)
Bus::recvRetry(PortID id)
{
// we got a retry from a peer that we tried to send something to
// and failed, but we sent it on the account of someone else, and
@ -500,14 +500,12 @@ Bus::recvRetry(Port::PortId id)
}
}
int
PortID
Bus::findPort(Addr addr)
{
/* An interval tree would be a better way to do this. --ali. */
int dest_id;
dest_id = checkPortCache(addr);
if (dest_id != Port::INVALID_PORT_ID)
PortID dest_id = checkPortCache(addr);
if (dest_id != InvalidPortID)
return dest_id;
// Check normal port ranges
@ -524,13 +522,13 @@ Bus::findPort(Addr addr)
for (AddrRangeIter i = defaultRange.begin(); i != a_end; i++) {
if (*i == addr) {
DPRINTF(Bus, " found addr %#llx on default\n", addr);
return defaultPortId;
return defaultPortID;
}
}
} else if (defaultPortId != Port::INVALID_PORT_ID) {
} else if (defaultPortID != InvalidPortID) {
DPRINTF(Bus, "Unable to find destination for addr %#llx, "
"will use default port\n", addr);
return defaultPortId;
return defaultPortID;
}
// we should use the range for the default port and it did not
@ -560,7 +558,7 @@ Bus::recvAtomic(PacketPtr pkt)
// even if we had a snoop response, we must continue and also
// perform the actual request at the destination
int dest_id = findPort(pkt->getAddr());
PortID dest_id = findPort(pkt->getAddr());
// forward the request to the appropriate destination
Tick response_latency = masterPorts[dest_id]->sendAtomic(pkt);
@ -586,7 +584,7 @@ Bus::recvAtomicSnoop(PacketPtr pkt)
// forward to all snoopers
std::pair<MemCmd, Tick> snoop_result =
forwardAtomic(pkt, Port::INVALID_PORT_ID);
forwardAtomic(pkt, InvalidPortID);
MemCmd snoop_response_cmd = snoop_result.first;
Tick snoop_response_latency = snoop_result.second;
@ -598,12 +596,12 @@ Bus::recvAtomicSnoop(PacketPtr pkt)
}
std::pair<MemCmd, Tick>
Bus::forwardAtomic(PacketPtr pkt, int exclude_slave_port_id)
Bus::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id)
{
// the packet may be changed on snoops, record the original source
// and command to enable us to restore it between snoops so that
// additional snoops can take place properly
Packet::NodeID orig_src_id = pkt->getSrc();
PortID orig_src_id = pkt->getSrc();
MemCmd orig_cmd = pkt->cmd;
MemCmd snoop_response_cmd = MemCmd::InvalidCmd;
Tick snoop_response_latency = 0;
@ -614,7 +612,7 @@ Bus::forwardAtomic(PacketPtr pkt, int exclude_slave_port_id)
// (corresponding to our own slave port that is also in
// snoopPorts) and should not send it back to where it came
// from
if (exclude_slave_port_id == Port::INVALID_PORT_ID ||
if (exclude_slave_port_id == InvalidPortID ||
p->getId() != exclude_slave_port_id) {
Tick latency = p->sendAtomicSnoop(pkt);
// in contrast to a functional access, we have to keep on
@ -662,7 +660,7 @@ Bus::recvFunctional(PacketPtr pkt)
// there is no need to continue if the snooping has found what we
// were looking for and the packet is already a response
if (!pkt->isResponse()) {
int dest_id = findPort(pkt->getAddr());
PortID dest_id = findPort(pkt->getAddr());
masterPorts[dest_id]->sendFunctional(pkt);
}
@ -680,11 +678,11 @@ Bus::recvFunctionalSnoop(PacketPtr pkt)
}
// forward to all snoopers
forwardFunctional(pkt, Port::INVALID_PORT_ID);
forwardFunctional(pkt, InvalidPortID);
}
void
Bus::forwardFunctional(PacketPtr pkt, int exclude_slave_port_id)
Bus::forwardFunctional(PacketPtr pkt, PortID exclude_slave_port_id)
{
for (SlavePortIter s = snoopPorts.begin(); s != snoopPorts.end(); ++s) {
SlavePort *p = *s;
@ -692,7 +690,7 @@ Bus::forwardFunctional(PacketPtr pkt, int exclude_slave_port_id)
// (corresponding to our own slave port that is also in
// snoopPorts) and should not send it back to where it came
// from
if (exclude_slave_port_id == Port::INVALID_PORT_ID ||
if (exclude_slave_port_id == InvalidPortID ||
p->getId() != exclude_slave_port_id)
p->sendFunctionalSnoop(pkt);
@ -705,7 +703,7 @@ Bus::forwardFunctional(PacketPtr pkt, int exclude_slave_port_id)
/** Function called by the port when the bus is receiving a range change.*/
void
Bus::recvRangeChange(Port::PortId id)
Bus::recvRangeChange(PortID id)
{
AddrRangeList ranges;
AddrRangeIter iter;
@ -717,7 +715,7 @@ Bus::recvRangeChange(Port::PortId id)
DPRINTF(BusAddrRanges, "received RangeChange from device id %d\n", id);
clearPortCache();
if (id == defaultPortId) {
if (id == defaultPortID) {
defaultRange.clear();
// Only try to update these ranges if the user set a default responder.
if (useDefaultRange) {
@ -749,7 +747,7 @@ Bus::recvRangeChange(Port::PortId id)
DPRINTF(BusAddrRanges, "Adding range %#llx - %#llx for id %d\n",
iter->start, iter->end, id);
if (portMap.insert(*iter, id) == portMap.end()) {
int conflict_id = portMap.find(*iter)->second;
PortID conflict_id = portMap.find(*iter)->second;
fatal("%s has two ports with same range:\n\t%s\n\t%s\n",
name(), masterPorts[id]->getSlavePort().name(),
masterPorts[conflict_id]->getSlavePort().name());
@ -768,7 +766,7 @@ Bus::recvRangeChange(Port::PortId id)
}
AddrRangeList
Bus::getAddrRanges(Port::PortId id)
Bus::getAddrRanges(PortID id)
{
AddrRangeList ranges;
@ -809,14 +807,14 @@ Bus::getAddrRanges(Port::PortId id)
}
bool
Bus::isSnooping(Port::PortId id) const
Bus::isSnooping(PortID id) const
{
// in essence, answer the question if there are snooping ports
return !snoopPorts.empty();
}
unsigned
Bus::findBlockSize(Port::PortId id)
Bus::findBlockSize(PortID id)
{
if (cachedBlockSizeValid)
return cachedBlockSize;

View file

@ -66,6 +66,7 @@
class Bus : public MemObject
{
/**
* Declaration of the bus slave port type, one will be
* instantiated for each of the master interfaces connecting to
@ -80,7 +81,7 @@ class Bus : public MemObject
public:
/** Constructor for the BusSlavePort.*/
BusSlavePort(const std::string &_name, Bus *_bus, Port::PortId _id)
BusSlavePort(const std::string &_name, Bus *_bus, PortID _id)
: SlavePort(_name, _bus, _id), bus(_bus)
{ }
@ -145,7 +146,7 @@ class Bus : public MemObject
public:
/** Constructor for the BusMasterPort.*/
BusMasterPort(const std::string &_name, Bus *_bus, Port::PortId _id)
BusMasterPort(const std::string &_name, Bus *_bus, PortID _id)
: MasterPort(_name, _bus, _id), bus(_bus)
{ }
@ -213,8 +214,8 @@ class Bus : public MemObject
Event * drainEvent;
typedef range_map<Addr,int>::iterator PortIter;
range_map<Addr, int> portMap;
typedef range_map<Addr, PortID>::iterator PortIter;
range_map<Addr, PortID> portMap;
AddrRangeList defaultRange;
@ -251,7 +252,7 @@ class Bus : public MemObject
* @param pkt Packet to forward
* @param exclude_slave_port_id Id of slave port to exclude
*/
void forwardTiming(PacketPtr pkt, Port::PortId exclude_slave_port_id);
void forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id);
/**
* Determine if the bus is to be considered occupied when being
@ -293,7 +294,7 @@ class Bus : public MemObject
* @return a pair containing the snoop response and snoop latency
*/
std::pair<MemCmd, Tick> forwardAtomic(PacketPtr pkt,
Port::PortId exclude_slave_port_id);
PortID exclude_slave_port_id);
/** Function called by the port when the bus is recieving a Functional
transaction.*/
@ -311,26 +312,26 @@ class Bus : public MemObject
* @param pkt Packet to forward
* @param exclude_slave_port_id Id of slave port to exclude
*/
void forwardFunctional(PacketPtr pkt, Port::PortId exclude_slave_port_id);
void forwardFunctional(PacketPtr pkt, PortID exclude_slave_port_id);
/** Timing function called by port when it is once again able to process
* requests. */
void recvRetry(Port::PortId id);
void recvRetry(PortID id);
/** Function called by the port when the bus is recieving a range change.*/
void recvRangeChange(Port::PortId id);
void recvRangeChange(PortID id);
/** Find which port connected to this bus (if any) should be given a packet
* with this address.
* @param addr Address to find port for.
* @return id of port that the packet should be sent out of.
*/
int findPort(Addr addr);
PortID findPort(Addr addr);
// Cache for the findPort function storing recently used ports from portMap
struct PortCache {
bool valid;
Port::PortId id;
PortID id;
Addr start;
Addr end;
};
@ -339,7 +340,7 @@ class Bus : public MemObject
// Checks the cache and returns the id of the port that has the requested
// address within its range
inline int checkPortCache(Addr addr) {
inline PortID checkPortCache(Addr addr) {
if (portCache[0].valid && addr >= portCache[0].start &&
addr < portCache[0].end) {
return portCache[0].id;
@ -353,7 +354,7 @@ class Bus : public MemObject
return portCache[2].id;
}
return Port::INVALID_PORT_ID;
return InvalidPortID;
}
// Clears the earliest entry of the cache and inserts a new port entry
@ -388,7 +389,7 @@ class Bus : public MemObject
*
* @return a list of non-overlapping address ranges
*/
AddrRangeList getAddrRanges(Port::PortId id);
AddrRangeList getAddrRanges(PortID id);
/**
* Determine if the bus port is snooping or not.
@ -397,7 +398,7 @@ class Bus : public MemObject
*
* @return a boolean indicating if this port is snooping or not
*/
bool isSnooping(Port::PortId id) const;
bool isSnooping(PortID id) const;
/** Calculate the timing parameters for the packet. Updates the
* firstWordTime and finishTime fields of the packet object.
@ -426,13 +427,13 @@ class Bus : public MemObject
* @param id id of the busport that made the request
* @return the max of all the sizes
*/
unsigned findBlockSize(Port::PortId id);
unsigned findBlockSize(PortID id);
// event used to schedule a release of the bus
EventWrapper<Bus, &Bus::releaseBus> busIdleEvent;
bool inRetry;
std::set<Port::PortId> inRecvRangeChange;
std::set<PortID> inRecvRangeChange;
/** The master and slave ports of the bus */
std::vector<SlavePort*> slavePorts;
@ -465,7 +466,7 @@ class Bus : public MemObject
}
/** Port that handles requests that don't match any of the interfaces.*/
short defaultPortId;
PortID defaultPortID;
/** If true, use address range provided by default device. Any
address not handled by another port and not in default device's

View file

@ -352,7 +352,7 @@ Cache<TagStore>::access(PacketPtr pkt, BlkType *&blk,
class ForwardResponseRecord : public Packet::SenderState, public FastAlloc
{
Packet::SenderState *prevSenderState;
Packet::NodeID prevSrc;
PortID prevSrc;
#ifndef NDEBUG
BaseCache *cache;
#endif
@ -1193,7 +1193,7 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
pkt->assertShared();
}
} else {
Packet::NodeID origSrc = pkt->getSrc();
PortID origSrc = pkt->getSrc();
cpuSidePort->sendAtomicSnoop(pkt);
if (!alreadyResponded && pkt->memInhibitAsserted()) {
// cache-to-cache response from some upper cache:

View file

@ -41,6 +41,7 @@
* Authors: Ron Dreslinski
* Steve Reinhardt
* Ali Saidi
* Andreas Hansson
*/
/**
@ -231,7 +232,6 @@ class Packet : public FastAlloc, public Printable
public:
typedef uint32_t FlagsType;
typedef ::Flags<FlagsType> Flags;
typedef short NodeID;
private:
static const FlagsType PUBLIC_FLAGS = 0x00000000;
@ -250,9 +250,6 @@ class Packet : public FastAlloc, public Printable
/// Are the 'addr' and 'size' fields valid?
static const FlagsType VALID_ADDR = 0x00000100;
static const FlagsType VALID_SIZE = 0x00000200;
/// Is the 'src' field valid?
static const FlagsType VALID_SRC = 0x00000400;
static const FlagsType VALID_DST = 0x00000800;
/// Is the data pointer set to a value that shouldn't be freed
/// when the packet is destroyed?
static const FlagsType STATIC_DATA = 0x00001000;
@ -306,7 +303,7 @@ class Packet : public FastAlloc, public Printable
* for example by using an appropriate sender state. The latter is
* done in the cache and bridge.
*/
NodeID src;
PortID src;
/**
* Destination port identifier that is present on all response
@ -316,7 +313,7 @@ class Packet : public FastAlloc, public Printable
* response, and the destination is used, e.g. by the bus, to
* select the appropriate path through the interconnect.
*/
NodeID dest;
PortID dest;
/**
* The original value of the command field. Only valid when the
@ -487,21 +484,21 @@ class Packet : public FastAlloc, public Printable
bool hadBadAddress() const { return cmd == MemCmd::BadAddressError; }
void copyError(Packet *pkt) { assert(pkt->isError()); cmd = pkt->cmd; }
bool isSrcValid() { return flags.isSet(VALID_SRC); }
bool isSrcValid() const { return src != InvalidPortID; }
/// Accessor function to get the source index of the packet.
NodeID getSrc() const { assert(flags.isSet(VALID_SRC)); return src; }
PortID getSrc() const { assert(isSrcValid()); return src; }
/// Accessor function to set the source index of the packet.
void setSrc(NodeID _src) { src = _src; flags.set(VALID_SRC); }
void setSrc(PortID _src) { src = _src; }
/// Reset source field, e.g. to retransmit packet on different bus.
void clearSrc() { flags.clear(VALID_SRC); }
void clearSrc() { src = InvalidPortID; }
bool isDestValid() { return flags.isSet(VALID_DST); }
bool isDestValid() const { return dest != InvalidPortID; }
/// Accessor function for the destination index of the packet.
NodeID getDest() const { assert(flags.isSet(VALID_DST)); return dest; }
PortID getDest() const { assert(isDestValid()); return dest; }
/// Accessor function to set the destination index of the packet.
void setDest(NodeID _dest) { dest = _dest; flags.set(VALID_DST); }
void setDest(PortID _dest) { dest = _dest; }
/// Reset destination field, e.g. to turn a response into a request again.
void clearDest() { flags.clear(VALID_DST); }
void clearDest() { dest = InvalidPortID; }
Addr getAddr() const { assert(flags.isSet(VALID_ADDR)); return addr; }
unsigned getSize() const { assert(flags.isSet(VALID_SIZE)); return size; }
@ -538,6 +535,7 @@ class Packet : public FastAlloc, public Printable
*/
Packet(Request *_req, MemCmd _cmd)
: cmd(_cmd), req(_req), data(NULL),
src(InvalidPortID), dest(InvalidPortID),
bytesValidStart(0), bytesValidEnd(0),
time(curTick()), senderState(NULL)
{
@ -558,6 +556,7 @@ class Packet : public FastAlloc, public Printable
*/
Packet(Request *_req, MemCmd _cmd, int _blkSize)
: cmd(_cmd), req(_req), data(NULL),
src(InvalidPortID), dest(InvalidPortID),
bytesValidStart(0), bytesValidEnd(0),
time(curTick()), senderState(NULL)
{
@ -586,7 +585,7 @@ class Packet : public FastAlloc, public Printable
if (!clearFlags)
flags.set(pkt->flags & COPY_FLAGS);
flags.set(pkt->flags & (VALID_ADDR|VALID_SIZE|VALID_SRC|VALID_DST));
flags.set(pkt->flags & (VALID_ADDR|VALID_SIZE));
flags.set(pkt->flags & STATIC_DATA);
}
@ -644,8 +643,7 @@ class Packet : public FastAlloc, public Printable
flags.clear(EXPRESS_SNOOP);
dest = src;
flags.set(VALID_DST, flags.isSet(VALID_SRC));
flags.clear(VALID_SRC);
clearSrc();
}
void

View file

@ -50,7 +50,7 @@
#include "mem/mem_object.hh"
#include "mem/port.hh"
Port::Port(const std::string &_name, MemObject& _owner, PortId _id)
Port::Port(const std::string &_name, MemObject& _owner, PortID _id)
: portName(_name), id(_id), peer(NULL), owner(_owner)
{
}
@ -62,7 +62,7 @@ Port::~Port()
/**
* Master port
*/
MasterPort::MasterPort(const std::string& name, MemObject* owner, PortId _id)
MasterPort::MasterPort(const std::string& name, MemObject* owner, PortID _id)
: Port(name, *owner, _id), _slavePort(NULL)
{
}
@ -146,7 +146,7 @@ MasterPort::printAddr(Addr a)
/**
* Slave port
*/
SlavePort::SlavePort(const std::string& name, MemObject* owner, PortId id)
SlavePort::SlavePort(const std::string& name, MemObject* owner, PortID id)
: Port(name, *owner, id), _masterPort(NULL)
{
}

View file

@ -78,14 +78,6 @@ class MemObject;
class Port
{
public:
/** A type name for the port identifier. */
typedef int PortId;
/** A symbolic name for the absence of a port id. */
static const PortId INVALID_PORT_ID = -1;
private:
/** Descriptive name (for DPRINTF output) */
@ -95,9 +87,9 @@ class Port
/**
* A numeric identifier to distinguish ports in a vector, and set
* to INVALID_PORT_ID in case this port is not part of a vector.
* to InvalidPortID in case this port is not part of a vector.
*/
const PortId id;
const PortID id;
/** A pointer to the peer port. */
Port* peer;
@ -112,7 +104,7 @@ class Port
* @param _owner The MemObject that is the structural owner of this port
* @param _id A port identifier for vector ports
*/
Port(const std::string& _name, MemObject& _owner, PortId _id);
Port(const std::string& _name, MemObject& _owner, PortID _id);
/**
* Virtual destructor due to inheritance.
@ -125,7 +117,7 @@ class Port
const std::string name() const { return portName; }
/** Get the port id. */
PortId getId() const { return id; }
PortID getId() const { return id; }
protected:
@ -167,7 +159,7 @@ class MasterPort : public Port
public:
MasterPort(const std::string& name, MemObject* owner,
PortId id = INVALID_PORT_ID);
PortID id = InvalidPortID);
virtual ~MasterPort();
void bind(SlavePort& slave_port);
@ -305,7 +297,7 @@ class SlavePort : public Port
public:
SlavePort(const std::string& name, MemObject* owner,
PortId id = INVALID_PORT_ID);
PortID id = InvalidPortID);
virtual ~SlavePort();
void bind(MasterPort& master_port);