-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1141/#review2510
-----------------------------------------------------------


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.


src/sim/addrtrie.hh
<http://reviews.gem5.org/r/1141/#comment2914>

    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.



src/sim/addrtrie.hh
<http://reviews.gem5.org/r/1141/#comment2915>

    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.



src/sim/addrtrie.hh
<http://reviews.gem5.org/r/1141/#comment2916>

    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.



src/sim/addrtrie.hh
<http://reviews.gem5.org/r/1141/#comment2917>

    i.e., just return node->value here instead of node and be done with it...


- Steve Reinhardt


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

Reply via email to