> On Sept. 10, 2012, 7:58 p.m., Joel Hestness wrote: > > configs/ruby/Ruby.py, line 182 > > <http://reviews.gem5.org/r/1366/diff/1/?file=28979#file28979line182> > > > > Is this distinction necessary? When the number of directory bits is 0, > > the DirectoryMemory doesn't use the numa_high_bit > > (mapAddressToDirectoryVersion and mapAddressToLocalIdx). Eliminating this > > if-statement and just setting numa_bit = block_size_bits + dir_bits - 1 > > would simplify the code and make it easier to debug directory mapping mods. > > Brad Beckmann wrote: > We could do that, but it seems awkward to set the numa_bit to be within > the block offset when the number of directories is 1. Actually, I think > there may be an assertion somewhere else in the code that checks against that. >
I think you might be referring to the bitSelect and bitRemove assertions that the lower bit is less than or equal to the upper bit. These functions are only called from mapAddressToDirectoryVersion and mapAddressToLocalIdx when the number of directories is > 1 (dir_bits > 0), so collapsing this if-statement doesn't change the result of simulation. My previous note also says something stronger: numa_high_bit is set but never referenced when there is only one directory. > On Sept. 10, 2012, 7:58 p.m., Joel Hestness wrote: > > configs/ruby/MOESI_hammer.py, line 152 > > <http://reviews.gem5.org/r/1366/diff/1/?file=28978#file28978line152> > > > > All of this probe filter setup should be predicated on whether the > > probe filter is actually enabled (options.pf_on = True). If the probe > > filter is NOT enabled, then it should be possible to set > > options.numa_high_bit within the pf_bits range (not currently possible > > because of this assertion). Trying to fix up the pf_start_bit without first > > checking whether the probe filter is enabled will probably cause redundant > > changes down the road. > > > > Is anyone actually using the probe filter? > > Brad Beckmann wrote: > I still use it and I suspect others find it useful as well. Sure we can > wrap the current set of computation under the assumption of whether the probe > filter or full-bit directoy is enabled. Cool. Thanks for the head's up. I've incorporated this into our updated patch which we'll resubmit for review. I'm also a bit confused about the default setting of numa_high_bit when the probe filter or full-bit directory is enabled. If you don't explicitly set numa_high_bit, it will default to one of the bits just above the block offset (depending on number of directories), which according to this assertion, should fail. Simulation seems to complete correctly with pf_on or dir_on, and the default numa_high_bit. Is that appropriate desired functionality? - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1366/#review3436 ----------------------------------------------------------- On Aug. 22, 2012, 12:15 p.m., Jason Power wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1366/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2012, 12:15 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9167:2bac206316cc > --------------------------- > Ruby: Fix hard-coded block size assumption for multiple directories > > > Diffs > ----- > > configs/ruby/MOESI_hammer.py 019047ead23b > configs/ruby/Ruby.py 019047ead23b > > Diff: http://reviews.gem5.org/r/1366/diff/ > > > Testing > ------- > > > Thanks, > > Jason Power > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
