Review Board 2.0.15


X86: Move the MSR lookup table out of the TLB and into its own file.

Review Request #838 - Created Sept. 5, 2011 and submitted

Information
Gabe Black
gem5
Reviewers
Default
ali, gblack, nate, stever
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.

   
Posted (Sept. 20, 2011, 4:01 p.m.)
bump
Posted (Sept. 21, 2011, 2:02 a.m.)
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.
  1. 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.
Ship it!
Posted (Sept. 21, 2011, 1:44 p.m.)
I see, you're right, this is not the switch statement I thought it was.

I do like that the hash map is pretty compact (textually).