----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3740/#review9170 -----------------------------------------------------------
Ship it! Looks good, thanks for fixing! You might want to consider the comments below before pushing the change. src/mem/comm_monitor.cc (line 280) <http://reviews.gem5.org/r/3740/#comment7887> This condition seems redunant since you check the same thing in the MonitorStats class. In fact, it might be a bit confusing if someone descides to extend the stat class in the future. src/mem/comm_monitor.cc (line 332) <http://reviews.gem5.org/r/3740/#comment7886> See above. - Andreas Sandberg On Nov. 21, 2016, 7:03 p.m., Rahul Thakur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3740/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2016, 7:03 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11708:33c0089caa50 > --------------------------- > commit 8606171b2c2e65d0b9931ccb4bd2ebc533c55d60 > Author: Rahul Thakur <rjtha...@google.com> > Date: Thu Oct 27 20:36:16 2016 -0700 > > mem: Refactor CommMonitor stats, add basic atomic mode stats > > Change-Id: I978f1155873b3882e16d9cd74e86400efd9c5e3b > > > Diffs > ----- > > src/mem/comm_monitor.hh 1d085f66c4ca > src/mem/comm_monitor.cc 1d085f66c4ca > > Diff: http://reviews.gem5.org/r/3740/diff/ > > > Testing > ------- > > > Thanks, > > Rahul Thakur > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev