> On April 14, 2012, 8:36 a.m., Steve Reinhardt wrote: > > src/arch/x86/tlb.cc, line 95 > > <http://reviews.gem5.org/r/1143/diff/4/?file=26044#file26044line95> > > > > 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.
That would imply that everything was already free, so really this function shouldn't have been called at all. > On April 14, 2012, 8:36 a.m., Steve Reinhardt wrote: > > src/arch/x86/tlb.cc, line 87 > > <http://reviews.gem5.org/r/1143/diff/4/?file=26044#file26044line87> > > > > 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? > > Yeah, this could be simplified a bit. I really don't like picking a fake max or min the real values are "sure" to beat out, because they aren't really certain to for all legal values, among other things. I'll take a different approach. > On April 14, 2012, 8:36 a.m., Steve Reinhardt wrote: > > src/arch/x86/tlb.cc, line 84 > > <http://reviews.gem5.org/r/1143/diff/4/?file=26044#file26044line84> > > > > Under what circumstances would size be 0? Should we even allow > > construction of a zero-size TLB? I was being paranoid, but it's probably not necessary/should be in the constructor. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1143/#review2543 ----------------------------------------------------------- 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
