> On May 28, 2012, 8:05 p.m., Steve Reinhardt wrote: > > src/arch/x86/decoder.hh, line 65 > > <http://reviews.gem5.org/r/1220/diff/1/?file=26857#file26857line65> > > > > I'm kind of confused by your data structure here... why have a separate > > 'chunks' variable instead of using bytes.size() (which I think is what > > chunks is tracking, no?)? And does 'masks' ever represent anything other > > than a contiguous sequence of bytes? > > > > Intuitively I'd think the cache entry would be something like > > std::pair<StaticInstPtr, std::vector<uint8_t> >, and I'm not following why > > it's so much more than that.
An instruction will be contiguous in memory (thankfully), but it doesn't necessarily start or end on the boundaries between MachInsts which are currently 8 bytes for x86. The masks let you pull out and compare against the bytes of interest without having to shift things around or figure out how many bytes are from this MachInst vs. the next one, etc. The masks can be computed once when the entry is established and used many times (or at least that's the idea), and having a value to and against leaving everything in place is nice and quick. I especially don't want to do things as a series of bytes because then you'd have to compose them together into a single value to compare against, find a mask to get just the incoming bytes you wanted, shift things to get them to line up, etc. The "chunks" variable is indeed tracking the number of elements in the bytes vector. I thought it might be faster to keep track of it locally as a simple variable and not have to reach into the bytes vector with a function, but it wouldn't surprise me if there was no actual benefit to that. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1220/#review2845 ----------------------------------------------------------- On May 25, 2012, 3:45 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1220/ > ----------------------------------------------------------- > > (Updated May 25, 2012, 3:45 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9025:bfe6203ca3d5 > --------------------------- > X86: Move address based decode caching in front of the predecoder. > > The predecoder in x86 does a lot of work, most of which can be skipped if the > decoder cache is put in front of it. > > > Diffs > ----- > > src/arch/x86/decoder.hh bb25e7646c41 > src/arch/x86/decoder.cc bb25e7646c41 > src/arch/x86/isa.hh bb25e7646c41 > src/arch/x86/isa.cc bb25e7646c41 > > Diff: http://reviews.gem5.org/r/1220/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
