That's a good point. I tend to forget about checkpointing since I don't ever really use it. To be totally correct we should add that in to the serialized state, and since it's not that long since the last time we changed things around (yeah, I know, sorry about that) it shouldn't be very painful. If we were to set it to 0, then I -think- what would happen is either nothing if the pc wasn't a branch and the CPU was flushed, or there would be a slight hiccup from a spurious mispredict and then the CPU would correct things and continue.
My recommendation is definitely to add it to the checkpoint code (I'll try to add it sometime today, but I have a flight) since that minimizes hassles in the long term. I think Lisa added the capability to handle a field not being present, and if that does actually exist we could use that to default to 0. We should add a line to the checkpoints explicitly still, although it's probably not feasible to determine what size to use without looking at the bytes involved. Gabe On 03/04/11 13:14, Beckmann, Brad wrote: > Hi Gabe, > > In this changeset, you added the instruction size to the x86 PCState. Since > checkpoints created prior to this changeset won't include an instruction > size, is it safe to set that value to zero? If not, what can we do to > rectify old checkpoints? > > Thanks, > > Brad > > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> On Behalf Of Gabe Black >> Sent: Sunday, February 13, 2011 5:45 PM >> To: [email protected] >> Subject: [m5-dev] changeset in m5: X86: Only reset npc to reflect instruction >> leng... >> >> changeset be8762db2561 in /z/repo/m5 >> details: http://repo.m5sim.org/m5?cmd=changeset;node=be8762db2561 >> description: >> X86: Only reset npc to reflect instruction length once. >> >> When redirecting fetch to handle branches, the npc of the current pc >> state >> needs to be left alone. This change makes the pc state record >> whether or not >> the npc already reflects a real value by making it keep track of the >> current >> instruction size, or if no size has been set. >> >> diffstat: >> >> src/arch/x86/predecoder.hh | 6 ++++- >> src/arch/x86/types.hh | 50 >> +++++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 54 insertions(+), 2 deletions(-) >> >> diffs (76 lines): >> >> diff -r 6d955240bb62 -r be8762db2561 src/arch/x86/predecoder.hh >> --- a/src/arch/x86/predecoder.hh Sun Feb 13 17:40:07 2011 -0800 >> +++ b/src/arch/x86/predecoder.hh Sun Feb 13 17:41:10 2011 -0800 >> @@ -225,7 +225,11 @@ >> { >> assert(emiIsReady); >> emiIsReady = false; >> - nextPC.npc(nextPC.pc() + getInstSize()); >> + if (!nextPC.size()) { >> + Addr size = getInstSize(); >> + nextPC.size(size); >> + nextPC.npc(nextPC.pc() + size); >> + } >> return emi; >> } >> }; >> diff -r 6d955240bb62 -r be8762db2561 src/arch/x86/types.hh >> --- a/src/arch/x86/types.hh Sun Feb 13 17:40:07 2011 -0800 >> +++ b/src/arch/x86/types.hh Sun Feb 13 17:41:10 2011 -0800 >> @@ -222,7 +222,55 @@ >> return true; >> } >> >> - typedef GenericISA::UPCState<MachInst> PCState; >> + class PCState : public GenericISA::UPCState<MachInst> >> + { >> + protected: >> + typedef GenericISA::UPCState<MachInst> Base; >> + >> + uint8_t _size; >> + >> + public: >> + void >> + set(Addr val) >> + { >> + Base::set(val); >> + _size = 0; >> + } >> + >> + PCState() {} >> + PCState(Addr val) { set(val); } >> + >> + uint8_t size() const { return _size; } >> + void size(uint8_t newSize) { _size = newSize; } >> + >> + void >> + advance() >> + { >> + Base::advance(); >> + _size = 0; >> + } >> + >> + void >> + uEnd() >> + { >> + Base::uEnd(); >> + _size = 0; >> + } >> + >> + void >> + serialize(std::ostream &os) >> + { >> + Base::serialize(os); >> + SERIALIZE_SCALAR(_size); >> + } >> + >> + void >> + unserialize(Checkpoint *cp, const std::string §ion) >> + { >> + Base::unserialize(cp, section); >> + UNSERIALIZE_SCALAR(_size); >> + } >> + }; >> >> struct CoreSpecific { >> int core_type; >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
