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

Reply via email to