src/arch/alpha/tlb.cc (Diff revision 1)

If these don't do anything and the caller won't expect/handle that, they should panic and say they're not implemented.

Show all issues

src/cpu/checker/cpu_impl.hh (Diff revision 1)

What's this from? I won't say it's not somewhere else or that it shouldn't be here, but I don't remember it. Also, the preprocessor is supposed to require #s to be all the way to the left.

Show all issues

src/cpu/checker/cpu_impl.hh (Diff revision 1)

You're checking for x86 here too. It's normally equivalent and generally harmless to compare ExtMachInsts instead of MachInsts. The difference is that the ExtMachInsts have been contextualized and the MachInsts are just the raw bytes from memory. In the x86 case exactly what should be the MachInst is ambiguous, but not the ExtMachInst.

Show all issues

1. Geoffrey Blake Feb. 10, 2012, 7:18 a.m. (Feb. 10, 2012, 7:18 a.m.)

   The original checker code assumes that a ExtMachInst and MachInst are equivalent and can be casted to each other.  For the RISC ISAs (ARM, POWER, MIPS, SPARC) this works.  As you say x86 doesn't behave this way and the x86 code won't compile.  This is just to make sure gem5 compiles.  The Checker really needs to be fixed to compare ExtMachInsts in the future so it works with all ISAs.  

2. Gabe Black Feb. 11, 2012, 4:42 a.m. (Feb. 11, 2012, 4:42 a.m.)

   Ok

src/cpu/o3/iew_impl.hh (Diff revision 1)

This looks pretty strange. Was this dead code? Did you bring it back to life here on purpose? Or in other words, is reintroducing it the right thing to do, or is it obsolete code?

Show all issues

1. Geoffrey Blake Feb. 10, 2012, 7:18 a.m. (Feb. 10, 2012, 7:18 a.m.)

   It was guarded by a #ifdef TARGET_ALPHA which has somewhere along the way ceased to exist.  Not sure if it's dead code or not.  If it is I'll just remove it.

2. Gabe Black Feb. 11, 2012, 4:42 a.m. (Feb. 11, 2012, 4:42 a.m.)

   I'm not sure either. Please take a look and see what you think. I'll leave it up to you.

3. Geoffrey Blake Feb. 13, 2012, 3:06 a.m. (Feb. 13, 2012, 3:06 a.m.)

   After digging some in the source this looks like code that should be included.  The ARM, x86 and ALPHA ISAs all have software prefetch instructions (IsDataPrefetch instruction flag in the ISA definitions) that this piece of code would count instead of lumping them with all the instructions.  There is another code snippet like this in src/cpu/o3/commit_impl.hh as well.  I guess Steve, Nate or Ali can offer their opinions on whether this stat is wanted or needed.

4. Ali Saidi Feb. 13, 2012, 3:16 a.m. (Feb. 13, 2012, 3:16 a.m.)

   i'm not sure why it was in there before and what this stat tells you. I would say it can go.

5. Steve Reinhardt Feb. 28, 2012, 3:13 p.m. (Feb. 28, 2012, 3:13 p.m.)

   I think this is an ancient artifact from some long-ago research that can (and should) be eliminated.  There's really no great justification for not counting software prefetches the same as other instructions.

Made requested changes to throw a panic in the non-implemented functions.  

Rebasing for recent changes.

src/cpu/o3/iew_impl.hh (Diff revision 1)

I'm a little confused... how does TARGET_ALPHA going away here correlate with compiling the checker CPU?

Split out the removal of the dead code in the O3 and Ozone cpu models as a separate patch.

Ship It!