As I continue to work on this, I'm getting contradictory performance measurements. I can't really tell at this point whether I'm making things better or worse. This may be a moot point.
Gabe On Wed, Feb 11, 2015 at 5:26 PM, Gabe Black <gabebl...@google.com> wrote: > So, right now what I'm doing is I'm splitting up the state necessary to > uniquely generate a particular StaticInst. Some of that state is more > global in nature like the CPU mode, and some of it isn't, like the current > opcode. To make decoding more efficient, I'm trying to avoid checking over > and over and over that yes, I'm still in 64 bit mode, and yes, the stack > size is still such and such. All that happens pre-decode cache, so it's > very performance sensitive. > > The problem is, if ExtMachInst doesn't carry all that state itself any > more, then the state of the decoder at the time it's fed an ExtMachInst > partially determines what will come out. Right now, the decoders are mostly > self contained. They (optionally) generate an ExtMachInst internal to > themselves, and will use that to create a StaticInst when requested and > once they have an entire ExtMachInst. > > The exception is that it's still possible to pass in an ExtMachInst, > circumventing the first part of the decoder. With the exception of the > Minor CPU, that's generally used to create a Noop instruction within the > CPUs which, I think, is used to fill bubbles, as a place holder on DynInsts > which are transports for Faults, etc. Since these instructions literally do > nothing, it shouldn't (*shouldn't*) be hard to make generic StaticInsts > which do nothing local to the CPUs, removing the decoder all together. > > With those two uses removed, then the decoder is a self contained module > which can treat its internal data however it likes and maintain its state > however it likes without having to coordinate two possible sets of inputs, > or worry about external users of its otherwise internal data types. For > instance, I'm considering leaving some fields derived from other state in > the ExtMachInst, but leave them unpopulated normally and making the hashing > and equality operators ignore them. If an instruction isn't in the cache, > then those fields can be populated and consumed. If an ExtMachInst is used > outside of the ISA, then that behavior might be really surprising and break > something. Also, there's no chance of somebody getting an ExtMachInst from > somewhere (perhaps an existing StaticInst), passiing it into the decoder, > and expecting to get an equivalent StaticInst back out. > > Gabe > > On Wed, Feb 11, 2015 at 5:13 PM, Steve Reinhardt via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hi Gabe, >> >> I'll second your comments on ExtMachInst needing to be opaque outside of >> the ISA-specific decoder code---I hadn't realized that the Minor model was >> taking such liberties. >> >> Regarding the bigger picture... is this related to your stateful decoder >> discussion? I can see where, if you put all the decode state in the >> decoder, you wouldn't need to have any additional state in ExtMachInst >> relative to the MachInst object. If you end up going that direction, >> though, what's the advantage of eliminating ExtMachInst vs. just defining >> ExtMachInst == MachInst for that ISA? >> >> Steve >> >> On Wed, Feb 11, 2015 at 4:52 PM, Gabe Black via gem5-dev < >> gem5-dev@gem5.org> >> wrote: >> >> > I'd like to remove the ExtMachInst concept from gem5, or at least hide >> it >> > within the ISAs that need it. The only place where it isn't obvious how >> to >> > do that is in the "Minor" cpu where it treats the ExtMachInst as a 64 >> bit >> > integer (!) and masks it against a bit mask (!) to see when certain >> > additional timing applies. This is really counter to the idea of what an >> > ExtMachInst is in the first place, and also seems highly specific to >> ISAs >> > where things are in particular places consistently and can be >> characterized >> > with bit masks. >> > >> > I'm not familiar with the Minor CPU, but it seems to me there must be a >> > better way to handle this. Removing the idea of an ExtMachInst as an ISA >> > external concept makes certain things easier and safer to do in the >> > decoder, so I would really like to clean this up. >> > >> > A comment makes it sound like this is supposed to behave specially >> based on >> > the source registers for an instruction. That information is already >> > available from the StaticInst, so no bit fiddling with the ExtMachInst >> > should be necessary. >> > >> > Gabe >> > _______________________________________________ >> > 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
