I'm pretty sure I mentioned twice that this is likely not the right way to do this. The ARM predecoder should consume the bytes it has, not ensure through special behavior in fetch that what it needs will always be there. I haven't tried it yet, but I remember having concerns that this would break x86 as well. On top of that, there are style problems.
Gabe Matt Horsnell wrote: > changeset b2c7e56572a4 in /z/repo/m5 > details: http://repo.m5sim.org/m5?cmd=changeset;node=b2c7e56572a4 > description: > O3: Fix some variable length instruction issues with the O3 CPU and ARM > ISA. > > diffstat: > > src/arch/arm/predecoder.cc | 15 +++++++++++---- > src/arch/arm/predecoder.hh | 22 ++++++++++++++++++---- > src/cpu/o3/fetch_impl.hh | 14 ++++++++++---- > 3 files changed, 39 insertions(+), 12 deletions(-) > > diffs (184 lines): > > diff -r d25827665112 -r b2c7e56572a4 src/arch/arm/predecoder.cc > --- a/src/arch/arm/predecoder.cc Tue Jan 18 16:30:05 2011 -0600 > +++ b/src/arch/arm/predecoder.cc Tue Jan 18 16:30:05 2011 -0600 > @@ -74,11 +74,15 @@ > void > Predecoder::process() > { > + // emi is typically ready, with some caveats below... > + emiReady = true; > + > if (!emi.thumb) { > emi.instBits = data; > emi.sevenAndFour = bits(data, 7) && bits(data, 4); > emi.isMisc = (bits(data, 24, 23) == 0x2 && > bits(data, 20) == 0); > + consumeBytes(4); > DPRINTF(Predecoder, "Arm inst: %#x.\n", (uint64_t)emi); > } else { > uint16_t word = (data >> (offset * 8)); > @@ -86,7 +90,7 @@ > // A 32 bit thumb inst is half collected. > emi.instBits = emi.instBits | word; > bigThumb = false; > - offset += 2; > + consumeBytes(2); > DPRINTF(Predecoder, "Second half of 32 bit Thumb: %#x.\n", > emi.instBits); > if (itstate.mask) { > @@ -105,7 +109,7 @@ > emi.instBits = (data >> 16) | (data << 16); > DPRINTF(Predecoder, "All of 32 bit Thumb: %#x.\n", > emi.instBits); > - offset += 4; > + consumeBytes(4); > if (itstate.mask) { > emi.itstate = itstate; > advanceThumbCond(); > @@ -117,11 +121,13 @@ > "First half of 32 bit Thumb.\n"); > emi.instBits = (uint32_t)word << 16; > bigThumb = true; > - offset += 2; > + consumeBytes(2); > + // emi not ready yet. > + emiReady = false; > } > } else { > // A 16 bit thumb inst. > - offset += 2; > + consumeBytes(2); > emi.instBits = word; > // Set the condition code field artificially. > emi.condCode = COND_UC; > @@ -159,6 +165,7 @@ > CPSR cpsr = tc->readMiscReg(MISCREG_CPSR); > itstate.top6 = cpsr.it2; > itstate.bottom2 = cpsr.it1; > + outOfBytes = false; > process(); > } > > diff -r d25827665112 -r b2c7e56572a4 src/arch/arm/predecoder.hh > --- a/src/arch/arm/predecoder.hh Tue Jan 18 16:30:05 2011 -0600 > +++ b/src/arch/arm/predecoder.hh Tue Jan 18 16:30:05 2011 -0600 > @@ -45,6 +45,8 @@ > #ifndef __ARCH_ARM_PREDECODER_HH__ > #define __ARCH_ARM_PREDECODER_HH__ > > +#include <cassert> > + > #include "arch/arm/types.hh" > #include "arch/arm/miscregs.hh" > #include "base/types.hh" > @@ -61,6 +63,8 @@ > ExtMachInst emi; > MachInst data; > bool bigThumb; > + bool emiReady; > + bool outOfBytes; > int offset; > ITSTATE itstate; > > @@ -70,6 +74,8 @@ > bigThumb = false; > offset = 0; > emi = 0; > + emiReady = false; > + outOfBytes = true; > } > > Predecoder(ThreadContext * _tc) : > @@ -103,16 +109,22 @@ > moreBytes(0, 0, machInst); > } > > + inline void consumeBytes(int numBytes) > + { > + offset += numBytes; > + assert(offset <= sizeof(MachInst)); > + if (offset == sizeof(MachInst)) > + outOfBytes = true; > + } > + > bool needMoreBytes() > { > - return sizeof(MachInst) > offset; > + return outOfBytes; > } > > bool extMachInstReady() > { > - // The only way an instruction wouldn't be ready is if this is a > - // 32 bit ARM instruction that's not 32 bit aligned. > - return !bigThumb; > + return emiReady; > } > > int getInstSize() > @@ -123,9 +135,11 @@ > //This returns a constant reference to the ExtMachInst to avoid a > copy > ExtMachInst getExtMachInst(PCState &pc) > { > + assert(emiReady); > ExtMachInst thisEmi = emi; > pc.npc(pc.pc() + getInstSize()); > emi = 0; > + emiReady = false; > return thisEmi; > } > }; > diff -r d25827665112 -r b2c7e56572a4 src/cpu/o3/fetch_impl.hh > --- a/src/cpu/o3/fetch_impl.hh Tue Jan 18 16:30:05 2011 -0600 > +++ b/src/cpu/o3/fetch_impl.hh Tue Jan 18 16:30:05 2011 -0600 > @@ -384,7 +384,7 @@ > { > ThreadID tid = pkt->req->threadId(); > > - DPRINTF(Fetch, "[tid:%u] Waking up from cache miss.\n",tid); > + DPRINTF(Fetch, "[tid:%u] Waking up from cache miss.\n", tid); > > assert(!pkt->wasNacked()); > > @@ -1011,7 +1011,7 @@ > instruction->setThreadState(cpu->thread[tid]); > > DPRINTF(Fetch, "[tid:%i]: Instruction PC %#x (%d) created " > - "[sn:%lli]\n", tid, thisPC.instAddr(), > + "[sn:%lli].\n", tid, thisPC.instAddr(), > thisPC.microPC(), seq); > > DPRINTF(Fetch, "[tid:%i]: Instruction is: %s\n", tid, > @@ -1180,7 +1180,6 @@ > ExtMachInst extMachInst; > > extMachInst = predecoder.getExtMachInst(thisPC); > - pcOffset = 0; > staticInst = StaticInstPtr(extMachInst, > thisPC.instAddr()); > > @@ -1188,7 +1187,12 @@ > ++fetchedInsts; > > if (staticInst->isMacroop()) > + { > curMacroop = staticInst; > + } > + else { > + pcOffset = 0; > + } > } else { > // We need more bytes for this instruction. > break; > @@ -1196,8 +1200,10 @@ > } > if (curMacroop) { > staticInst = curMacroop->fetchMicroop(thisPC.microPC()); > - if (staticInst->isLastMicroop()) > + if (staticInst->isLastMicroop()) { > curMacroop = NULL; > + pcOffset = 0; > + } > } > > DynInstPtr instruction = > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
