CheckerCPU: Re-factor CheckerCPU to be compatible current state of gem5
Review Request #907 - Created Nov. 3, 2011 and submitted
| Information | |
|---|---|
| Ali Saidi | |
| gem5 | |
| Reviewers | |
| Default | |
| ali, gblack, nate, stever | |
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.
Posted (Nov. 5, 2011, 6:51 p.m.)
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 (Diff revision 1) -
Get rid of debugging output.
-
src/cpu/BaseCPU.py (Diff revision 1) -
What's this doing here?
-
src/cpu/base_dyn_inst.hh (Diff revision 1) -
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 (Diff revision 1) -
Don't comment out code, delete it.
-
src/cpu/checker/cpu_impl.hh (Diff revision 1) -
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 (Diff revision 1) -
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 (Diff revision 1) -
This is another Alpha-ism I'd like to clean up one of these days.
-
src/cpu/checker/cpu_impl.hh (Diff revision 1) -
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 (Diff revision 1) -
Line length?
-
src/cpu/checker/thread_context.hh (Diff revision 1) -
Line length?
-
src/cpu/o3/fetch_impl.hh (Diff revision 1) -
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 (Diff revision 1) -
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 (Diff revision 1) -
Does this belong in the ThreadContext class? I doubt it, but I'm not completely sure.
-
src/mem/packet.hh (Diff revision 1) -
by => be
-
src/mem/packet.cc (Diff revision 1) -
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.
