Sure, I can fix the style and ship this. Anyone have any concerns about this patch before I do (only has 1 ship it so far)?
Anthony Gutierrez http://web.eecs.umich.edu/~atgutier On Wed, May 15, 2013 at 1:53 PM, Korey Sewell <[email protected]> wrote: > Seems like Tony would be a good person to push this through since he > recently merged redundancy in the branch predictors. > > Tony, can I volunteer you for that? > > -Korey > > > On Sat, May 11, 2013 at 7:04 AM, Ali Saidi <[email protected]> wrote: > > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviews.gem5.org/r/1865/#review4325 > > ----------------------------------------------------------- > > > > > > Hi Taylor, > > > > It looks quite good and is very well documented. Thanks! There are a > > couple of gem5 style things that we need to correct. If you want to take > > care of them that is great, if not whomever commits the code can do it. > > I've commented on an instance of these below. > > > > Thanks again, > > Ali > > > > > > > > > > > > src/cpu/pred/global.cc > > <http://reviews.gem5.org/r/1865/#comment4079> > > > > please put authors on a separate line > > > > > > > > > > src/cpu/pred/global.cc > > <http://reviews.gem5.org/r/1865/#comment4082> > > > > please put the type on a separate line from the method name. > > > > > > > > > > src/cpu/pred/global.cc > > <http://reviews.gem5.org/r/1865/#comment4080> > > > > the indenting here should be fixed > > > > > > > > src/cpu/pred/global.cc > > <http://reviews.gem5.org/r/1865/#comment4081> > > > > please put a space between if and (taken) > > > > > > - Ali Saidi > > > > > > On May 10, 2013, 10:29 a.m., Taylor Lloyd wrote: > > > > > > ----------------------------------------------------------- > > > This is an automatically generated e-mail. To reply, visit: > > > http://reviews.gem5.org/r/1865/ > > > ----------------------------------------------------------- > > > > > > (Updated May 10, 2013, 10:29 a.m.) > > > > > > > > > Review request for Default. > > > > > > > > > Description > > > ------- > > > > > > This patch adds a global-global (gAG) branch predictor, used by > > specifying the "global" predType. > > > From BranchPredictor, it uses the following variables for > configuration: > > > - globalPredictorSize > > > - globalCtrBits > > > - globalHistoryBits > > > > > > As such, it may conflict with the ongoing effort to factor out the > > redundant variable in the predictors. > > > > > > > > > Diffs > > > ----- > > > > > > src/cpu/pred/SConscript 00dca8a9b560 > > > src/cpu/pred/bpred_unit.cc 00dca8a9b560 > > > src/cpu/pred/global.hh PRE-CREATION > > > src/cpu/pred/global.cc PRE-CREATION > > > > > > Diff: http://reviews.gem5.org/r/1865/diff/ > > > > > > > > > Testing > > > ------- > > > > > > Prior to submission, this was tested by building build/ARM/gem5.debug, > > and run against a subset of the SPEC CPU2006 benchmarks. Statistics > > generated seem valid, and it caused no execution errors. > > > > > > > > > Thanks, > > > > > > Taylor Lloyd > > > > > > > > > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > > > > > > -- > - Korey > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
