> On May 27, 2012, 9:45 p.m., Nilay Vaish wrote: > > 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.
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. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1216/#review2823 ----------------------------------------------------------- On May 25, 2012, 4:38 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1216/ > ----------------------------------------------------------- > > (Updated May 25, 2012, 4:38 a.m.) > > > Review request for Default. > > > Description > ------- > > 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. > > > Diffs > ----- > > src/mem/bridge.hh bb25e7646c41 > src/mem/bus.hh bb25e7646c41 > src/mem/bus.cc bb25e7646c41 > src/mem/cache/cache_impl.hh bb25e7646c41 > src/mem/packet.hh bb25e7646c41 > src/mem/port.hh bb25e7646c41 > > Diff: http://reviews.gem5.org/r/1216/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
