Hey Jason, thanks for taking a look. As far as comments go, there aren't really that many comments in the Accellera implementation either, at least not doxygen style, per function args, ret style comments. I think for things which are defined in the spec and/or in systemc books, tutorials, etc., those comments are really that necessary since the APIs are already fairly extensively documented elsewhere, and in a better more usable form than comments in the headers. When it comes to the stuff that isn't in the spec, like the guts of the implementation that make all those parts work, then comments make a lot of sense since there isn't an already existing and superior form of documentation out there.
So in short, I don't intend to document systemc itself since that's already been extensively done elsewhere, but I do intend to document the difference in the gem5 flavor, and the internals. Gabe On Thu, Jun 14, 2018 at 6:19 PM Jason Lowe-Power <ja...@lowepower.com> wrote: > Hey Gabe, > > So, I've finally starting going through all of the patches (as you could > see). One thing that I don't want to write on every single patch is "add > comments", but I really think that the header files should have doxygen > comments on everything. Is it possible to copy them over from the Accellera > implementation? > > Another general documentation thing is that at some point it would be good > to put at least a short README in the systemc directory. I'm sure you're > planning to write extensive documentation on the wiki ;), but it would also > be a good idea to have something brief the code for those people who don't > want to leave the command line. > > Finally, for everyone else that's reviewing: I find it hard to review big > changes like this on gerrit since you only see one change at a time and you > can't see what the "end result" is. However, if you go to the last commit ( > https://gem5-review.googlesource.com/c/public/gem5/+/10975/1) and click on > the sha hash of the commit it lead you to this page ( > > https://gem5.googlesource.com/public/gem5/+/ebd453d32fe1abcf1fb1a047dbcd6151558a117e > ) > which will show you the "final" code after all of the patches are applied. > Hopefully this can save someone some time! > > Cheers, > Jason > > PS: I don't know much about SystemC. I've been learning a little trying to > review Gabe's changes... I have a general question: what are the tradeoffs > compared to gem5? Specifically, is there *anything* that's better about > gem5? This is a much larger conversation, but the suggestion of replacing > gem5's simulation engine with SystemC is intriguing (and scary...). > > On Mon, Jun 11, 2018 at 7:19 PM Gabe Black <gabebl...@google.com> wrote: > > > Ok great, glad to hear it. > > > > Gabe > > > > On Mon, Jun 11, 2018 at 3:34 PM Matthias Jung <jun...@eit.uni-kl.de> > > wrote: > > > > > Hi Christian, > > > > > > I think you summarised the 3 approaches very well. I mean, we have > > > approach 1 already. It makes sense if Gabe drives approach 2 because > > > it has many advantages compared to approach 1. I think we could see > > > approach 3 as a longterm goal and we should go for approach 2 for now. > > > > > > Thanks for all the opinions so far, > > > Best, > > > Matthias > > > > > > > Am 11.06.2018 um 18:02 schrieb Christian Menard < > > > christian.men...@tu-dresden.de>: > > > > > > > > Hi, > > > > > > > > I am following the discussion for a while now and finally found the > > time > > > > to look at Gabe's proposal. > > > > > > > > As I see it, there are three approaches for combining gem5 and > SystemC > > > > as outlined below. (Sorry for repeating stuff that was mentioned > > before, > > > > I just find it helpful to summarize some points) > > > > > > > > 1. Bridging gem5 ans Systemc. > > > > This is the approach I and Matthias implemented and presented last > > > > year. It provides bridges between the gem5 and SystemC communication > > > > interfaces, as well as a wrapper SystemC module that hosts a complete > > > > gem5 simulation on top of the SystemC kernel. While this approach > > > > certainly has limitations, it allows to combine gem5 and SystemC > > models. > > > > > > > > 2. Implementing the SystemC standard using gem5 > > > > This is the approach proposed by Gabe which, as I understand it, > > > > provides a wrapper around gem5 implementing the SystemC standard. > With > > > > this approach, gem5 becomes a fully fledged SystemC kernel which > > > > arbitrary standard compliant SystemC models can run on. Compared to > > > > approach 1, this allows for more interaction between both domains, as > > > > everything can be compiled in a single pass and there is not just one > > > > single point of interaction. However, this approach prevents certain > > use > > > > cases, e.g. where SystemC models are closed source or where a > specific > > > > SystemC implementation is required. > > > > > > > > 3. Replacing the gem5 simulation kernel by SystemC. > > > > This is the most radical approach but also gives most flexibility. It > > > > replaces the entire gem5 simulation kernel by SystemC. In this > > approach, > > > > gem5 could be seen as a system modeling framework and as an > abstraction > > > > layer on top of SystemC. This would give maximum flexibility as > > > > arbitrary SystemC and gem5 models can be combined and even the > SystemC > > > > kernel can be exchanged arbitrarily. However, it is not clear (at > least > > > > to me) how exactly gem5 and SystemC modules could be connected and > > > > interact with each other. I think for this approach to work, aspects > of > > > > approach 1 or/and 2 are still required. > > > > > > > > So as I see it: 3 covers more use cases than 2 but both are in a way > > > > superior to the existing approach (1). However, in order to > implement 3 > > > > a lot of changes to the code base are required. Implementing these > > > > changes will take some time, so there will probably be two versions > of > > > > gem5: a legacy one and the SystemC one. This again produces more work > > in > > > > maintaining the code base. Now I wonder: who is willing to do all > this > > > > work? > > > > > > > > While I favour approach 3 for its benefits and the points Matthias > > made, > > > > I still like Gabe's idea very much. It minimizes the changes required > > to > > > > the existing code base while providing many benefits to a broader > > > > community. As Gabe mentioned before, his approach neither breaks with > > > > the existing bridges implemented by me and Matthias, nor does it > > prevent > > > > implementation of approach 3 in the future. To sum it up: there are > no > > > > objections from my side. > > > > > > > > Unfortunately, I am not very active in hardware modeling anymore, > but I > > > > am very interested in this development and I hope to find the time to > > > > have a look on the patches soon. > > > > > > > > Best, > > > > > > > > Christian > > > > > > > > Matthias Jung <jun...@eit.uni-kl.de> writes: > > > > > > > >> Hi Gabe, > > > >> > > > >> I totally agree with you. SytemC is a standard and the code > maintained > > > >> by accellera is just an „example“ of how SystemC could be > implemented. > > > >> > > > >> However, that is part of my argument. If I want to use e.g. another > > > >> fancy SystemC kernel (e.g. > https://dl.acm.org/citation.cfm?id=2987374 > > ) > > > >> or a commercial one like the one in the Synopsys toolchains, I > cannot > > > >> use gem5 (beside the coupling that is already there, which has also > > > >> several drawbacks). So I like more the separation of simulation > models > > > >> and the kernel. > > > >> > > > >> But I also understand it from your side. In Google you don’t have > this > > > >> specific need and you want to find quickly a solution with less > > effort. > > > >> Anyway we should discuss if a full switch to SystemC as a kernel > might > > > >> be a reasonable long term goal. I think many people would benefit > from > > > >> that. > > > >> > > > >> I’m also keen to know Christian’s, Andreas’ and Jason’s opinions. > > > >> > > > >> It’s a pitty that gem5 and SystemC started back at the same time > > > >> and evolved separately... > > > >> > > > >> Best, > > > >> Matthias > > > >> > > > >>> Am 09.06.2018 um 02:34 schrieb Gabe Black <gabebl...@google.com>: > > > >>> > > > >>> Also, I should point out that the systemc standard defines a set of > > > >>> mechanisms and an interface, not an implementation. The Accellera > > > version > > > >>> of systemc is *not* the standard, it's just an implementation (a > very > > > >>> common and important one) of that standard. It's dangerous to > > conflate > > > >>> those two ideas, and it leads to a lot of problems for everybody. > > > >>> > > > >>> Gabe > > > >>> > > > >>> On Fri, Jun 8, 2018 at 12:50 PM Gabe Black <gabebl...@google.com> > > > wrote: > > > >>> > > > >>>> Giacomo, if you're proposing linking in the systemc library and > then > > > >>>> adding wrappers to somehow plug that into gem5's underlying > > > mechanisms, I'm > > > >>>> not sure that's technically feasible since the existing > > implementation > > > >>>> isn't intended to be built on top of something else. Also a lot of > > the > > > >>>> mechanisms are built into base classes, and so if you change what > > data > > > >>>> members are in the base classes, ie the internal implementation, > you > > > break > > > >>>> existing binaries. There isn't much a wrapper can do in that case. > > > >>>> > > > >>>> If you're proposing rebuilding gem5 on top of systemc, there are a > > > variety > > > >>>> of reasons I'm not proposing that which I've gone through > > > exhaustively (at > > > >>>> least I feel exhausted from it) in other places (you didn't miss > it, > > > that > > > >>>> was internally at Google). > > > >>>> > > > >>>> This approach addresses best addresses the problem we (Google) are > > > trying > > > >>>> to solve, which is to leverage existing models vendors may have > > > developed > > > >>>> already in systemc, or which may be developed primarily in systemc > > in > > > the > > > >>>> future. Rather than ask them to rewrite their models as gem5 > models, > > > >>>> simultaneously develop two models, one for systemc and one for > gem5, > > > or to > > > >>>> architect their model as a core with two interface layers, one for > > > each > > > >>>> system, any of which may be prohibitive from an engineering effort > > > >>>> perspective, this way they can write a systemc model (or take one > > they > > > >>>> already wrote) and then just recompile it with a different systemc > > > header > > > >>>> and use it directly as a model within gem5. > > > >>>> > > > >>>> There will be very little modification to gem5 proper for this, > and > > > so if > > > >>>> this sort of integration doesn't do what you want you can just > > ignore > > > it or > > > >>>> even turn it off and not suffer any overhead, and then use > whatever > > > other > > > >>>> mechanism you like. > > > >>>> > > > >>>> Matthias, while being able to use closed source models would be > > nice, > > > it > > > >>>> isn't the problem we're trying to solve, and reimplementing gem5 > on > > > top of > > > >>>> systemc would be a lot more work that doesn't really gain us > > anything. > > > >>>> > > > >>>> Jason, thanks. Please start with that python CL if you have a > chance > > > since > > > >>>> I think it's about ready to go in, Andreas just wanted you to ack > it > > > before > > > >>>> checking it in. > > > >>>> > > > >>>> Gabe > > > >>>> > > > >>>> > > > >>>> On Fri, Jun 8, 2018 at 7:15 AM Jason Lowe-Power < > > ja...@lowepower.com> > > > >>>> wrote: > > > >>>> > > > >>>>> I'll have some time next week to dig into this. > > > >>>>> > > > >>>>> Cheers, > > > >>>>> Jason > > > >>>>> > > > >>>>> On Fri, Jun 8, 2018, 7:13 AM Dr.-Ing. Matthias Jung < > > > jun...@eit.uni-kl.de > > > >>>>>> > > > >>>>> wrote: > > > >>>>> > > > >>>>>> Hi Giacomo, Gabe, > > > >>>>>> > > > >>>>>> I'm a large supporter of SystemC because its 'the' IEEE standard > > for > > > >>>>>> simulation, therefore I support always activities towards that > > > >>>>> direction. > > > >>>>>> However I have a similar concern like Giacomo. > > > >>>>>> > > > >>>>>> I would prefer to just 'use' the SystemC kernel by accellera as > > > kernel > > > >>>>> for > > > >>>>>> gem5 in order to have a proper separation between model and > > kernel. > > > I > > > >>>>> think > > > >>>>>> its very interesting for many people to use gem5 in their > SystemC > > > >>>>>> environment also together with commercial IP SystemC models that > > > have a > > > >>>>>> closed source. With the current aimed approach it would be > > required > > > to > > > >>>>> have > > > >>>>>> access to the source code of all used models in the system and > > > >>>>> commercial > > > >>>>>> SystemC tools like Synopsys Platform Architect etc. could not be > > > used. > > > >>>>>> > > > >>>>>> > > > >>>>>> Best, > > > >>>>>> Matthias > > > >>>>>> > > > >>>>>> 8. Juni 2018 15:47, "Giacomo Travaglini" < > > > giacomo.travagl...@arm.com> > > > >>>>>> schrieb: > > > >>>>>> > > > >>>>>>> Hi Gabe, > > > >>>>>>> > > > >>>>>>> I have had a quick glance at the patches and there's one thing > I > > > don't > > > >>>>>> understand: > > > >>>>>>> > > > >>>>>>> It seems to me that you are sort of reimplementing the SystemC > > > runtime > > > >>>>>> kernel inside > > > >>>>>>> > > > >>>>>>> gem5 for scratch. > > > >>>>>>> > > > >>>>>>> Is there a reason for doing it? Can't we just link to the > > external > > > >>>>>> SystemC library > > > >>>>>>> > > > >>>>>>> and just write some wrappers? > > > >>>>>>> > > > >>>>>>> Thanks in advance for the clarifications > > > >>>>>>> > > > >>>>>>> Giacomo > > > >>>>>>> > > > >>>>>>> ________________________________ > > > >>>>>>> From: gem5-dev <gem5-dev-boun...@gem5.org> on behalf of Gabe > > > Black < > > > >>>>>> gabebl...@google.com> > > > >>>>>>> Sent: 08 June 2018 06:32:52 > > > >>>>>>> To: gem5 Developer List > > > >>>>>>> Subject: [gem5-dev] systemc reviews > > > >>>>>>> > > > >>>>>>> Hi folks. I've posted a lot of systemc reviews recently, and I > > > expect > > > >>>>> to > > > >>>>>>> keep doing so for the foreseeable future. To keep the pool of > > > pending > > > >>>>> CLs > > > >>>>>>> from growing from a lot to unmanageably a lot please don't let > > them > > > >>>>> sit > > > >>>>>> too > > > >>>>>>> long if you feel comfortable trying to review them. Alot of > these > > > >>>>> earlier > > > >>>>>>> ones aren't very interesting, they're just defining header > files, > > > >>>>> stubbed > > > >>>>>>> out implementations, or importing code from the Accellera > version > > > of > > > >>>>>>> SystemC. There are a few that are a little more interesting > > > though, to > > > >>>>>> keep > > > >>>>>>> you from getting bored ;-) > > > >>>>>>> > > > >>>>>>> Gabe > > > >>>>>>> _______________________________________________ > > > >>>>>>> gem5-dev mailing list > > > >>>>>>> gem5-dev@gem5.org > > > >>>>>>> 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 > > > >>>>>>> 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 > > > >> > > > >> _______________________________________________ > > > >> 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 > _______________________________________________ > 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
