Yes, you are right, it's indeed not a general solution..
We might be able to solve it by adding an extra variable to
inorder_dyninst. Just a flag that let us check if the branch is
allready resolved.
What do you think?
By the way, I also made a blunder in my patch. Because I saw such a
good improvement of the branch predicter I didn't see this one (at
squash in cpu/inorder/resources/branch_predictor.cc):
bool taken = inst->predTaken();
should be
bool taken = !(corr_targ == (inst->readPC() +
sizeof(TheISA::MachInst)));
I though the predictTaken-flag of InOrderDynInst gets updated but it
doesn't =).
If you want I will do another mailpatch or try to bundle it in one
patch.
Greets,
Max
Op 14-apr-10, om 17:26 heeft Korey Sewell het volgende geschreven:
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
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
