> On April 8, 2012, 5:33 p.m., Steve Reinhardt wrote: > > Looks basically ok. I'd prefer to see this merged with the unit test and > > the x86 TLB patch and committed as a single changeset.
It makes sense to merge this with the unit test since you can make a pretty convincing argument that they're a unit. Even though they're different things, they're two parts of a single whole, going along two different dimensions. Using the new data structure in the x86 TLB is a different change and needs to be in a different CL. > On April 8, 2012, 5:33 p.m., Steve Reinhardt wrote: > > src/sim/addrtrie.hh, line 38 > > <http://reviews.gem5.org/r/1141/diff/1/?file=25679#file25679line38> > > > > What about templating on the key type too (Addr)? Then it would be a > > totally generic trie and should definitely go in base and not sim. It would be slightly more generic, but really you'd just be able to change the size of the integer key. It wouldn't work with, for instance, a string key, or if you wanted the most specific match like for IP routing, etc. I'll do it anyway since it's not a totally unreasonable thing to do, and someone may make it more genuinely generic in the future. > On April 8, 2012, 5:33 p.m., Steve Reinhardt wrote: > > src/sim/addrtrie.hh, line 144 > > <http://reviews.gem5.org/r/1141/diff/1/?file=25679#file25679line144> > > > > Not sure if the official style guide says anything, but my gut (and I > > think emacs) would line up the two goesAfter() calls, not offset them by > > one space. This is because it's on the next line, and I think my editor automatically indented it two stops/eight spaces. That happens to almost but not quite line up with the line above. I don't care greatly which way it's done, so I'll line it up. > On April 8, 2012, 5:33 p.m., Steve Reinhardt wrote: > > src/sim/addrtrie.hh, line 207 > > <http://reviews.gem5.org/r/1141/diff/1/?file=25679#file25679line207> > > > > I don't see what you gain by separating lookup() and lookupNode(), > > since the latter is only called from the former, and the separation > > requires an extra check of node for being non-null. If you don't record the Node pointer when you insert something, you need a way to get at it later so you can delete that mapping. Otherwise there's no way to retrieve the Node, just the value it contains. I'm pretty sure the compiler will inline one into the other anyway and be able to get rid of the NULL check. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1141/#review2510 ----------------------------------------------------------- On April 8, 2012, 1:02 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1141/ > ----------------------------------------------------------- > > (Updated April 8, 2012, 1:02 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8943:95056bc98bf5 > --------------------------- > sim: A trie data structure specifically to speed up paging lookups. > > This change adds a trie data structure which stores an arbitrary pointer type > based on an address and a number of relevant bits. Then lookups can be done > against the trie where the tree is traversed and the first legitimate match > found is retuned. > > > Diffs > ----- > > src/sim/addrtrie.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/1141/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
