> On Sept. 23, 2015, 5:20 p.m., Nilay Vaish wrote: > > Seems fine to me. Just one question, is it necessary to keep > > the lookup result as a class variable? Can you not pass it > > around after the first lookup? Some how, I do not like doing this. > > Andreas Hansson wrote: > You are not the only one. We have had many heated debates, and the main > argument for using the class variable is to not have to expose the types > outside the snoop filter, and leave the responsibility of tracking very > specific iterators to the crossbar. Not great...but it still seems like the > best solution. > > Nilay Vaish wrote: > Are you sure it is reliable to store the iterator? Typically any > modification to the > underlying data structure invalidates them. en.cppreference.com tells me > that insertion and emplace > to an unordered_map may invalidate all iterators. > > Andreas Hansson wrote: > The second function needs to be called before any changes are made. It's > not great...but I cannot think or any better way of doing it. > > Nilay Vaish wrote: > Did you consider returning the iterator from lookupRequest()? > > Nilay Vaish wrote: > OK, now I get your point on not exposing types to the crossbar.
As I mentioned earlier, we have had many heated debates, and the main argument for using the class variable is to not have to expose the types outside the snoop filter, and leave the responsibility of tracking very specific iterators to the crossbar. Not great...but it still seems like the best solution. Otherwise we are just pushing the problem/responsibility to the crossbar. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3059/#review7256 ----------------------------------------------------------- On Aug. 21, 2015, 3:49 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3059/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2015, 3:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11062:85ac67b53235 > --------------------------- > mem: Store snoop filter lookup result to avoid second lookup > > This patch introduces a private member storing the iterator from the > lookupRequest call, such that it can be re-used when the request > eventually finishes. The method previously called updateRequest is > renamed finishRequest to make it more clear that the two functions > must be called together. > > > Diffs > ----- > > src/mem/coherent_xbar.cc 842f56345a42 > src/mem/snoop_filter.hh 842f56345a42 > src/mem/snoop_filter.cc 842f56345a42 > > Diff: http://reviews.gem5.org/r/3059/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
