I'll move over to email since this is turning into a discussion more than review feedback. It's not the first thing on my agenda, but fairly high up is going to be simplifying this interface at least a little bit. I did some grepping, and I think the only place getExtMachInst is used is in the checker right now. extMachInstReady can be renamed to instReady. We may be able to get rid of it entirely and rely on decode() returning NULL if nothing is ready, but I don't want to go quite that far without looking at current uses carefully and considering what that might preclude doing in the future. That would primarily simplify things for everything except ARM and x86 which have to keep track of that anyway.
The interface I'd most like to get rid of, though, is the one where you can pass an ExtMachInst to the decoder and have it decode it for you. In x86, this means having to check whether the thing you're decoding is the thing you just predecoded in case somebody has some other ExtMachInst lying around they want to toss in. That isn't possible if the decode(pc) style interface is used. This is especially painful in x86 because ExtMachInsts are big and relatively complicated to compare. I have some changes I'm hammering the bugs out of which seem to improve x86 performance in atomic mode on twolf by about 10%, but I lose about 3-4% of that with the ExtMachInst comparison. It looks like the only consumer of the old interface that won't be easy to move over is O3 and its noops. Any ideas how to build a working StaticInst outside of the parser and its execute function duplicating magic? The magic I'm referring to is how it makes a copy of execute for each possible ExecContext based on what CPUs are build it. Gabe On 05/20/12 18:15, Steve Reinhardt wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1195/ > > > On May 20th, 2012, 5:54 p.m., *Gabe Black* wrote: > > I meant to also say that I'm not trying to change the interface to the > decoder/predecoder combo here. That would come later and is outside the scope > of this change. > > OK, I guess I'm just confused because there are places where you did change > the interface more significantly, like the change in cpu/simple/base.cc > around lines 380-400, getting rid of extMachInstReady() and getExtMachInst() > entirely, and I was hoping to see that kind of transformation everywhere that > those functions were used. _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
