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.

1 files changed, 12 insertions, 4 deletions

diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh
index 84f2c3506..545969d82 100644
--- a/src/cpu/o3/fetch_impl.hh
+++ b/src/cpu/o3/fetch_impl.hh
@@ -1301,6 +1301,10 @@ DefaultFetch<Impl>::fetch(bool &status_change)
                     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 @@ DefaultFetch<Impl>::fetch(bool &status_change)
                 } else {
                     staticInst = curMacroop->fetchMicroop(thisPC.microPC());
                 }
-                if (staticInst->isLastMicroop()) {
-                    curMacroop = NULL;
-                    pcOffset = 0;
-                }
+                newMacro |= staticInst->isLastMicroop();
             }
 
             DynInstPtr instruction =
@@ -1335,9 +1336,16 @@ DefaultFetch<Impl>::fetch(bool &status_change)
                 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!");