On Thu, Jan 5, 2012 at 8:04 AM, Andreas Hansson <[email protected]>wrote:
> I like it! #5 I will definitely do as part of this. > Great! > > Suggestions: > 6. Make SysComponent a SysModule instead? > SysModule sounds OK; I like SysObject too (more parallel with SimObject and MemObject) though it is slightly more ambiguous than SysModule. I'd hold off on finalizing a name until we get another opinion or two. > 7. Keep MemObject in the hierarchy between SimObject and > SysComponent/SysModule and let the System be a MemObject? > I guess great minds think alike, as they say... > 8. Assuming 7, alternatively rename the MemObject StructuralObject or > StructObject to distinguish the fact that it is a "real" structural entity, > compared to SimObjects like processes etc. > If we're going to keep the class, I'd prefer to keep the same name... one of the main reasons I'd lean toward keeping the class is to minimize the changes to the existing hierarchy, and changing the name loses that benefit. Steve > > Andreas > > -----Original Message----- > From: [email protected] [mailto:[email protected]] On > Behalf Of Steve Reinhardt > Sent: 05 January 2012 15:53 > To: gem5 Developer List > Cc: Ali Saidi > Subject: Re: [gem5-dev] Review Request: MEM: Add the system port as a > central access point > > I think we all agree that being able to traverse the hierarchy in C++ seems > reasonable is something we'll probably add eventually. I don't want to add > it for something that doesn't really need it though, because our general > rule has been to do as much in python as we reasonably can, and I'd rather > fix things on the python side to make this work there than to punt and > force it into C++. > > As far as changing the object hierarchy: what about just taking all the > port support (what I was proposing be split off into the poorly named > PortObject) and push it up into SimObject? Then instead of having a > separate class for objects that have ports, we just say that every object > can have ports, but some have zero of them. > > We'd still need MemObject to distingush objects that get a pointer back to > the System from those that don't. Though if that's the only distinction, > MemObject isn't the greatest name either; maybe it should be > SystemComponentObject or something like that (though that's too long IMO: > maybe SysObject, or SysComponent, or something like that). Then most > things would probably derive from SysComponent, and only things that > clearly aren't proper elements of a single system (like System itself, and > maybe EthernetLink, and maybe some of the fake SimObjects that aren't > really component models) would derive directly from SimObject. > > So my proposal is: > 1. Move MemObject methods into SimObject (getPort() etc.) and get rid of > MemObject entirely. > 2. Replace MemObject with SimObject in mem/port.{cc,hh} and similar places > where it's just used to indicate a class that can have ports. > 3. Create a new class SysComponent (I'm open to other names) that derives > from SimObject, and doesn't do anything but add a system = > Param.System(Parent.any) param. > 4. Replace MemObject with SysComponent anywhere it's used as a base class. > 5. (optional, but very desirable) Track down any non-MemObject derived > class that has its own Param.System(Parent.any) param, make it derive from > SysComponent too, and get rid of that explicit param. > > Thoughts? > > Steve > > On Thu, Jan 5, 2012 at 1:06 AM, Andreas Hansson <[email protected] > >wrote: > > > Steve, Nate, > > > > Thanks for the feedback! I hope we can reach consensus on this soon. > > > > To start with Nate's suggestion: It is not the members of System or > > MemObject that cause the problem, this is already in place. The issue is > > with the parameter header generation and SWIG wrapper generation. This > > would happen also without the PhysicalMemory, since System inherits from > > MemObject, and not a MemObject has a System pointer. Note that the latter > > could be fixed in c++ by forward declaring System in MemObject.hh. This, > > however, requires some serious modifications of the cxx_predecls, > > swig_predecls etc. > > > > A more general way of traversing the object hierarchy (I feel design > > patterns coming on :-) would definitely solve the problem and the > setParent > > is really the not-so-general solution to this problem. Are there any > other > > obvious good uses of this hierarchy that have been implemented in other > > (more inconvenient) ways? > > > > Steve's suggestion: Very much seems doable, with an additional class in > > the hierarchy. I also struggle with the naming, but I'll give it a quick > > bash and see how it goes. > > > > Andreas > > > > > > -----Original Message----- > > From: [email protected] [mailto:[email protected]] On > > Behalf Of nathan binkert > > Sent: 04 January 2012 21:53 > > To: gem5 Developer List > > Cc: Ali Saidi > > Subject: Re: [gem5-dev] Review Request: MEM: Add the system port as a > > central access point > > > > > The issue is essentially that the System inherits from MemObject, and > > placing a system = Param.System(...) in MemObject creates a cycle. > > params/System.hh ends up including MemObject.hh and vice versa. I have > > tried a number of permutations of forward declarations etc using the > > cxx_predecls vs swig_decls for the System/MemObject but not managed to > > solve it so far. It is starting to look pretty nasty, but I will keep on > > going and see if I can solve the circular dependency this creates in a > > decent way. > > The standard way of doing this is to break the cycle in C++. The > > reason you have a cycle (I believe) is that System has the physmem > > parameter and the memories parameter which are both PhysicalMemory > > objects which are in turn MemObjects. So, to do this the standard > > way, you'd put the system = Param.System stuff into MemObject and > > remove the physmem and memories parameters from System. Then in C++ > > during the constructor of PhysicalMemory, you'd call > > params()->system->registerPhysicalMemory (you'd have to write that > > function). The only hangup I see is that physmem is only one of > > potentially many PhysicalMemory Objects, so to make this work, you'd > > probably have to add a boolean parameter to PhysicalMemory that's like > > system_memory so that one object would get written back. > > > > It may just be better to allow cycles and enforce a more rigorous > > split between the constructors and init() (which I've wanted to do for > > a long time), but that's a much more complicated fix. Something else > > that would be possible and would make things easier for this sort of > > thing would be to add support for traversing the object tree in C++. > > It probably wouldn't be all that difficult. > > > > > > Nate > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > > > > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > > confidential and may also be privileged. If you are not the intended > > recipient, please notify the sender immediately and do not disclose the > > contents to any other person, use it for any purpose, or store or copy > the > > information in any medium. Thank you. > > > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
