----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3743/#review9154 -----------------------------------------------------------
A few thing below. Overall, could you please add more comments? No need to write the entire TAGE paper in the comments, but a little high-level information would help new people coming to this code a lot. src/cpu/o3/O3CPU.py (line 142) <http://reviews.gem5.org/r/3743/#comment7866> I'm not sure we want to make the TAGE predictor the default. Are there any products that use something like the TAGE predictor? Alternatively, if the tournamentBP is *much* worse than current products, I can see the argument for make the TAGE predictor the default. src/cpu/pred/BranchPredictor.py (line 99) <http://reviews.gem5.org/r/3743/#comment7868> Is the global history really 128 megabytes? I'm not very familiar with the TAGE predictor. Does a real implementation somehow reduce this size and you're making a simplifying assumtion here? src/cpu/pred/ltage.hh (line 181) <http://reviews.gem5.org/r/3743/#comment7867> Can you add some comments on all of these functions? Doxygen comments would be best. src/cpu/pred/ltage.cc (line 68) <http://reviews.gem5.org/r/3743/#comment7869> I'm not sure if you're really using "histBufferSize" as a "MemorySize". This seems to just be a count that defaults to 2**27. Is this really a MemorySize? - Jason Lowe-Power On Nov. 22, 2016, 2:31 p.m., Arthur Perais wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3743/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2016, 2:31 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11707:1d085f66c4ca > --------------------------- > > cpu: implement an L-TAGE branch predictor > > This patch implements an L-TAGE predictor, based on André Seznec's code > available from > CBP-2 > (http://hpca23.cse.tamu.edu/taco/camino/cbp2/cbp-src/realistic-seznec.h). The > patch also changes the default branch predictor of o3 from the tournament > predictor > to L-TAGE. > > This patch requires patch #3727 (http://reviews.gem5.org/r/3727/) to compile. > > > Diffs > ----- > > src/cpu/o3/O3CPU.py 1d085f66c4ca > src/cpu/pred/BranchPredictor.py 1d085f66c4ca > src/cpu/pred/SConscript 1d085f66c4ca > src/cpu/pred/ltage.hh PRE-CREATION > src/cpu/pred/ltage.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3743/diff/ > > > Testing > ------- > > > Thanks, > > Arthur Perais > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
