OK I agree with the premise of the patch, but not quite sure if the solution is general enough yet.
Basically, you are saying if the instruction is executed (or resolved) then we should call one branch predictor squash function BUT if we are just making a prediction then we should call another squash function. In this patch you seem to be basing if a branch is resolved or not based on the stage. However, there is no guarantee that at a particular stage the branch is updated or not. BackEndStartStage should typically represent after the branch is predicted but not necessarily where that branch is resolved. So it looks like we somehow need to check: 1. If the squash is generating from a branch 2. If that branch has been resolved If we can do the above, then I think the code should work no matter where a user arbitrarily places the branch prediction in their pipeline description. On Tue, Apr 13, 2010 at 5:35 AM, Maximilien Breughe < [email protected]> wrote: > # HG changeset patch > # User Maximilien Breughe <[email protected]> > # Date 1271144284 -7200 > # Branch dev-perflab > # Node ID 401bc1d45798b11db33cf5c995c7c44fceacc4c3 > # Parent 9342cbe0c2c9fd94300a3762f518f3d4313f2951 > Bug fix: Another branch predictor fix > ============================ > > Branches can cause squashes for 2 reasons: > 1) branch prediction > 2) branch resolution > > Prediction happens in the Decode-stage. At this point, no information is > known about what path is taken. > Therefore a squash caused at prediction-time should not update the branch > predictor and hence the bug fix. > > Only squashes resulting from the branch resolution (in the Execution-stage) > can update the branch predictor. > > Another possibility was to update the branch predictor only at the > graduation-stage. > > diff -r 9342cbe0c2c9 -r 401bc1d45798 > src/cpu/inorder/resources/branch_predictor.cc > --- a/src/cpu/inorder/resources/branch_predictor.cc Fri Mar 26 09:57:52 > 2010 +0100 > +++ b/src/cpu/inorder/resources/branch_predictor.cc Tue Apr 13 09:38:04 > 2010 +0200 > @@ -142,10 +142,13 @@ > InstSeqNum squash_seq_num, ThreadID tid) > { > DPRINTF(InOrderBPred, "Squashing...\n"); > - Addr corr_targ=inst->readPredPC(); > - bool taken=inst->predTaken(); > - branchPred.squash(squash_seq_num,corr_targ,taken,tid); > -// branchPred.squash(squash_seq_num,tid); > + if(squash_stage>=ThePipeline::BackEndStartStage){ > + Addr corr_targ=inst->readPredPC(); > + bool taken=inst->predTaken(); > + branchPred.squash(squash_seq_num,corr_targ,taken,tid); > + } > + else > + branchPred.squash(squash_seq_num,tid); > } > > void > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > -- - Korey
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
