-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1216/#review2823
-----------------------------------------------------------


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.

- Nilay Vaish


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

Reply via email to