----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1143/#review2543 -----------------------------------------------------------
src/arch/x86/tlb.cc <http://reviews.gem5.org/r/1143/#comment2952> You've increased the number of comments here by infinity percent, so that's good, but this method still seems a little sparse... src/arch/x86/tlb.cc <http://reviews.gem5.org/r/1143/#comment2953> Under what circumstances would size be 0? Should we even allow construction of a zero-size TLB? src/arch/x86/tlb.cc <http://reviews.gem5.org/r/1143/#comment2954> So is this whole initial loop just to find an initial value for lru, so it's not undefined in the second loop where we look for the minimum value? If so, wouldn't it be easier to just initialize a minLruSeq variable with UINT64_MAX and use that? Also, wouldn't trieHandle be NULL only if the entry is invalid, in which case that's probably the entry we want to free up even if it's not LRU? src/arch/x86/tlb.cc <http://reviews.gem5.org/r/1143/#comment2955> What happens if we take this early return? Seems like in this case we didn't add anything to the free list, and the caller could be rather disappointed about that. - Steve Reinhardt On April 14, 2012, 3:39 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1143/ > ----------------------------------------------------------- > > (Updated April 14, 2012, 3:39 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8952:10e50c96f33e > --------------------------- > X86: Use the AddrTrie class to implement the TLB. > > This change also adjusts the TlbEntry class so that it stores the number of > address bits wide a page is rather than its size in bytes. In other words, > instead of storing 4K for a 4K page, it stores 12. 12 is easy to turn into 4K, > but it's a little harder going the other way. > > > Diffs > ----- > > src/arch/x86/pagetable.hh a6830d615eff > src/arch/x86/pagetable.cc a6830d615eff > src/arch/x86/pagetable_walker.hh a6830d615eff > src/arch/x86/pagetable_walker.cc a6830d615eff > src/arch/x86/tlb.hh a6830d615eff > src/arch/x86/tlb.cc a6830d615eff > src/arch/x86/vtophys.cc a6830d615eff > > Diff: http://reviews.gem5.org/r/1143/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
