Patch vtable slots and similar when tiering is enabled (#21292)

Patch vtable slots and similar when tiering is enabled

For a method eligible for code versioning and vtable slot backpatch:
  - It does not have a precode (`HasPrecode()` returns false)
  - It does not have a stable entry point (`HasStableEntryPoint()` returns false)
  - A call to the method may be:
    - An indirect call through the `MethodTable`'s backpatchable vtable slot
    - A direct call to a backpatchable `FuncPtrStub`, perhaps through a `JumpStub`
    - For interface methods, an indirect call through the virtual stub dispatch (VSD) indirection cell to a backpatchable `DispatchStub` or a `ResolveStub` that refers to a backpatchable `ResolveCacheEntry`
  - The purpose is that typical calls to the method have no additional overhead when code versioning is enabled

Recording and backpatching slots:
  - In order for all vtable slots for the method to be backpatchable:
    - A vtable slot initially points to the `MethodDesc`'s temporary entry point, even when the method is inherited by a derived type (the slot's value is not copied from the parent)
    - The temporary entry point always points to the prestub and is never backpatched, in order to be able to discover new vtable slots through which the method may be called
    - The prestub, as part of `DoBackpatch()`, records any slots that are transitioned from the temporary entry point to the method's at-the-time current, non-prestub entry point
    - Any further changes to the method's entry point cause recorded slots to be backpatched in `BackpatchEntryPointSlots()`
  - In order for the `FuncPtrStub` to be backpatchable:
    - After the `FuncPtrStub` is created and exposed, it is patched to point to the method's at-the-time current entry point if necessary
    - Any further changes to the method's entry point cause the `FuncPtrStub` to be backpatched in `BackpatchEntryPointSlots()`
  - In order for VSD entities to be backpatchable:
    - A `DispatchStub`'s entry point target is aligned and recorded for backpatching in `BackpatchEntryPointSlots()`
      - The `DispatchStub` was modified on x86 and x64 such that the entry point target is aligned to a pointer to make it backpatchable
    - A `ResolveCacheEntry`'s entry point target is recorded for backpatching in `BackpatchEntryPointSlots()`

Slot lifetime and management of recorded slots:
  - A slot is recorded in the `LoaderAllocator` in which the slot is allocated, see `RecordAndBackpatchEntryPointSlot()`
  - An inherited slot that has a shorter lifetime than the `MethodDesc`, when recorded, needs to be accessible by the `MethodDesc` for backpatching, so the dependent `LoaderAllocator` with the slot to backpatch is also recorded in the `MethodDesc`'s `LoaderAllocator`, see `MethodDescBackpatchInfo::AddDependentLoaderAllocator_Locked()`
  - At the end of a `LoaderAllocator`'s lifetime, the `LoaderAllocator` is unregistered from dependency `LoaderAllocators`, see `MethodDescBackpatchInfoTracker::ClearDependencyMethodDescEntryPointSlots()`
  - When a `MethodDesc`'s entry point changes, backpatching also includes iterating over recorded dependent `LoaderAllocators` to backpatch the relevant slots recorded there, see `BackpatchEntryPointSlots()`

Synchronization between entry point changes and backpatching slots
  - A global lock is used to ensure that all recorded backpatchable slots corresponding to a `MethodDesc` point to the same entry point, see `DoBackpatch()` and `BackpatchEntryPointSlots()` for examples

Due to startup time perf issues:
  - `IsEligibleForTieredCompilation()` is called more frequently with this change and in hotter paths. I chose to use a `MethodDesc` flag to store that information for fast retreival. The flag is initialized by `DetermineAndSetIsEligibleForTieredCompilation()`.
  - Initially, I experimented with allowing a method versionable with vtable slot backpatch to have a precode, and allocated a new precode that would also be the stable entry point when a direct call is necessary. That also allows recording a new slot to be optional - in the event of an OOM, the slot may just point to the stable entry point. There are a large number of such methods and the allocations were slowing down startup perf. So, I had to eliminate precodes for methods versionable with vtable slot backpatch and that in turn means that recording slots is necessary for versionability.

1 files changed, 136 insertions, 55 deletions

diff --git a/src/vm/prestub.cpp b/src/vm/prestub.cpp
index 29f09ab109..5c08becafa 100644
--- a/src/vm/prestub.cpp
+++ b/src/vm/prestub.cpp
@@ -46,9 +46,8 @@
 #include "perfmap.h"
 #endif
 
-#ifdef FEATURE_TIERED_COMPILATION
 #include "callcounter.h"
-#endif
+#include "methoddescbackpatchinfo.h"
 
 #if defined(FEATURE_GDBJIT)
 #include "gdbjit.h"
@@ -83,58 +82,116 @@ PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BO
     {
         STANDARD_VM_CHECK;
         PRECONDITION(!ContainsGenericVariables());
-        PRECONDITION(HasStableEntryPoint());
         PRECONDITION(pMT == GetMethodTable());
     }
     CONTRACTL_END;
-    PCODE pTarget = GetStableEntryPoint();
 
-    if (!HasTemporaryEntryPoint())
-        return pTarget;
+    bool isVersionableWithVtableSlotBackpatch = IsVersionableWithVtableSlotBackpatch();
+    LoaderAllocator *mdLoaderAllocator = isVersionableWithVtableSlotBackpatch ? GetLoaderAllocator() : nullptr;
+
+    // Only take the lock if the method is versionable with vtable slot backpatch, for recording slots and synchronizing with
+    // backpatching slots
+    MethodDescBackpatchInfoTracker::ConditionalLockHolder lockHolder(isVersionableWithVtableSlotBackpatch);
+
+    // Get the method entry point inside the lock above to synchronize with backpatching in
+    // MethodDesc::BackpatchEntryPointSlots()
+    PCODE pTarget = GetMethodEntryPoint();
+
+    PCODE pExpected;
+    if (isVersionableWithVtableSlotBackpatch)
+    {
+        _ASSERTE(pTarget == GetEntryPointToBackpatch_Locked());
 
-    PCODE pExpected = GetTemporaryEntryPoint();
+        pExpected = GetTemporaryEntryPoint();
+        if (pExpected == pTarget)
+            return pTarget;
 
-    if (pExpected == pTarget)
-        return pTarget;
+        // True interface methods are never backpatched and are not versionable with vtable slot backpatch
+        _ASSERTE(!(pMT->IsInterface() && !IsStatic()));
 
-    // True interface methods are never backpatched
-    if (pMT->IsInterface() && !IsStatic())
-        return pTarget;
+        // Backpatching the funcptr stub:
+        //     For methods versionable with vtable slot backpatch, a funcptr stub is guaranteed to point to the at-the-time
+        //     current entry point shortly after creation, and backpatching it further is taken care of by
+        //     MethodDesc::BackpatchEntryPointSlots()
 
-    if (fFullBackPatch)
+        // Backpatching the temporary entry point:
+        //     The temporary entry point is never backpatched for methods versionable with vtable slot backpatch. New vtable
+        //     slots inheriting the method will initially point to the temporary entry point and it must point to the prestub
+        //     and come here for backpatching such that the new vtable slot can be discovered and recorded for future
+        //     backpatching.
+
+        _ASSERTE(!HasNonVtableSlot());
+    }
+    else
     {
-        FuncPtrStubs * pFuncPtrStubs = GetLoaderAllocator()->GetFuncPtrStubsNoCreate();
-        if (pFuncPtrStubs != NULL)
+        _ASSERTE(pTarget == GetStableEntryPoint());
+
+        if (!HasTemporaryEntryPoint())
+            return pTarget;
+
+        pExpected = GetTemporaryEntryPoint();
+        if (pExpected == pTarget)
+            return pTarget;
+
+        // True interface methods are never backpatched
+        if (pMT->IsInterface() && !IsStatic())
+            return pTarget;
+
+        if (fFullBackPatch)
         {
-            Precode* pFuncPtrPrecode = pFuncPtrStubs->Lookup(this);
-            if (pFuncPtrPrecode != NULL)
+            FuncPtrStubs * pFuncPtrStubs = GetLoaderAllocator()->GetFuncPtrStubsNoCreate();
+            if (pFuncPtrStubs != NULL)
             {
-                // If there is a funcptr precode to patch, we are done for this round.
-                if (pFuncPtrPrecode->SetTargetInterlocked(pTarget))
-                    return pTarget;
+                Precode* pFuncPtrPrecode = pFuncPtrStubs->Lookup(this);
+                if (pFuncPtrPrecode != NULL)
+                {
+                    // If there is a funcptr precode to patch, we are done for this round.
+                    if (pFuncPtrPrecode->SetTargetInterlocked(pTarget))
+                        return pTarget;
+                }
             }
-        }
 
 #ifndef HAS_COMPACT_ENTRYPOINTS
-        // Patch the fake entrypoint if necessary
-        Precode::GetPrecodeFromEntryPoint(pExpected)->SetTargetInterlocked(pTarget);
+            // Patch the fake entrypoint if necessary
+            Precode::GetPrecodeFromEntryPoint(pExpected)->SetTargetInterlocked(pTarget);
 #endif // HAS_COMPACT_ENTRYPOINTS
+        }
+
+        if (HasNonVtableSlot())
+            return pTarget;
     }
 
-    if (HasNonVtableSlot())
-        return pTarget;
+    auto RecordAndBackpatchSlot = [&](MethodTable *patchedMT, DWORD slotIndex)
+    {
+        WRAPPER_NO_CONTRACT;
+        _ASSERTE(isVersionableWithVtableSlotBackpatch);
+
+        RecordAndBackpatchEntryPointSlot_Locked(
+            mdLoaderAllocator,
+            patchedMT->GetLoaderAllocator(),
+            patchedMT->GetSlotPtr(slotIndex),
+            EntryPointSlots::SlotType_Vtable,
+            pTarget);
+    };
 
     BOOL fBackpatched = FALSE;
 
-#define BACKPATCH(pPatchedMT)                           \
-    do                                                  \
-    {                                                   \
-        if (pPatchedMT->GetSlot(dwSlot) == pExpected)   \
-        {                                               \
-            pPatchedMT->SetSlot(dwSlot, pTarget);       \
-            fBackpatched = TRUE;                        \
-        }                                               \
-    }                                                   \
+#define BACKPATCH(pPatchedMT)                                   \
+    do                                                          \
+    {                                                           \
+        if (pPatchedMT->GetSlot(dwSlot) == pExpected)           \
+        {                                                       \
+            if (isVersionableWithVtableSlotBackpatch)           \
+            {                                                   \
+                RecordAndBackpatchSlot(pPatchedMT, dwSlot);     \
+            }                                                   \
+            else                                                \
+            {                                                   \
+                pPatchedMT->SetSlot(dwSlot, pTarget);           \
+            }                                                   \
+            fBackpatched = TRUE;                                \
+        }                                                       \
+    }                                                           \
     while(0)
 
     // The owning slot has been updated already, so there is no need to backpatch it
@@ -154,8 +211,10 @@ PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BO
             // that it returns the stable entrypoint eventually to avoid going through the slow path all the time.
             //
             MethodTable * pRestoredSlotMT = pDispatchingMT->GetRestoredSlotMT(dwSlot);
-
-            BACKPATCH(pRestoredSlotMT);
+            if (pRestoredSlotMT != pDispatchingMT)
+            {
+                BACKPATCH(pRestoredSlotMT);
+            }
         }
     }
 
@@ -168,7 +227,7 @@ PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BO
 
             BACKPATCH(pMT);
 
-            if (pDispatchingMT != NULL)
+            if (pDispatchingMT != NULL && pDispatchingMT != pMT)
             {
                 BACKPATCH(pDispatchingMT);
             }
@@ -185,7 +244,7 @@ PCODE MethodDesc::DoBackpatch(MethodTable * pMT, MethodTable *pDispatchingMT, BO
         {
             BACKPATCH(pMT);
 
-            if (pDispatchingMT != NULL)
+            if (pDispatchingMT != NULL && pDispatchingMT != pMT)
             {
                 BACKPATCH(pDispatchingMT);
             }
@@ -1752,8 +1811,10 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
     /***************************  VERSIONABLE CODE    *********************/
 
     BOOL fIsPointingToPrestub = IsPointingToPrestub();
+    bool fIsVersionableWithoutJumpStamp = false;
 #ifdef FEATURE_CODE_VERSIONING
-    if (IsVersionableWithPrecode() ||
+    fIsVersionableWithoutJumpStamp = IsVersionableWithoutJumpStamp();
+    if (fIsVersionableWithoutJumpStamp ||
         (!fIsPointingToPrestub && IsVersionableWithJumpStamp()))
     {
         pCode = GetCodeVersionManager()->PublishVersionableCodeIfNecessary(this, fCanBackpatchPrestub);
@@ -1803,7 +1864,7 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
 #endif // defined(FEATURE_SHARE_GENERIC_CODE)
     else if (IsIL() || IsNoMetadata())
     {
-        if (!IsNativeCodeStableAfterInit())
+        if (!IsNativeCodeStableAfterInit() && (!fIsVersionableWithoutJumpStamp || IsVersionableWithPrecode()))
         {
             GetOrCreatePrecode();
         }
@@ -1874,13 +1935,14 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
 
     if (pCode != NULL)
     {
-        if (HasPrecode())
-            GetPrecode()->SetTargetInterlocked(pCode);
-        else
-            if (!HasStableEntryPoint())
-            {
-                SetStableEntryPointInterlocked(pCode);
-            }
+        if (fIsVersionableWithoutJumpStamp)
+        {
+            // Methods versionable without a jump stamp should not get here unless there was a failure. There may have been a
+            // failure to update the code versions above for some reason. Don't backpatch this time and try again next time.
+            return pCode;
+        }
+
+        SetCodeEntryPoint(pCode);
     }
     else
     {
@@ -1888,8 +1950,7 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT)
         {
             pStub->DecRef();
         }
-        else
-        if (pStub->HasExternalEntryPoint())
+        else if (pStub->HasExternalEntryPoint())
         {
             // If the Stub wraps code that is outside of the Stub allocation, then we
             // need to free the Stub allocation now.
@@ -2319,11 +2380,20 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl
             pCode = pMD->GetMethodEntryPoint();
 
             //
-            // Note that we do not want to call code:MethodDesc::IsPointingToPrestub() here. It does not take remoting interception 
-            // into account and so it would cause otherwise intercepted methods to be JITed. It is a compat issue if the JITing fails.
+            // Note that we do not want to call code:MethodDesc::IsPointingToPrestub() here. It does not take remoting
+            // interception into account and so it would cause otherwise intercepted methods to be JITed. It is a compat
+            // issue if the JITing fails.
             //
             if (!DoesSlotCallPrestub(pCode))
             {
+                if (pMD->IsVersionableWithVtableSlotBackpatch())
+                {
+                    // The entry point for this method needs to be versionable, so use a FuncPtrStub similarly to what is done
+                    // in MethodDesc::GetMultiCallableAddrOfCode()
+                    GCX_COOP();
+                    pCode = pMD->GetLoaderAllocator()->GetFuncPtrStubs()->GetFuncPtrStub(pMD);
+                }
+
                 pCode = PatchNonVirtualExternalMethod(pMD, pCode, pImportSection, pIndirection);
             }
         }
@@ -2380,10 +2450,21 @@ EXTERN_C PCODE VirtualMethodFixupWorker(Object * pThisPtr,  CORCOMPILE_VIRTUAL_I
 
     if (!DoesSlotCallPrestub(pCode))
     {
-        // Skip fixup precode jump for better perf
-        PCODE pDirectTarget = Precode::TryToSkipFixupPrecode(pCode);
-        if (pDirectTarget != NULL)
-            pCode = pDirectTarget;
+        MethodDesc *pMD = MethodTable::GetMethodDescForSlotAddress(pCode);
+        if (pMD->IsVersionableWithVtableSlotBackpatch())
+        {
+            // The entry point for this method needs to be versionable, so use a FuncPtrStub similarly to what is done in
+            // MethodDesc::GetMultiCallableAddrOfCode()
+            GCX_COOP();
+            pCode = pMD->GetLoaderAllocator()->GetFuncPtrStubs()->GetFuncPtrStub(pMD);
+        }
+        else
+        {
+            // Skip fixup precode jump for better perf
+            PCODE pDirectTarget = Precode::TryToSkipFixupPrecode(pCode);
+            if (pDirectTarget != NULL)
+                pCode = pDirectTarget;
+        }
 
         // Patch the thunk to the actual method body
         if (EnsureWritableExecutablePagesNoThrow(&pThunk->m_pTarget, sizeof(pThunk->m_pTarget)))