Hey Gabe, Thanks for the detailed explanation. Sorry it's taken me so long to get back to you.
I think I understand all of your points, now. I'll do my best to review the code in the next few days. Although, I make no promises as I've started teaching again, which is eating into my code review time. Cheers, Jason On Tue, Jan 7, 2020 at 6:30 PM Gabe Black <gabebl...@google.com> wrote: > Hey folks, ping, and also I wanted to mention that I clumped the GuestABI > vs. general SE cleanup stuff in my patch series at the start and then later > at the end when it was too annoying to move it in front of other changes. > > First clump: > https://gem5-review.googlesource.com/c/public/gem5/+/23195/6 > > Second clump: > https://gem5-review.googlesource.com/c/public/gem5/+/24103/1 > > Please take a look when you get a chance. > > Gabe > > > On Thu, Dec 19, 2019 at 4:33 PM Gabe Black <gabebl...@google.com> wrote: > > > My original intention was actually to represent the ABIs as objects and > > not as systems of templates, so that you could do something like this: > > > > ABI abi; > > abi->call(foo); > > > > There are a few problems with that though which made it impossible. > First, > > C++ won't let you have templated virtual methods. It has to know up front > > how many slots to make in the vtable, and what should go in each slot in > > subclasses. That would mean that you'd have to define categories of > inputs > > ahead of time like 64 bit ints, float, double, structures, pointers, etc, > > etc. The unbounded number of types and the subtle nuances in how an ABI > > might handle them would make predefining those categories impossible. For > > instance, in the aarch64 ABI, there are special rules for arrays and > > structures which have only one type within them which is a type of float, > > and have 4 or less members. How you treat structures is also different > > depending on if they're bigger or smaller than 16 bytes. There's > basically > > no chance the types of arguments would be fine grained enough to catch > > those differences without knowing about them ahead of time. On top of > that, > > exactly where things end up on the stack depend on what alignment they > > need, and that in turn depends (at least to some extent) on what's inside > > them. > > > > Second, you need to know what the signature of the function your calling > > is, and there's really no good way to take that apart that I know of > other > > than templates. If you don't do it that way, it's sort of like making all > > your functions take "..." as their only argument. Any system of functions > > *can* work that way if you, for instance, premangle the names so > > polymorphism works out, but you lose a major source of documentation > about > > how the function should work/be used. > > > > There are also some things that style (and the existing one-by-one > > argument extraction) makes harder than it could be. If, for instance, you > > have multiple similar versions of a function which take slightly > different > > arguments, or that take them in a different order, then might need to > have > > a central implementation which takes a bunch of flags telling it what > > arguments to skip over or read, or what order to extract things in. The > > "pipe" system call is like that, as well as "clone". > > > > If your functions just look like functions, you can have: > > > > SyscallReturn cloneFunc(..., Addr a, Addr b) > > { > > ... > > } > > > > SyscallReturn cloneBackwardsFunc(..., Addr b, Addr a) > > { > > return cloneFunc(..., a, b); > > } > > > > Since everything is just an argument, you can call using normal C++ from > > within gem5 just as easily as you could call from inside the TC, and that > > lets you add, replace, remove, or reorder arguments easily without having > > to try to tell the main implementation what to do at arm's length. > > > > Aside from the mechanical necessity of templates for this system to work > > well, I think the complexity (which is real and worth noting) is actually > > not that big a problem since unlike the stats I've made a concerted > effort > > to explain how they work, and almost everyone who interacts with this > > mechanism won't need to worry about how it works down in its guts anyway, > > they just need to know how to use it. > > > > For instance, let's say you want to add a new syscall which adds two > > numbers together and returns the result. With this new mechanism, that > > would look like this: > > > > SyscallReturn > > addFunc(SyscallDesc *desc, int num, ThreadContext *tc, int a, int b) > > { > > return a + b; > > } > > > > That looks pretty simple, and you can tell what to do just by looking at > > the functions around it without having to use any unfamiliar API to get > > your arguments or set your return value. > > > > As a matter of fact, that reminds me I wanted to fold "num" into > > SyscallDesc since it's almost never used and makes all the signatures > > messier, but that's a bit of a tangent. > > > > Gabe > > > > On Thu, Dec 19, 2019 at 2:09 PM Jason Lowe-Power <ja...@lowepower.com> > > wrote: > > > >> Hi Gabe, > >> > >> Thanks for the document! This really cleared things up for me. > >> > >> I made a bunch of comments in the document, but I think they can > >> almost all be boiled down to this one thing: > >> > >> I would like to understand the tradeoff between implementing the ABIs > >> as virtual objects vs templates. Generally, I would like to see us > >> avoid using templates except in exceptional cases. I think that in > >> many cases templates over complicate the code. For instance it's quite > >> difficult to understand the code when using variadic recursive > >> template functions! > >> > >> I completely understand the need for a more general ABI > >> implementation, and I really like the idea of unifying the syscall, > >> pseudo instruction, and other interfaces! I think that what you've > >> designed is quite beautiful and very clever. However, I worry that > >> it's a little *too* clever, and won't be easy to maintain by other > >> developers in the future. Another good example of this in gem5 is the > >> statistics interface. That's a block of code that most of us are too > >> scared to touch due the template complexities. > >> > >> That said, if there are required features that cannot be implemented > >> by using virtual objects, then I can get behind the template > >> implementation. You hint that there are things that can't be done with > >> virtual objects at the end of the document. I'd like to understand > >> this better before I review the code. > >> > >> Thanks for the hard work! I think the end goals of this proposal are > >> important and I definitely want to see this implemented! > >> > >> I'm traveling right now, but I'll do my best to keep up with this > >> thread and reply in a timely manner. > >> > >> Cheers, > >> Jason > >> > >> On Thu, Dec 19, 2019 at 3:41 AM Gabe Black <gabebl...@google.com> > wrote: > >> > > >> > I made another editing pass and made a few small changes which > hopefully > >> > will make things easier to understand. > >> > > >> > Gabe > >> > > >> > On Wed, Dec 18, 2019 at 6:51 PM Gabe Black <gabebl...@google.com> > >> wrote: > >> > > >> > > Ok, great, thanks. I know the implementation is heavy on C++ > sorcery, > >> so > >> > > please let me know if you have any questions. > >> > > > >> > > Gabe > >> > > > >> > > On Wed, Dec 18, 2019 at 6:20 PM Jason Lowe-Power < > ja...@lowepower.com > >> > > >> > > wrote: > >> > > > >> > >> I'll have some time tomorrow to look it over. > >> > >> > >> > >> Cheers, > >> > >> Jason > >> > >> > >> > >> On Wed, Dec 18, 2019 at 4:06 PM Gabe Black <gabebl...@google.com> > >> wrote: > >> > >> > > >> > >> > Great, thanks! > >> > >> > > >> > >> > Gabe > >> > >> > > >> > >> > On Wed, Dec 18, 2019 at 5:04 PM Giacomo Travaglini < > >> > >> > giacomo.travagl...@arm.com> wrote: > >> > >> > > >> > >> > > Thanks Gabe, I am reviewing the document, will address your > >> patchset > >> > >> as > >> > >> > > soon as I finish > >> > >> > > > >> > >> > > Giacomo > >> > >> > > ________________________________ > >> > >> > > From: gem5-dev <gem5-dev-boun...@gem5.org> on behalf of Gabe > >> Black < > >> > >> > > gabebl...@google.com> > >> > >> > > Sent: 18 December 2019 02:05 > >> > >> > > To: gem5 Developer List <gem5-dev@gem5.org> > >> > >> > > Subject: [gem5-dev] Guest ABI design doc > >> > >> > > > >> > >> > > Hi folks. I talked to some people and got permission to share > >> this > >> > >> design > >> > >> > > doc externally first. Please take a look to when you get a > >> chance. > >> > >> > > > >> > >> > > > >> > >> > > > >> > >> > >> > https://docs.google.com/document/d/1CvB_srNAXXi6JgU8Q8Ou0tEcAv4DXdulhNj8kxowFkk/edit?usp=sharing > >> > >> > > > >> > >> > > 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
