If these are adapted from Rick's patches, what was the license on those patches?

src/cpu/BaseCPU.py (Diff revision 1)

Is this supposed to be here?  It doesn't seem related to McPAT.  What does it do anyway?  Also, this should be done by adding a couple of new params with default '=None' values instead of duplicating code (note that this already seems broken wrt ARM and interrupt support when comparing with the preceding version).

1. Brad Beckmann Jan. 10, 2011, 2:51 a.m. (Jan. 10, 2011, 2:51 a.m.)

   I'm going to defer this question to Joel.  It seems to me that this is unused and not related to McPAT.  However I want to hear from Joel before removing it.

src/cpu/base.hh (Diff revision 1)

This needs to be a System-object field, not a global, so it works when doing multi-system simulations.

1. Brad Beckmann Jan. 10, 2011, 2:51 a.m. (Jan. 10, 2011, 2:51 a.m.)

   Done

src/cpu/base.hh (Diff revision 1)

Same here.

1. Brad Beckmann Jan. 10, 2011, 2:51 a.m. (Jan. 10, 2011, 2:51 a.m.)

   Done

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

I'm curious why this counts as a read and a write... is the write just the update to remove the inst?  Shouldn't that be cheaper than actually writing a new instruction into the queue?

1. Brad Beckmann Jan. 10, 2011, 2:51 a.m. (Jan. 10, 2011, 2:51 a.m.)

   Joel, do you have any thoughts on this?

src/sim/system.hh (Diff revision 2)

I didn't modify this comment from Joel's/Rick's initial patch, but as currently stated it doesn't seem accurate. There is not a vector of per-thread event queues, but instead there is one per-system.   

src/sim/System.py (Diff revision 2)

Where is this parameter actually used?  For the latest version of the patch, moved it from BaseCPU to System, but it seesm like we should just remove it altogether.  Am I missing something?

src/cpu/o3/cpu.cc (Diff revision 2)

Is this the right place to be reseting this value?  Can we do it in system instead?

src/cpu/base.cc (Diff revision 2)

Don't add this blank line arbitrarily here.

src/cpu/base.cc (Diff revision 2)

Why is "static" needed here?

src/cpu/o3/cpu.cc (Diff revision 2)

readMiscRegNoEffect is for functional accesses and shouldn't modify any stats like this.

src/cpu/o3/cpu.cc (Diff revision 2)

No stats in NoEffect.

src/cpu/o3/cpu.cc (Diff revision 2)

Why would you increment by two?

src/cpu/o3/cpu.cc (Diff revision 2)

Why?

src/cpu/simple/base.hh (Diff revision 2)

No stats in NoEffect.

src/cpu/simple/base.cc (Diff revision 2)

Why not use idleFraction instead of constant(1.0) - notIdleFraction?

src/cpu/simple/base.cc (Diff revision 2)

Why caps? I see increments and not addition. How about a less jarring "/* Power model statistics. */"

src/cpu/simple/base.cc (Diff revision 2)

Don't need caps.