Changeset 106006 in vbox for trunk/src/VBox

Timestamp:

Sep 10, 2024 8:35:20 PM (3 months ago)

Author:

vboxsync

Message:

VMM/IEM: Extended iemNativeEmitFetchEFlags (IEM_MC_FETCH_EFLAGS_EX) to get EFLAGS from host register if we're currently shadowing it. The new code seems to get the job done, but requires more tweaking in the liveness department soon. This addresses todo 7 in bugref:10720.

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• IEMAllN8veRecompFuncs.h (modified) (8 diffs)
• IEMAllN8veRecompiler.cpp (modified) (3 diffs)

trunk/src/VBox/VMM/VMMAll/IEMAllN8veRecompFuncs.h

@@ -301 +301 @@
     off = iemNativeVarSetAmd64HostRegisterForLocal(pReNative, off, a_VarNm, a_idxHostReg)
 
-DECL_INLINE_THROW(uint8_t) iemNativeVarRegisterSet(PIEMRECOMPILERSTATE pReNative, uint8_t idxVar, uint8_t idxReg, uint32_t off);
+DECL_INLINE_THROW(uint8_t) iemNativeVarRegisterSet(PIEMRECOMPILERSTATE pReNative, uint8_t idxVar, uint8_t idxReg,
+                                                   uint32_t off, bool fAllocated);
 
 DECL_INLINE_THROW(uint32_t)
@@ -317 +318 @@
 # endif
 
-    iemNativeVarRegisterSet(pReNative, idxVar, idxHstReg, off);
+    iemNativeVarRegisterSet(pReNative, idxVar, idxHstReg, off, false /*fAllocated*/);
     return off;
 }
@@ -3889 +3890 @@
  * Sets the host register for @a idxVarRc to @a idxReg.
  *
- * The register must not be allocated. Any guest register shadowing will be
- * implictly dropped by this call.
+ * Any guest register shadowing will be implictly dropped by this call.
  *
  * The variable must not have any register associated with it (causes
@@ -3901 +3901 @@
  * @param   idxReg      The host register (typically IEMNATIVE_CALL_RET_GREG).
  * @param   off         For recording in debug info.
+ * @param   fAllocated  Set if the register is already allocated, false if not.
  *
  * @throws  VERR_IEM_VAR_IPE_10, VERR_IEM_VAR_IPE_11
  */
-DECL_INLINE_THROW(uint8_t) iemNativeVarRegisterSet(PIEMRECOMPILERSTATE pReNative, uint8_t idxVar, uint8_t idxReg, uint32_t off)
+DECL_INLINE_THROW(uint8_t)
+iemNativeVarRegisterSet(PIEMRECOMPILERSTATE pReNative, uint8_t idxVar, uint8_t idxReg, uint32_t off, bool fAllocated)
 {
     IEMNATIVE_ASSERT_VAR_IDX(pReNative, idxVar);
@@ -3911 +3913 @@
     Assert(idxReg < RT_ELEMENTS(pReNative->Core.aHstRegs));
     AssertStmt(pVar->idxReg == UINT8_MAX, IEMNATIVE_DO_LONGJMP(pReNative, VERR_IEM_VAR_IPE_10));
-    AssertStmt(!(pReNative->Core.bmHstRegs & RT_BIT_32(idxReg)), IEMNATIVE_DO_LONGJMP(pReNative, VERR_IEM_VAR_IPE_11));
+    AssertStmt(RT_BOOL(pReNative->Core.bmHstRegs & RT_BIT_32(idxReg)) == fAllocated,
+               IEMNATIVE_DO_LONGJMP(pReNative, VERR_IEM_VAR_IPE_11));
 
     iemNativeRegClearGstRegShadowing(pReNative, idxReg, off);
@@ -3929 +3932 @@
                                                              uint8_t idxReg, uint32_t *poff)
 {
-    idxReg = iemNativeVarRegisterSet(pReNative, idxVar, idxReg, *poff);
+    idxReg = iemNativeVarRegisterSet(pReNative, idxVar, idxReg, *poff, false /*fAllocated*/);
     pReNative->Core.aVars[IEMNATIVE_VAR_IDX_UNPACK(idxVar)].fRegAcquired = true;
     return idxReg;
@@ -4253 +4256 @@
     off = iemNativeEmitCallImm(pReNative, off, pfnAImpl);
     if (idxVarRc != UINT8_MAX)
-        iemNativeVarRegisterSet(pReNative, idxVarRc, IEMNATIVE_CALL_RET_GREG, off);
+        iemNativeVarRegisterSet(pReNative, idxVarRc, IEMNATIVE_CALL_RET_GREG, off, false /*fAllocated*/);
 
     return off;
@@ -5861 +5864 @@
     IEMNATIVE_STRICT_EFLAGS_SKIPPING_EMIT_CHECK(pReNative, off, fEflInput);
 
-    /** @todo this is suboptimial. EFLAGS is probably shadowed and we should use
-     *        the existing shadow copy. */
-    uint8_t const idxReg = iemNativeVarRegisterAcquire(pReNative, idxVarEFlags, &off, false /*fInitialized*/);
-    iemNativeRegClearAndMarkAsGstRegShadow(pReNative, idxReg, kIemNativeGstReg_EFlags, off);
-    off = iemNativeEmitLoadGprFromVCpuU32(pReNative, off, idxReg, RT_UOFFSETOF(VMCPUCC, cpum.GstCtx.eflags));
-    iemNativeVarRegisterRelease(pReNative, idxVarEFlags);
+    /** @todo This could be prettier...*/
+    /** @todo Also, the shadowing+liveness handling of EFlags is currently
+     *        problematic, but I'll try tackle that soon (@bugref{10720}). */
+    PCIEMNATIVEVAR const pVar = &pReNative->Core.aVars[IEMNATIVE_VAR_IDX_UNPACK(idxVarEFlags)];
+    Assert(pVar->enmKind == kIemNativeVarKind_Invalid || pVar->enmKind == kIemNativeVarKind_Stack);
+    Assert(pVar->idxReg  == UINT8_MAX);
+    if (pVar->uArgNo >= IEMNATIVE_CALL_ARG_GREG_COUNT)
+    {
+        /** @todo We could use kIemNativeGstRegUse_ReadOnly here when fOutput is
+         *        zero, but since iemNativeVarRegisterSet clears the shadowing,
+         *        that's counter productive... */
+        uint8_t const idxGstReg = iemNativeRegAllocTmpForGuestReg(pReNative, &off, kIemNativeGstReg_EFlags,
+                                                                  kIemNativeGstRegUse_ForUpdate, false /*fNoVolatileRegs*/,
+                                                                  true /** @todo EFlags shadowing+liveness weirdness (@bugref{10720}). */);
+        iemNativeVarRegisterSet(pReNative, idxVarEFlags, idxGstReg, off, true /*fAllocated*/);
+    }
+    else
+    {
+        /* Register argument variable: Avoid assertions in generic call code and load it the traditional way. */
+        uint8_t const idxVarReg = iemNativeVarRegisterAcquire(pReNative, idxVarEFlags, &off, false /*fInitialized*/);
+        uint8_t const idxGstReg = iemNativeRegAllocTmpForGuestRegIfAlreadyPresent(pReNative, &off, kIemNativeGstReg_EFlags);
+        if (idxGstReg != UINT8_MAX)
+        {
+            off = iemNativeEmitLoadGprFromGpr32(pReNative, off, idxVarReg, idxGstReg);
+            iemNativeRegFreeTmp(pReNative, idxGstReg);
+        }
+        else
+            off = iemNativeEmitLoadGprFromVCpuU32(pReNative, off, idxVarReg, RT_UOFFSETOF(VMCPUCC, cpum.GstCtx.eflags));
+        iemNativeVarRegisterRelease(pReNative, idxVarEFlags);
+    }
     return off;
 }

trunk/src/VBox/VMM/VMMAll/IEMAllN8veRecompiler.cpp

@@ -3819 +3819 @@
     AssertMsg(   pReNative->idxCurCall == 0
               || IEMLIVENESS_STATE_IS_INPUT_EXPECTED(iemNativeLivenessGetPrevStateByGstReg(pReNative, enmGstReg))
-              || enmGstReg == kIemNativeGstReg_Pc,
+              || enmGstReg == kIemNativeGstReg_Pc
+              || enmGstReg == kIemNativeGstReg_EFlags /** @todo EFlags shadowing+liveness is weird and needs fixing (@bugref{10720}) */,
               ("%s - %u\n", g_aGstShadowInfo[enmGstReg].pszName, iemNativeLivenessGetPrevStateByGstReg(pReNative, enmGstReg)));
 #endif
@@ -7343 +7344 @@
 
 
-DECL_HIDDEN_THROW(uint8_t)  iemNativeVarAllocAssign(PIEMRECOMPILERSTATE pReNative, uint32_t *poff, uint8_t cbType, uint8_t idxVarOther)
+DECL_HIDDEN_THROW(uint8_t) iemNativeVarAllocAssign(PIEMRECOMPILERSTATE pReNative, uint32_t *poff,
+                                                   uint8_t cbType, uint8_t idxVarOther)
 {
     uint8_t const idxVar = IEMNATIVE_VAR_IDX_PACK(iemNativeVarAllocInt(pReNative, cbType));
@@ -7351 +7353 @@
     uint8_t const idxVarReg = iemNativeVarRegisterAcquire(pReNative, idxVar, poff);
 
+/** @todo combine MOV and AND using MOVZX/similar. */
     *poff = iemNativeEmitLoadGprFromGpr(pReNative, *poff, idxVarReg, idxVarOtherReg);