> On 2011-09-21 09:02:14, Steve Reinhardt wrote: > > The separate file seems like a good idea. I'm less sure about the hash map > > though... do you know for sure that this improves performance? In theory, > > there's no reason the compiler couldn't do something similar automatically. > > Plus I'd be surprised if there are enough MSR accesses to matter anyway > > (am I wrong?). > > > > So if we assume that performance is a non-issue, then I'd lean toward > > keeping the switch statement just because it will support MSRs with side > > effects, unless we know with high confidence that MSRs never have side > > effects.
As far as whether there's actually a performance difference, I'm just guessing. I'm at least fairly confident that it won't be significantly worse, and you're right that MSRs aren't going to be used *that* much. As far as the side effects, I think you're thinking of the wrong switch statement. This isn't the one in the miscreg file that receives reads/writes and "does" them, this is in the TLB where it figures out when you access MSR 0xc0018456 you really want the CSTAR MSR. I totally made up that index, but you get the idea. It translates from a sparse indexing space into the actual miscreg indices used by gem5. The sparseness is what made me think the hashmap may do a better job, because I think switch statements generally only (or especially?) do well when they operate on contiguous values and they can set up a jump table. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/838/#review1548 ----------------------------------------------------------- On 2011-09-05 18:40:22, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/838/ > ----------------------------------------------------------- > > (Updated 2011-09-05 18:40:22) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > X86: Move the MSR lookup table out of the TLB and into its own file. > > Translating MSR addresses into MSR register indices took a lot of space in the > TLB source and made looking around in that file awkward. This change moves > the lookup into its own file to get it out of the way. It also changes it from > a switch statement to a hash map which should hopefully be a little more > efficient. > > > Diffs > ----- > > src/arch/x86/regs/SConscript PRE-CREATION > src/arch/x86/regs/msr.hh PRE-CREATION > src/arch/x86/regs/msr.cc PRE-CREATION > src/arch/x86/tlb.cc b3585da1f970 > > Diff: http://reviews.m5sim.org/r/838/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
