----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3339/#review8131 -----------------------------------------------------------
src/cpu/minor/decode.hh (line 90) <http://reviews.gem5.org/r/3339/#comment6994> Style: all other class defns. have the { on the next line. src/cpu/minor/decode.hh (line 146) <http://reviews.gem5.org/r/3339/#comment6995> Dropping Reservable is a good move. src/cpu/minor/decode.cc (line 182) <http://reviews.gem5.org/r/3339/#comment6996> Over indendation. Pull the first line up to the = ? src/cpu/minor/decode.cc (line 196) <http://reviews.gem5.org/r/3339/#comment6997> Add [tid:] to this and other DPRINTFs like you've done in other cases. src/cpu/minor/decode.cc (line 317) <http://reviews.gem5.org/r/3339/#comment6998> Drop open { on multi-line condition to next line to match the rest of the code. src/cpu/minor/decode.cc (line 342) <http://reviews.gem5.org/r/3339/#comment6999> minorTrace output really should reflect the threading. I notice you've added a thread index to (some?) blocks. Are you planning to dump all thread info in minorTrace() in another patch? src/cpu/minor/execute.hh (line 188) <http://reviews.gem5.org/r/3339/#comment7000> Doesn't this disappear into the thread info? src/cpu/minor/execute.cc <http://reviews.gem5.org/r/3339/#comment7001> Is this missing clause correct? The only_issue_microops flag is a bodgy way of handling draining, do you replace it with another mechanism? src/cpu/minor/execute.cc (line 1197) <http://reviews.gem5.org/r/3339/#comment7002> Id the full id comparison done to include the threadId? As the execSeqNums will be the same (as you didn't pass the condition on the previous clause), just comparing thread ids will work here, won't it? src/cpu/minor/execute.cc (line 1282) <http://reviews.gem5.org/r/3339/#comment7003> "Unstalling fu: %d" ... src/cpu/minor/execute.cc (line 1493) <http://reviews.gem5.org/r/3339/#comment7004> Original spelling was correct. src/cpu/minor/execute.cc (line 1514) <http://reviews.gem5.org/r/3339/#comment7005> Next line before { src/cpu/minor/fetch1.hh (line 236) <http://reviews.gem5.org/r/3339/#comment7006> <nl> { src/cpu/minor/fetch1.hh (line 238) <http://reviews.gem5.org/r/3339/#comment7007> Spelling src/cpu/minor/fetch1.cc (line 137) <http://reviews.gem5.org/r/3339/#comment7008> <nl> { src/cpu/minor/fetch1.cc (line 764) <http://reviews.gem5.org/r/3339/#comment7009> He he src/cpu/minor/fetch2.hh (line 100) <http://reviews.gem5.org/r/3339/#comment7010> Style util/minorview/minor.pic (line 118) <http://reviews.gem5.org/r/3339/#comment7011> Add more than one set of thread structures to the default .pic file? - Andrew Bardsley On Feb. 23, 2016, 8:58 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3339/ > ----------------------------------------------------------- > > (Updated Feb. 23, 2016, 8:58 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > cpu: Add SMT support to MinorCPU > > This patch adds SMT support to the MinorCPU. Currently > RoundRobin or Random thread scheduling are supported. > > > Diffs > ----- > > src/cpu/minor/MinorCPU.py ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/cpu.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/cpu.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/decode.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/decode.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/dyn_inst.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/exec_context.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/execute.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/execute.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/fetch1.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/fetch1.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/fetch2.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/fetch2.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/lsq.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/lsq.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/pipe_data.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/pipe_data.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/pipeline.hh ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/cpu/minor/pipeline.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > src/sim/pseudo_inst.cc ef6e57ac0d708aff0af51c77ff0aee2c069993cf > util/minorview/minor.pic ef6e57ac0d708aff0af51c77ff0aee2c069993cf > > Diff: http://reviews.gem5.org/r/3339/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
