> On April 8, 2015, 1:43 p.m., Stephan Diestelhorst wrote: > > src/mem/cache/cache_impl.hh, line 1359 > > <http://reviews.gem5.org/r/2691/diff/1/?file=44226#file44226line1359> > > > > This should really be: > > > > if (!mshr.hasTargets()) > > > > and all the stuff below has too deep an (or two) indentation level, > > IMHO. I seriously consider goto an appropriate way to improve readability > > here.
Changing the test to 'if (!mshr.hasTargets())' is equivalent (I believe) but it's not clear to me that it's better... the only reason that condition could be true would be because early_exit is false, and early_exit is the more meaningful cause, I'd say. I could use a goto... as I mentioned above, I seriously considered it, but rejected it since it wouldn't completely replace early_exit. I'd have no problem with doing it here, but others might, so I was doing it as an if-block to avoid the controversy. I will post another version of the patch with a goto for comments on this specifically. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2691/#review6028 ----------------------------------------------------------- On May 6, 2015, 3:21 p.m., Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2691/ > ----------------------------------------------------------- > > (Updated May 6, 2015, 3:21 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10745:9b84d1b570e3 > --------------------------- > mem: implement x86 locked accesses in timing-mode classic cache > > Add LockedRMW(Read|Write)(Req|Resp) commands. In timing mode, > use a combination of clearing permission bits and leaving > an MSHR in place to prevent accesses & snoops from touching > a locked block between the read and write parts of an locked > RMW sequence. > > > Diffs > ----- > > src/mem/cache/cache_impl.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/cache/mshr.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/packet.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/packet.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2691/diff/ > > > Testing > ------- > > > Thanks, > > Steve Reinhardt > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
