changeset 6ee3a2359fcb in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=6ee3a2359fcb
description:
O3: Stop using the current macroop no matter why you're leaving it.
Until now, the only reason a macroop would be left was because it ended
at a
microop marked as the last microop. In O3 with branch prediction, it's
possible for the branch predictor to have entries which originally came
from
different instructions which happened to have the same RIP. This could
theoretically happen in many ways, but it was encountered specifically
when
different programs in different address spaces ran one after the other
in
X86_FS.
What would happen in that case was that the macroop would continue to be
looped over and microops fetched from it until it reached the last
microop
even though the macropc had moved out from under it. If things lined up
properly, this could mean that the end bytes of an instruction actually
fell
into the instruction sized block of memory after the one in the
predecoder.
The fetch loop implicitly assumes that the last instruction sized chunk
of
memory processed was the last one needed for the instruction it just
finished
executing. It would then tell the predecoder to move to an offset
within the
bytes it was given that is larger than those bytes, and that would trip
an
assert in the x86 predecoder.
This change fixes this problem by making fetch stop processing the
current
macroop if the address it should be fetching from changed when the PC is
updated. That happens when the last microop was reached because the
instruction
handled it properly, and it also catches the case where the branch
predictor
makes fetch do a macro level branch when it shouldn't.
The check of isLastMicroop is retained because otherwise, a macroop that
branches back to itself would act like a single, long macroop instead of
multiple instances of the same microop. There may be situations (which
may
turn out to be purely hypothetical) where that matters.
This also fixes a relatively minor issue where the curMacroop variable
would
be set to NULL immediately after seeing that a microop was the last one
before
curMacroop was used to build the dyninst. The traceData structure would
have a
NULL pointer to the macroop for that microop.
diffstat:
src/cpu/o3/fetch_impl.hh | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diffs (43 lines):
diff -r 0ef219f1d8ca -r 6ee3a2359fcb src/cpu/o3/fetch_impl.hh
--- a/src/cpu/o3/fetch_impl.hh Tue Aug 09 03:37:45 2011 -0700
+++ b/src/cpu/o3/fetch_impl.hh Tue Aug 09 11:30:43 2011 -0700
@@ -1301,6 +1301,10 @@
break;
}
}
+ // Whether we're moving to a new macroop because we're at the
+ // end of the current one, or the branch predictor incorrectly
+ // thinks we are...
+ bool newMacro = false;
if (curMacroop || inRom) {
if (inRom) {
staticInst = cpu->microcodeRom.fetchMicroop(
@@ -1308,10 +1312,7 @@
} else {
staticInst = curMacroop->fetchMicroop(thisPC.microPC());
}
- if (staticInst->isLastMicroop()) {
- curMacroop = NULL;
- pcOffset = 0;
- }
+ newMacro |= staticInst->isLastMicroop();
}
DynInstPtr instruction =
@@ -1335,9 +1336,16 @@
DPRINTF(Fetch, "Branch detected with PC = %s\n", thisPC);
}
+ newMacro |= thisPC.instAddr() != nextPC.instAddr();
+
// Move to the next instruction, unless we have a branch.
thisPC = nextPC;
+ if (newMacro) {
+ pcOffset = 0;
+ curMacroop = NULL;
+ }
+
if (instruction->isQuiesce()) {
DPRINTF(Fetch,
"Quiesce instruction encountered, halting fetch!");
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev