----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/907/#review1640 -----------------------------------------------------------
I'm glad to see the Checker CPU brought back from the dead, and I'm especially glad I didn't have to do it myself :-). This is a pretty big patch, and there are a number of places where things need to be fixed, there were questions, clarifications, etc. This isn't ready to commit but it's a solid start. I would request this gets broken into smaller patches that do one thing at a time instead of doing all this at once, but at this point that's probably impractical. Please shoot for that in the future though. src/cpu/BaseCPU.py <http://reviews.m5sim.org/r/907/#comment2119> Get rid of debugging output. src/cpu/BaseCPU.py <http://reviews.m5sim.org/r/907/#comment2120> What's this doing here? src/cpu/base_dyn_inst.hh <http://reviews.m5sim.org/r/907/#comment2121> This whole result thing is a relic that needs to go away. You shouldn't build your functionality on top of it. The results of the instruction are the memory request it generates, if any, and the destination registers it sets. You should frame things in those terms. I see you're changing the instResult from a scalar value to a queue which partially corrects one of its deficiencies, but it's still a redundant and obsolete mechanism. src/cpu/checker/cpu.cc <http://reviews.m5sim.org/r/907/#comment2124> Don't comment out code, delete it. src/cpu/checker/cpu_impl.hh <http://reviews.m5sim.org/r/907/#comment2127> This fetch logic is as gnarly as it is because O3 is doing branch prediction, microcode extraction, fetching a cache line at a time, blah blah blah. If you're doing simpler functional execution modeling you might get away with something a lot simpler modeled off of, say, the simple CPU. I'm not sure how the Checker CPU works in this area (or most others). src/cpu/checker/cpu_impl.hh <http://reviews.m5sim.org/r/907/#comment2125> Yes it is. It's unnecessary but accommodated. One of these days I'm planning to handle this in a better, less Alpha-y way. src/cpu/checker/cpu_impl.hh <http://reviews.m5sim.org/r/907/#comment2126> This is another Alpha-ism I'd like to clean up one of these days. src/cpu/checker/cpu_impl.hh <http://reviews.m5sim.org/r/907/#comment2128> The instructions themselves sort out if writes to registers are really using registers or the PC. From the CPU's perspective this should all be abstracted away. src/cpu/checker/thread_context.hh <http://reviews.m5sim.org/r/907/#comment2129> Line length? src/cpu/checker/thread_context.hh <http://reviews.m5sim.org/r/907/#comment2130> Line length? src/cpu/o3/fetch_impl.hh <http://reviews.m5sim.org/r/907/#comment2131> What are these for? I don't see anything changing here that would suggest you need them now but didn't before, although that's not to say that you needed them before and didn't have them. If these are leftovers please get rid of them. src/cpu/simple_thread.hh <http://reviews.m5sim.org/r/907/#comment2122> This seems like a more serious problem that should actually be tracked down rather than worked around. Maybe NumFloatRegs isn't high enough? Maybe the index isn't scaled/offset/whatever properly? src/cpu/thread_context.hh <http://reviews.m5sim.org/r/907/#comment2123> Does this belong in the ThreadContext class? I doubt it, but I'm not completely sure. src/mem/packet.hh <http://reviews.m5sim.org/r/907/#comment2132> by => be src/mem/packet.cc <http://reviews.m5sim.org/r/907/#comment2133> What's this function used for? There's a pretty decent chance I just missed it, but I didn't see where it was called by anything. If it's not used, then we don't need to make any changes to the Packet class. - Gabe On 2011-11-03 13:32:25, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/907/ > ----------------------------------------------------------- > > (Updated 2011-11-03 13:32:25) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > CheckerCPU: Re-factor CheckerCPU to be compatible current state of gem5 > > Brings the CheckerCPU back to a functioning state allowing FS and SE mode > checking of the O3CPU. Lacking compatibility with checkpoints and > fast-forwarding currently. These changes have only been tested on the > ARM ISA. Other ISAs should work, but some modification will be required. > > > Diffs > ----- > > build_opts/ARM_wCHECKER_FS PRE-CREATION > build_opts/ARM_wCHECKER_SE PRE-CREATION > src/arch/arm/isa.cc 5fb918115c07 > src/arch/arm/isa/insts/m5ops.isa 5fb918115c07 > src/arch/arm/isa/insts/misc.isa 5fb918115c07 > src/arch/arm/table_walker.hh 5fb918115c07 > src/arch/arm/table_walker.cc 5fb918115c07 > src/arch/arm/tlb.hh 5fb918115c07 > src/arch/arm/tlb.cc 5fb918115c07 > src/arch/arm/utility.cc 5fb918115c07 > src/cpu/BaseCPU.py 5fb918115c07 > src/cpu/CheckerCPU.py 5fb918115c07 > src/cpu/base.cc 5fb918115c07 > src/cpu/base_dyn_inst.hh 5fb918115c07 > src/cpu/base_dyn_inst_impl.hh 5fb918115c07 > src/cpu/checker/cpu.hh 5fb918115c07 > src/cpu/checker/cpu.cc 5fb918115c07 > src/cpu/checker/cpu_impl.hh 5fb918115c07 > src/cpu/checker/thread_context.hh 5fb918115c07 > src/cpu/o3/O3CPU.py 5fb918115c07 > src/cpu/o3/O3Checker.py 5fb918115c07 > src/cpu/o3/checker_builder.cc 5fb918115c07 > src/cpu/o3/commit_impl.hh 5fb918115c07 > src/cpu/o3/cpu.hh 5fb918115c07 > src/cpu/o3/cpu.cc 5fb918115c07 > src/cpu/o3/dyn_inst_impl.hh 5fb918115c07 > src/cpu/o3/fetch_impl.hh 5fb918115c07 > src/cpu/o3/iew_impl.hh 5fb918115c07 > src/cpu/o3/lsq_unit_impl.hh 5fb918115c07 > src/cpu/o3/thread_context.hh 5fb918115c07 > src/cpu/o3/thread_context_impl.hh 5fb918115c07 > src/cpu/simple_thread.hh 5fb918115c07 > src/cpu/thread_context.hh 5fb918115c07 > src/mem/bus.cc 5fb918115c07 > src/mem/packet.hh 5fb918115c07 > src/mem/packet.cc 5fb918115c07 > > Diff: http://reviews.m5sim.org/r/907/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
