Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/55589 )

Change subject: arch-x86: Correct how default segments are handled.
......................................................................

arch-x86: Correct how default segments are handled.

The stack segment is the default for instructions that use rSP or rBP in
their address calculations at all, except if they're used as a base.
Even though the wording in the AMD manual is a bit misleading, the
presence of a displacement does not make the default DS.

Also, allow segment override prefixes even if the default is SS. If an
instruction *must* use SS (like push or pop) it will have explicitly
specified that in the microcode.

Change-Id: I73c6e367440a664c5c7c483337c16d4ab14f0e34
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55589
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Gabe Black <gabe.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
---
M src/arch/x86/emulenv.cc
1 file changed, 28 insertions(+), 8 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/x86/emulenv.cc b/src/arch/x86/emulenv.cc
index 44afc5a..b628293 100644
--- a/src/arch/x86/emulenv.cc
+++ b/src/arch/x86/emulenv.cc
@@ -100,18 +100,16 @@
             }
         }
     }
-    //Figure out what segment to use. This won't be entirely accurate since
-    //the presence of a displacement is supposed to make the instruction
-    //default to the data segment.
-    if ((base != INTREG_RBP && base != INTREG_RSP) || machInst.dispSize) {
+    //Figure out what segment to use.
+    if (base != INTREG_RBP && base != INTREG_RSP) {
         seg = SEGMENT_REG_DS;
- //Handle any segment override that might have been in the instruction
-        int segFromInst = machInst.legacy.seg;
-        if (segFromInst)
-            seg = (SegmentRegIndex)(segFromInst - 1);
     } else {
         seg = SEGMENT_REG_SS;
     }
+    //Handle any segment override that might have been in the instruction
+    int segFromInst = machInst.legacy.seg;
+    if (segFromInst)
+        seg = (SegmentRegIndex)(segFromInst - 1);
 }

 void EmulEnv::setSeg(const ExtMachInst & machInst)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55589
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I73c6e367440a664c5c7c483337c16d4ab14f0e34
Gerrit-Change-Number: 55589
Gerrit-PatchSet: 13
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Bradford Beckmann <bradford.beckm...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Matt Sinclair <mattdsincl...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to