----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1523/#review3646 -----------------------------------------------------------
Thanks for working on this Erik. We very much appreciate your contribution. I've got some questions below. Additionally, the quick regressions test very little functionality in the branch predictor. I'd be curious to see how the long regeressions turn out with your changes. There are a couple of places that don't quite seem right to me as far as differentiating between taken and not taken branches. Thanks, Ali src/cpu/pred/tournament.cc <http://reviews.gem5.org/r/1523/#comment3614> it would be great if you could remove it then src/cpu/pred/tournament.cc <http://reviews.gem5.org/r/1523/#comment3615> should this be the historyRegisterMask? Couldn't this be larger than the globalHistory was specified? This doesn't appear to do what the previous implementation did which was signify that the branch was taken (by oring in a 1 in the lsb). Both the taken and not taken versions are the same which doesn't seem correct. src/cpu/pred/tournament.cc <http://reviews.gem5.org/r/1523/#comment3616> you created the globalHistoryMask but you're not using it here? src/cpu/pred/tournament.cc <http://reviews.gem5.org/r/1523/#comment3618> a temporary variable would be very helpful here src/cpu/pred/tournament.cc <http://reviews.gem5.org/r/1523/#comment3617> it would be great to use a temporary variable here and make this much more readble - Ali Saidi On Oct. 29, 2012, 9:26 a.m., Erik Tomusk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1523/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2012, 9:26 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9356:55cf0b4cddc8 > --------------------------- > TournamentBP: Fix some bugs with table sizes and counters > > > Diffs > ----- > > src/cpu/pred/tournament.hh e71f71ce233a > src/cpu/pred/tournament.cc e71f71ce233a > > Diff: http://reviews.gem5.org/r/1523/diff/ > > > Testing > ------- > > Works with ARM SE quick regression (except for dram target, which fails > with and without this patch). > > > Thanks, > > Erik Tomusk > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
