----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/14/#review2 -----------------------------------------------------------
This should be considered a first pass review which doesn't address every issue. I expect that future passes will be necessary. How much of the code here is directly copied from O3, and how much was modified or added for EDGE? It would be great if we could avoid all this duplication and share the common components between these CPUs. I realize O3 may not make that easy as it's written right now. It would also be great, although probably outside of the scope of what you're doing, to modularize/simplify O3's implementation so that's easier to do. src/cpu/SConscript <http://reviews.m5sim.org/r/14/#comment22> Why do you need special function signatures here? src/cpu/SConscript <http://reviews.m5sim.org/r/14/#comment23> This code will create exec signatures for all CPUs if any CPU is an edge CPU. I don't think that's what you wanted. src/cpu/edge/SConscript <http://reviews.m5sim.org/r/14/#comment28> If your flag has the same name as an existing one, that's actually a good thing. That way users don't have to learn a new set of options for doing basically the same thing. If you turn on the "Fetch" trace flag for whatever CPU your using, you should see what's going on in the fetch stage. Making an Edge version is unnecessary specialization. src/cpu/edge/base_dyn_inst_impl.hh <http://reviews.m5sim.org/r/14/#comment29> Don't comment out code, delete it. src/cpu/edge/exetrace.cc <http://reviews.m5sim.org/r/14/#comment21> This file doesn't need to exist. You should just use the original, not a direct copy. If you did make any changes, you should subclass this. Also, you seem to have more than one tracer. You should only need one. src/cpu/edge/insttracer.hh <http://reviews.m5sim.org/r/14/#comment20> I don't think this file needs to exist. You can just use the default tracer which this seems to just be a direct copy of. If you did make some modification, it should be a subclass, not a copy of the source. This removes the need to specially set the default tracer in BaseCPU.py. src/cpu/simple_thread.hh <http://reviews.m5sim.org/r/14/#comment24> What is this function for? Why can't it be built out of some other existing mechanism? src/cpu/simple_thread.cc <http://reviews.m5sim.org/r/14/#comment25> Don't commit whitespace only changes to a file. src/cpu/static_inst.hh <http://reviews.m5sim.org/r/14/#comment26> This definitely doesn't go here. StaticInst is at a building block and should have no knowledge of TRIPS or the edge CPU. src/cpu/static_inst.hh <http://reviews.m5sim.org/r/14/#comment27> What are these flags for? Can they be implemented in your subclasses? If they're only consumed in your ISA description they should be in a subclass. There shouldn't be any overt coupling between your ISA and your CPU model. If your CPU model needs these flags to work, we should try to figure out a way to move that to the ISA implementation and not force it on all the other CPU models and ISAs. M5 is complicated enough without details of their use leaking into generic components. - Gabe On 2010-04-28 18:52:04, Gou Pengfei wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/14/ > ----------------------------------------------------------- > > (Updated 2010-04-28 18:52:04) > > > Review request for Default, Ali Saidi and Nathan Binkert. > > > Summary > ------- > > Adding a cpu model named simpleEdgeCPU to support EDGE style execution. Most > of these codes are borrowed from O3. There're some outstanding features: > 1) 4-stage model including fetch, map, execute and commit, to target on > modeling the common characteristics of EDGE. > 2) Adding a class named edgeBlock to hold information of an EDGE inst block. > Right now it is mainly for the TRIPS inst block. > 3) Fetch will fetch instructions and form inst blocks. > 4) Map is planned to implement some dynamic mapping algorithms but right it's > just a dummy stage. > 5) Execute will execute inst in EDGE style, say, waking dependent insts > directly without renaming. > 6) Commit will commit in granularity of inst blocks. > > > Diffs > ----- > > src/cpu/BaseCPU.py edde97a6ea7c > src/cpu/SConscript edde97a6ea7c > src/cpu/edge/EdgeExeTracer.py PRE-CREATION > src/cpu/edge/EdgeInstTracer.py PRE-CREATION > src/cpu/edge/FUPool.py PRE-CREATION > src/cpu/edge/FuncUnitConfig.py PRE-CREATION > src/cpu/edge/SConscript PRE-CREATION > src/cpu/edge/SConsopts PRE-CREATION > src/cpu/edge/SimpleEdgeCPU.py PRE-CREATION > src/cpu/edge/base_block.hh PRE-CREATION > src/cpu/edge/base_block.cc PRE-CREATION > src/cpu/edge/base_block_impl.hh PRE-CREATION > src/cpu/edge/base_dyn_inst.hh PRE-CREATION > src/cpu/edge/base_dyn_inst.cc PRE-CREATION > src/cpu/edge/base_dyn_inst_impl.hh PRE-CREATION > src/cpu/edge/block.hh PRE-CREATION > src/cpu/edge/block.cc PRE-CREATION > src/cpu/edge/block_impl.hh PRE-CREATION > src/cpu/edge/bpred_unit.hh PRE-CREATION > src/cpu/edge/bpred_unit.cc PRE-CREATION > src/cpu/edge/bpred_unit_impl.hh PRE-CREATION > src/cpu/edge/comm.hh PRE-CREATION > src/cpu/edge/commit.hh PRE-CREATION > src/cpu/edge/commit.cc PRE-CREATION > src/cpu/edge/commit_impl.hh PRE-CREATION > src/cpu/edge/cpu.hh PRE-CREATION > src/cpu/edge/cpu.cc PRE-CREATION > src/cpu/edge/cpu_builder.cc PRE-CREATION > src/cpu/edge/cpu_policy.hh PRE-CREATION > src/cpu/edge/dep_graph.hh PRE-CREATION > src/cpu/edge/dyn_inst.hh PRE-CREATION > src/cpu/edge/dyn_inst.cc PRE-CREATION > src/cpu/edge/dyn_inst_impl.hh PRE-CREATION > src/cpu/edge/execute.hh PRE-CREATION > src/cpu/edge/execute.cc PRE-CREATION > src/cpu/edge/execute_impl.hh PRE-CREATION > src/cpu/edge/exetrace.hh PRE-CREATION > src/cpu/edge/exetrace.cc PRE-CREATION > src/cpu/edge/fetch.hh PRE-CREATION > src/cpu/edge/fetch.cc PRE-CREATION > src/cpu/edge/fetch_impl.hh PRE-CREATION > src/cpu/edge/fu_pool.hh PRE-CREATION > src/cpu/edge/fu_pool.cc PRE-CREATION > src/cpu/edge/global_regfile.hh PRE-CREATION > src/cpu/edge/impl.hh PRE-CREATION > src/cpu/edge/inst_queue.hh PRE-CREATION > src/cpu/edge/inst_queue.cc PRE-CREATION > src/cpu/edge/inst_queue_impl.hh PRE-CREATION > src/cpu/edge/insttracer.hh PRE-CREATION > src/cpu/edge/isa_specific.hh PRE-CREATION > src/cpu/edge/lsq.hh PRE-CREATION > src/cpu/edge/lsq.cc PRE-CREATION > src/cpu/edge/lsq_impl.hh PRE-CREATION > src/cpu/edge/lsq_unit.hh PRE-CREATION > src/cpu/edge/lsq_unit.cc PRE-CREATION > src/cpu/edge/lsq_unit_impl.hh PRE-CREATION > src/cpu/edge/map.hh PRE-CREATION > src/cpu/edge/map.cc PRE-CREATION > src/cpu/edge/map_impl.hh PRE-CREATION > src/cpu/edge/mem_dep_unit.hh PRE-CREATION > src/cpu/edge/mem_dep_unit.cc PRE-CREATION > src/cpu/edge/mem_dep_unit_impl.hh PRE-CREATION > src/cpu/edge/pred/2bit_local.hh PRE-CREATION > src/cpu/edge/pred/2bit_local.cc PRE-CREATION > src/cpu/edge/pred/SConscript PRE-CREATION > src/cpu/edge/pred/btb.hh PRE-CREATION > src/cpu/edge/pred/btb.cc PRE-CREATION > src/cpu/edge/pred/btp.hh PRE-CREATION > src/cpu/edge/pred/btp.cc PRE-CREATION > src/cpu/edge/pred/ctb.hh PRE-CREATION > src/cpu/edge/pred/ctb.cc PRE-CREATION > src/cpu/edge/pred/ibtb.hh PRE-CREATION > src/cpu/edge/pred/ibtb.cc PRE-CREATION > src/cpu/edge/pred/ras.hh PRE-CREATION > src/cpu/edge/pred/ras.cc PRE-CREATION > src/cpu/edge/pred/tournament.hh PRE-CREATION > src/cpu/edge/pred/tournament.cc PRE-CREATION > src/cpu/edge/rob.hh PRE-CREATION > src/cpu/edge/rob.cc PRE-CREATION > src/cpu/edge/rob_impl.hh PRE-CREATION > src/cpu/edge/sat_counter.hh PRE-CREATION > src/cpu/edge/sat_counter.cc PRE-CREATION > src/cpu/edge/static_inst.hh PRE-CREATION > src/cpu/edge/static_inst.cc PRE-CREATION > src/cpu/edge/store_set.hh PRE-CREATION > src/cpu/edge/store_set.cc PRE-CREATION > src/cpu/edge/thread_context.hh PRE-CREATION > src/cpu/edge/thread_context.cc PRE-CREATION > src/cpu/edge/thread_context_impl.hh PRE-CREATION > src/cpu/edge/thread_state.hh PRE-CREATION > src/cpu/simple_thread.hh edde97a6ea7c > src/cpu/simple_thread.cc edde97a6ea7c > src/cpu/static_inst.hh edde97a6ea7c > src/cpu/static_inst.cc edde97a6ea7c > src/cpu/thread_context.hh edde97a6ea7c > > Diff: http://reviews.m5sim.org/r/14/diff > > > Testing > ------- > > This model with TRIPS ISA support can run SPEC CPU2000 using test input set. > > build cmd lines: > > % scons CPU_MODELS=simpleEdgeCPU build/TRIPS_SE/m5.debug > > > Thanks, > > Gou > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
