> On June 13, 2013, 8:33 a.m., Andreas Hansson wrote: > > You want to remove some ugly code in a portion that is only really done > > once for most systems, and the "price" is a more complex object structure > > for the ports. My gut feeling is that these different ports have little in > > common. For example, their actual interfaces are very different. I also > > think this patch makes the port/interface separation much more difficult. > > > > > > Steve Reinhardt wrote: > Actually my motivation for doing this is that I want to introduce a third > type of port connection (for internal use only, for now), and scaling the > current approach to a third type really compounds the ugliness, particularly > because it requires modifying pyobject.cc directly (i.e., it can't be done > via EXTRAS). The advantage of this change is that it makes the port > connection paradigm independent of the type of ports, meaning it's easy to > add more types. Sorry for not explaining that up front. > > The different ports do indeed have little in common other than their > method of connection, but I contend that if they are going to share the same > method of connection, the new code is a much better way of doing that. > Another approach that I considered would be to forget using python assignment > to bind Ethernet devices and links. If we add the constraints that (1) all > Ethernet connections are made via point-to-point links and (2) those links > are always symmetric in delay and bandwidth, then we could just pass in an > EtherLink pointer as a param for each object that wants a connection. While > I would be fine with that, particularly if we had started out that way, > switching to that approach now both introduces more constraints and would > require modifications to the (admittedly few) scripts that use Ethernet > links, so I thought this approach was preferable. > > I'm not sure what you mean by "makes the port/interface separation much > more difficult"; to me, it makes the port concept more abstract, which should > make it more separable from other issues.
The last statement was referring to the changes needed to support: http://reviews.gem5.org/r/1301/. I fear this additional level of Port makes that binding process more challenging. I definitely support the approach you mention with the EtherLink pointers. Supposedly that also avoids the (in my opinion rather confusing) binding by =. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1922/#review4429 ----------------------------------------------------------- On June 13, 2013, 7:58 a.m., Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1922/ > ----------------------------------------------------------- > > (Updated June 13, 2013, 7:58 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9757:071a2d97327a > --------------------------- > sim: unify memory & ethernet port binding mechanisms. > > Get rid of the ugly special-casing and dynamic casting going > on in connectPorts(), and make the port binding mechanism > more extensible. This is done by moving the port lookup > methods to SimObject and creating a common base Port class > from which both the old Port (now MemPort) and EtherInt > (which perhaps should be renamed EtherPort) derive. > > > Diffs > ----- > > src/arch/x86/bios/acpi.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherdevice.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherdevice.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherint.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherint.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherlink.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/dev/etherobject.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/mem/mem_object.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/mem/mem_object.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/mem/port.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/mem/port.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/python/swig/pyobject.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/sim/port.hh PRE-CREATION > src/sim/sim_object.hh 0b4a08751b42ac522e296bd1a12408b816068fa1 > src/sim/sim_object.cc 0b4a08751b42ac522e296bd1a12408b816068fa1 > > Diff: http://reviews.gem5.org/r/1922/diff/ > > > Testing > ------- > > > Thanks, > > Steve Reinhardt > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
