My doc is updated with what my patches turned into, so PTAL if you have a chance. I think it turned out to be pretty straightforward which should make it an easier read.
I took a look at Andrea's patch, and it has some pretty interesting ideas in there. One prominent difference I see is that I made the Port class my primitive, and he made MasterPort and SlavePort (or similar parent classes) his primitives. My version makes symmetrical port connections more natural which is important for Ethernet ports, something he may not have considered since I don't see mention of it in his description. One thing he does which I thought was particularly interesting is to make it so that a module can implement the Port interface itself. I do notice that there are a lot of classes (and I've implemented at least a few) where the Port class is basically just a thin wrapper which delegates calls from the peer to its parent SimObject/MemObject. It's appealing to remove a lot of that boilerplate when possible. Perhaps we can have a template DelegatingPort class which takes its parent's class as a parameter and then does all the delegating? That would avoid having that same pattern manually reimplemented over and over, like I think some of the templated versions of the Event class have done. One thing that it's a little concerning about his approach is that it inherits some of the fanciness and complexity of the TLM approach, and has a built in and likely unavoidable layer of indirection in a high traffic interface. One thing I've noticed in my pretty extensive expeditions diving into the guts of systemc is that there are some fundamental design decisions and implementation choices that seem fairly innocuous, but put a limit on how good performance can be. For instance they use std::vector a lot, and have to do weird tricks like swapping a middle element with the final element in the vector and then shortening it by one to avoid really bad performance when deleting an interior vector element. That works, but it basically stirs your vectors and can make behavior erratic (and can be very annoying to replicated to pass regression tests, *sigh*). Also they don't do as much as they could to avoid context switches by keeping all the "method" "processes" (systemc's version of those terms) bunched together. The point being that while system is prominent and doing things in a systemc-ish way can make it easier to learn and less confusing, blindly imitating it can be a recipe for trouble. Another difference I see (neither good nor bad) is that this seems to have a slightly different objective than what I'm going for. It looks like Andrea was trying to make gem5's protocol and port mechanism extensible to the point where it would include the TLM mechanisms as a subset, and then could either use them natively or connect gem5 and TLM models together under the same roof so to speak. What I'm trying to do is to leave gem5's protocol and ports mostly alone, and to make TLM's sockets, Ethernet interfaces, etc., all separate lobes of the Port mechanism which can be managed with common mechanisms but which are still all their own things with their own rules, etc. For instance, I don't really see any config related code in Andrea's patch. Note that these are not mutually exclusive either. We could, for instance, use my approach to rationalize how ports are configured and connected, and then also use Andrea's approach to modify gem5's protocol within the MasterPort/SlavePort lobe, and/or use it to bridge the MasterPort/SlavePort and TLM lobes. I would still worry a bit about the complexity, but if you're willing to live with that in the first place I don't think there would be any additional issues. From a practical perspective, moving Andrea's change forward 6-7 years would probably be a non-trivial porting effort, so my (somewhat biased, somewhat selfish) suggestion would be to go ahead with my approach and then look to porting his in whole or in part after that. Gabe On Thu, Mar 7, 2019 at 11:42 AM Steve Reinhardt <ste...@gmail.com> wrote: > Thanks for digging this up, Jason. I knew this issue had been addressed > multiple times before (I think I even had a patch at one point that was a > smaller change, but held it off in favor of Andreas's version). I don't > know why Andreas's change was never committed either. > > Anyway, it will be good to see this cleaned up, regardless of whether we go > with Andreas's code or Gabe's proposal or some hybrid. I've been out of > the loop long enough that I don't have a specific preference. > > Steve > > > On Thu, Mar 7, 2019 at 10:19 AM Jason Lowe-Power <ja...@lowepower.com> > wrote: > > > Hey Gabe, > > > > I was digging through the old reviewboard and found this patch that also > > re-did this interface: http://reviews.gem5.org/r/1301 > > > > I'm not sure why this was never committed. > > > > I believe Andreas H's goal was to enable TLM-2 interfaces with the gem5, > > IIRC. > > > > Just something to consider. > > > > Cheers, > > Jason > > > > On Thu, Mar 7, 2019 at 7:00 AM Gabe Black <gabebl...@google.com> wrote: > > > > > Hey folks, specifically folks looking at this doc. I have a series of > > > patches which largely implement what I was going for, although it > turned > > > out differently than what I have in my doc. I'll update the doc > soon(ish) > > > to describe the current version. Go ahead and review the CLs if you > want, > > > although I should probably run another test or two on them and the > > > discussion of the design is still open over on the doc. > > > > > > > > > > > > https://gem5-review.googlesource.com/q/topic:%22tlm%22+(status:open%20OR%20status:merged) > > > > > > Gabe > > > > > > On Wed, Mar 6, 2019 at 12:20 AM Gabe Black <gabebl...@google.com> > wrote: > > > > > > > Hi folks. I've been looking at how to configure TLM sockets through > > > gem5's > > > > port configuration mechanism and how gem5's port configuration > > mechanism > > > > works in general, and I think I've mostly come up with a plan. I've > > > written > > > > everything up in a doc over here: > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/17eXkE9YtzvYXEgkHFNR1my_xYKl3mYNNtXM9pIAX-t0/edit?usp=sharing > > > > > > > > Please take a look if you have a chance, and please comment on the > doc > > if > > > > you have any questions, concerns, etc. > > > > > > > > I created the doc on my personal account but wrote it from my work > > > account > > > > so it *should* be accessible and commentable by anyone with the link. > > > > Please let me know if that's not the case. > > > > > > > > Gabe > > > > > > > _______________________________________________ > > > gem5-dev mailing list > > > gem5-dev@gem5.org > > > http://m5sim.org/mailman/listinfo/gem5-dev > > _______________________________________________ > > gem5-dev mailing list > > gem5-dev@gem5.org > > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ > gem5-dev mailing list > gem5-dev@gem5.org > http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
