I'm confused... this patch looks like it pretty much just inserts a bunch of DPRINTFs (plus the seemingly mistaken inclusion of a whitespace change in mips/isa/decoder.isa).
I don't know anything specific about the O3 predictor, but it makes sense that the history is updated speculatively in decode, but I don't see how you can even know whether a branch is mispredicted while you're still in the decode stage, so the fixup would have to wait at least until execute (and you may want to wait until commit if that makes recovery substantially simpler). Steve On Wed, Apr 15, 2009 at 8:15 PM, Korey Sewell <[email protected]> wrote: > This is the patch that will prevent me from committing. > > Basically, the decode stage is finding a mispredicted branch and then > deleting information out the branch history table for squashed instructions > AND itself. > > This messes things up for when the actual mispredicted instruction gets to > commit and wants to update the branch history. > > So basically, I ignored that condition and it branch prediction started > working. I did the same for ALPHA and SPARC and it seemed to work for Hello > World as well (maybe I just got lucky?) > > The next thing I guess I should do is run the quick/long regressions on > ALPHA/SPARC to make sure I'm not breaking anything if I were to remove that > code. > > Anybody have any thoughts on this (Gabe, Kevin?)? > > > > On Wed, Apr 15, 2009 at 11:06 PM, Korey Sewell <[email protected]>wrote: > >> # HG changeset patch >> # User Korey Sewell <[email protected]> >> # Date 1239850525 14400 >> # Node ID ce539a58327b78b049f8af0bf8a00737f2b97640 >> # Parent 7b685fee8773c2fe0bd09ec7abb0a43873aeed92 >> Fix Branch Prediction for MIPS. Namely, i dont see the reason for updating >> the branch history on anything that happens in decode. this should happen >> only in commit, right? >> >> diff --git a/src/arch/mips/isa/decoder.isa b/src/arch/mips/isa/decoder.isa >> --- a/src/arch/mips/isa/decoder.isa >> +++ b/src/arch/mips/isa/decoder.isa >> @@ -135,7 +135,7 @@ decode OPCODE_HI default Unknown::unknow >> 0x2: movz({{ Rd = (Rt == 0) ? Rs : Rd; }}); >> 0x3: movn({{ Rd = (Rt != 0) ? Rs : Rd; }}); >> #if FULL_SYSTEM >> - 0x4: syscall({{ >> + 0x4: syscall({{ >> fault = new SystemCallFault(); >> }}); >> #else >> diff --git a/src/cpu/o3/bpred_unit_impl.hh b/src/cpu/o3/bpred_unit_impl.hh >> --- a/src/cpu/o3/bpred_unit_impl.hh >> +++ b/src/cpu/o3/bpred_unit_impl.hh >> @@ -173,6 +173,10 @@ BPredUnit<Impl>::predict(DynInstPtr &ins >> tid, pred_taken, inst->readPC()); >> } >> >> + DPRINTF(Fetch, "BranchPred: [tid:%i]: [sn:%i] Creating prediction >> history " >> + "for PC %#x\n", >> + tid, inst->seqNum, inst->readPC()); >> + >> PredictorHistory predict_record(inst->seqNum, PC, pred_taken, >> bp_history, tid); >> >> @@ -249,7 +253,7 @@ void >> void >> BPredUnit<Impl>::update(const InstSeqNum &done_sn, unsigned tid) >> { >> - DPRINTF(Fetch, "BranchPred: [tid:%i]: Commiting branches until " >> + DPRINTF(Fetch, "BranchPred: [tid:%i]: Committing branches until " >> "[sn:%lli].\n", tid, done_sn); >> >> while (!predHist[tid].empty() && >> @@ -290,7 +294,12 @@ BPredUnit<Impl>::squash(const InstSeqNum >> // This call should delete the bpHistory. >> BPSquash(pred_hist.front().bpHistory); >> >> + DPRINTF(Fetch, "BranchPred: [tid:%i]: Removing history for >> [sn:%i] " >> + "PC %#x.\n", tid, pred_hist.front().seqNum, >> pred_hist.front().PC); >> + >> pred_hist.pop_front(); >> + >> + DPRINTF(Fetch, "[tid:%i]: predHist.size(): %i\n", tid, >> predHist[tid].size()); >> } >> >> } >> @@ -320,7 +329,13 @@ BPredUnit<Impl>::squash(const InstSeqNum >> // corresponding to the squash. In that case, don't bother trying to >> // fix up the entry. >> if (!pred_hist.empty()) { >> - assert(pred_hist.front().seqNum == squashed_sn); >> + if (pred_hist.front().seqNum != squashed_sn) >> + { >> + DPRINTF(Fetch, "PredHist.front.sn = %i | squashed_sn = >> %i.\n", >> + pred_hist.front().seqNum, squashed_sn); >> + assert(pred_hist.front().seqNum == squashed_sn); >> + } >> + >> if (pred_hist.front().usedRAS) { >> ++RASIncorrect; >> } >> @@ -329,7 +344,13 @@ BPredUnit<Impl>::squash(const InstSeqNum >> pred_hist.front().bpHistory); >> >> BTB.update(pred_hist.front().PC, corr_target, tid); >> + >> + DPRINTF(Fetch, "BranchPred: [tid:%i]: Removing history for >> [sn:%i] " >> + "PC %#x.\n", tid, pred_hist.front().seqNum, >> pred_hist.front().PC); >> + >> pred_hist.pop_front(); >> + >> + DPRINTF(Fetch, "[tid:%i]: predHist.size(): %i\n", tid, >> predHist[tid].size()); >> } >> } >> >> diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh >> --- a/src/cpu/o3/fetch_impl.hh >> +++ b/src/cpu/o3/fetch_impl.hh >> @@ -939,7 +939,11 @@ DefaultFetch<Impl>::checkSignalsAndUpdat >> "from decode.\n",tid); >> >> // Update the branch predictor. >> +#if THE_ISA == MIPS_ISA >> + if (fromDecode->decodeInfo[tid].branchMispredict && 0) { >> +#else >> if (fromDecode->decodeInfo[tid].branchMispredict) { >> +#endif >> branchPred.squash(fromDecode->decodeInfo[tid].doneSeqNum, >> fromDecode->decodeInfo[tid].nextPC, >> fromDecode->decodeInfo[tid].branchTaken, >> diff --git a/src/cpu/o3/iew_impl.hh b/src/cpu/o3/iew_impl.hh >> --- a/src/cpu/o3/iew_impl.hh >> +++ b/src/cpu/o3/iew_impl.hh >> @@ -1282,11 +1282,11 @@ DefaultIEW<Impl>::executeInsts() >> fetchRedirect[tid] = true; >> >> DPRINTF(IEW, "Execute: Branch mispredict detected.\n"); >> - DPRINTF(IEW, "Predicted target was %#x, %#x.\n", >> + DPRINTF(IEW, "Predicted target was PC:%#x, NPC:%#x.\n", >> inst->readPredPC(), inst->readPredNPC()); >> DPRINTF(IEW, "Execute: Redirecting fetch to PC: %#x," >> " NPC: %#x.\n", inst->readNextPC(), >> - inst->readNextNPC()); >> + inst->readNextPC()); >> // If incorrect, then signal the ROB that it must be >> squashed. >> squashDueToBranch(inst, tid); >> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> > > > > -- > ---------- > Korey L Sewell > Graduate Student - PhD Candidate > Computer Science & Engineering > University of Michigan > > _______________________________________________ > 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
