On Mon, May 21, 2012 at 3:48 AM, Gabe Black <[email protected]> wrote:
> 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. > Can that usage be easily eliminated? That's one of my main comments on this patch: you eliminated it in a couple of other places (like simple/base.cc, inorder/resources/fetch_unit.cc, and legiontrace.cc) and it wasn't clear to me why it wasn't equally easy to eliminate it in the checker. If you did that, then we could get rid of getExtMachInst() entirely (or at least make it provate to the decoder). > extMachInstReady can be renamed to instReady. > That sounds good. > 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. > I'm fine with leaving that as a future possibility. > 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. > Agreed, this would be great, but it can wait for a later patch. Would it be possible for each ISA to internally generate a no-op StaticInst during initilization (e.g., during the Decoder constructor or init(), by passing in a no-op ExtMachInst that only that ISA's Decoder knows about), then just have a function that returns a pointer to it? I.e., if the goal is just to get a no-op StaticInst, why not just have the Decoder provide that directly instead of tricking it into providing one by passing in a special ExtMachInst? Steve _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
