Packet: Unify the use of PortID in packet and port

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9031:d9e932f46019
---------------------------
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.

Testing Done

util/regress all passing (disregarding t1000 and eio)

seems fine to me... steve?

Description:

~		Changeset 9022:8ecb65b49f22
	~	Changeset 9025:03e01e750285

		Packet: Unify the use of PortId in packet and port

		This patch removes the Packet::NodeID typedef and unifies it with the
		PortId that was previously declared in the Port class. 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. As a side effect,
		the INVALID_PORT_ID is also moved to the Packet class.

		Before this patch, two flags were used for valid destination and
		source, rather than relying on a named value (INVALID_PORT_ID), 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.

Diff:

Revision 2 (+75 -74)

Show changes

	src/mem/bridge.hh
	src/mem/bus.hh
	src/mem/bus.cc
	src/mem/packet.hh
	src/mem/port.hh
	src/mem/cache/cache_impl.hh

Overall this patch seems fine to me except for one thing. Why should
the PortId typedef be moved to Packet class? I would rather have the 
Packet class use the definition made in Port class, rather than the 
other way round.

Andreas Hansson May 28, 2012, 1:36 a.m. (May 28, 2012, 1:36 a.m.)

I agree on that entirely, but it becomes problematic with the order of inclusion. Essentially, the very first item in the chain of inclusions is the packet, that is used by the ports, that are instantiated by the mem_object etc. If the PortId would live in the Port then packet would have to include port, and not vice versa.

Suggestions are welcome.

Steve Reinhardt May 28, 2012, 12:45 p.m. (May 28, 2012, 12:45 p.m.)

If it's impractical for PortId to be inside the Port class, should we just make it global?

Andreas Hansson May 28, 2012, 5:40 p.m. (May 28, 2012, 5:40 p.m.)

That is absolutely fine with me. In types.hh then? Any objections?

Summary:

-	Packet: Unify the use of PortId in packet and port
+	Packet: Unify the use of PortID in packet and port

Description:

~		Changeset 9025:03e01e750285
	~	Changeset 9031:d9e932f46019

~		Packet: Unify the use of PortId in packet and port
	~	Packet: Unify the use of PortID in packet and port

		This patch removes the Packet::NodeID typedef and unifies it with the
~		PortId that was previously declared in the Port class. 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. As a side effect,
~		the INVALID_PORT_ID is also moved to the Packet class.
	~	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 (INVALID_PORT_ID), 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.
	~	variables are updated to use PortID.

~		Note that Ruby still has its own NodeID typedef.
	~	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.

Diff:

Revision 3 (+84 -89)

Show changes

	src/base/types.hh
	src/cpu/testers/directedtest/RubyDirectedTester.hh
	src/cpu/testers/rubytest/RubyTester.hh
	src/mem/bridge.hh
	src/mem/bus.hh
	src/mem/bus.cc
	src/mem/packet.hh
	src/mem/port.hh
	2 more

Ship It!

Ship It!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Description:

Changeset 9022:8ecb65b49f22

Changeset 9025:03e01e750285

Diff:

Summary:

Description:

Changeset 9025:03e01e750285

Changeset 9031:d9e932f46019

Diff:

Status: Closed (submitted)