I'm replying on m5-dev instead of review board since this is wandering away from the topic of Timothy's code. Please let me know if we're trying to keep things on there anyway.
I've thought before that it would be nice to have generic versions of utility and other functionality as well. A class is nice because you can use inheritance with all of its benefits, but you can't add to a class once it's defined and that can be a real pain. That's why, way back, I went and moved everything from ISA classes into ISA namespaces (That was me, wasn't it? How can I forget these details?). What I think would work and is similar to what I've seen in some other projects is to have a "generic" isa directory and a GenericISA namespace. In that we'd put all the common versions of things (or even multiple versions, if there's more than one common case) and the normal ISAs would be able to either wrap them or add "using GenericISA::foo" to their namespace to pull those in. That complicates building a tiny bit since the GenericISA always needs to be built in and isn't really an ISA (maybe we just treat it like a source directory?) but I think it mostly solves the problem. As far as process initialization, that does need to be worked on. I was alluding to that in my other email in the thread about the initialization stuff. I think this might benefit from providing utility functions/utility classes/base class methods that do smaller parts of the initialization and then mix/match/replace those in the ISA classes. It would be too messy, I think, to try to parameterize everything that might change, and too complicated to add hooks in the base class methods everywhere someone might want to override something. We could, though, provide a generic version of the function that strings together the components in a sensible way, if nothing else to provide a baseline for someone implementing a new ISA. Or, if there no new ISA's we care about still to implement (?), a new OS. Also, this makes me think of the isa_parser. I know that dead horse probably looks like mashed potatoes by now, but I think it would be better to provide that functionality as a python library with a collection of generic classes and whatnot, and then for each ISA to use those or extend them as necessary. The alternatives are, I think, to either make the parser handle every quirk of every ISA, or to provide hooks to override all of its functionality anyway. Otherwise you (or really I) end up with things like the x86 implementation which use the parser but have to do a lot of extra work to subvert it in some cases. I think if we take the same functionality but wrap it in components that can be picked and chosen from and/or specialized and then combined into a whole, things will be cleaner and easier than making one more monolithic system that needs to be broken apart and then layered over and/or specialized after the fact. Also also, I think at one time I was convinced the SPARC implementation didn't need to be specialized either if the common one was implemented carefully. I also think I tried it and it didn't work, so that may just be wrong. Gabe Steve Reinhardt wrote: > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/35/#review63 > ----------------------------------------------------------- > > > It looks like this is identical to the Alpha version of this code, and is > probably the right piece of code to replace the "panic unimplemented" version > in every other ISA except SPARC. I'm guessing this is a can of worms that > Timothy did not intend to open, but it seems like we should have a mechanism > for having some common implementations of utility functions like this that > can get overridden as necessary... say some sort of ISAUtility class with > virtual functions. I have not kept up with the details of the evolution of > the ISA code (maybe Gabe is the only one who really knows all the ins and > outs anymore?). > > I'm also partly prompted by recent experience with overly zealous code > replication in other areas... there used to be a default argsInit() method in > LiveProcess that had a pretty generic stack setup, but then as minor tweaks > were needed for different ISAs, we ended up with each ISA having its own > copy, with about 90%+ similarity across the versions, and no one was calling > the default version anymore. It would be nice if people would find ways to > add hooks to common pieces of code to do just the specialization they need > rather than copy-and-pasting large functions just so they can change a couple > of lines. (That was a general rant, not directed at you, Timothy.) > > - Steve > > > On 2010-07-09 17:53:55, Timothy Jones wrote: > >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> http://reviews.m5sim.org/r/35/ >> ----------------------------------------------------------- >> >> (Updated 2010-07-09 17:53:55) >> >> >> Review request for Default. >> >> >> Summary >> ------- >> >> Power: Provide a utility function to copy registers from one thread context >> to another in the Power ISA. >> >> >> Diffs >> ----- >> >> src/arch/power/SConscript 249f174e6f37 >> src/arch/power/utility.hh 249f174e6f37 >> src/arch/power/utility.cc PRE-CREATION >> >> Diff: http://reviews.m5sim.org/r/35/diff >> >> >> Testing >> ------- >> >> >> Thanks, >> >> Timothy >> >> >> > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
