> 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.
> 
> Gabe Black wrote:
>     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.

It's not worth a big argument, but I disagree.  Checking in code that's unused 
seems pointless.  New code and the changes that use it seem highly related.  I 
won't fight with keeping it as two changesets, but if someone else proposed the 
same thing as a single changeset, I would not buy the argument that they should 
be split up.


- Steve


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


On April 10, 2012, 11:04 a.m., Gabe Black wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1141/
> -----------------------------------------------------------
> 
> (Updated April 10, 2012, 11:04 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 8943:524c4b58c710
> ---------------------------
> 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 returned.
> 
> 
> Diffs
> -----
> 
>   src/base/trie.hh PRE-CREATION 
>   src/unittest/SConscript 5534a564f6a0 
>   src/unittest/trietest.cc 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