Add ThisptrRetbufPrecode to set of precodes handled by StubPrecode path (#112548)

This adds the ThisPtrRetBufPrecode to the set of codes handled by the StubPrecode stub pasting path.

Perf changes from about 2.35 ns per iteration of a minimal loop with a ThisPtrRetBufPrecode to about 3.03 ns per iteration on my computer (Windows X64). I expect the cost to be a little less on an Arm64 machine since the old ThisPtrRetBufPrecode was a bit more expensive there. However, this removes the previous scheme where we allocate and jit custom stubs for this, so I think the simplification is worth the runtime cost.

The overall change has a size increase though, as this work adds support for this type of precode to the cDAC, as well as some tests.

Author: davidwrighton

docs/design/datacontracts/PrecodeStubs.md

@@ -9,7 +9,7 @@ This contract provides support for examining [precode](../coreclr/botr/method-de
     TargetPointer GetMethodDescFromStubAddress(TargetCodePointer entryPoint);
 ```
 
-## Version 1
+## Version 1 and 2
 
 Data descriptors used:
 | Data Descriptor Name | Field | Meaning |
@@ -24,9 +24,12 @@ Data descriptors used:
 | PrecodeMachineDescriptor | PInvokeImportPrecodeType| precode sort byte for PInvoke precode stubs, if supported |
 | PrecodeMachineDescriptor | HasFixupPrecode | 1 if platform supports fixup precode stubs |
 | PrecodeMachineDescriptor | FixupPrecodeType| precode sort byte for fixup precode stubs, if supported |
-| StubPrecodeData | MethodDesc | pointer to the MethodDesc associated with this stub precode |
+| PrecodeMachineDescriptor | ThisPointerRetBufPrecodeType | precode sort byte for this pointer ret buf precodes |
+| StubPrecodeData | MethodDesc | pointer to the MethodDesc associated with this stub precode (Version 1 only) |
+| StubPrecodeData | SecretParam | pointer to the MethodDesc associated with this stub precode or a second stub data pointer for other types (Version 2 only) |
 | StubPrecodeData | Type | precise sort of stub precode |
 | FixupPrecodeData | MethodDesc | pointer to the MethodDesc associated with this fixup precode |
+| ThisPtrRetBufPrecodeData | MethodDesc | pointer to the MethodDesc associated with the ThisPtrRetBufPrecode (Version 2 only) |
 
 arm32 note: the `CodePointerToInstrPointerMask` is used to convert IP values that may include an arm Thumb bit (for example extracted from disassembling a call instruction or from a snapshot of the registers) into an address.  On other architectures applying the mask is a no-op.
 
@@ -145,7 +148,10 @@ After the initial precode type is determined, for stub precodes a refined precod
         internal override TargetPointer GetMethodDesc(Target target, Data.PrecodeMachineDescriptor precodeMachineDescriptor)
         {
             TargetPointer stubPrecodeDataAddress = InstrPointer + precodeMachineDescriptor.StubCodePageSize;
-            return target.ReadPointer (stubPrecodeDataAddress + /* offset of StubPrecodeData.MethodDesc */ );
+            if (ContractVersion(PrecodeStubs) == 1)
+                return target.ReadPointer (stubPrecodeDataAddress + /* offset of StubPrecodeData.MethodDesc */ );
+            else
+                return target.ReadPointer (stubPrecodeDataAddress + /* offset of StubPrecodeData.SecretParam */ );
         }
     }
 
@@ -170,7 +176,10 @@ After the initial precode type is determined, for stub precodes a refined precod
 
         internal override TargetPointer GetMethodDesc(Target target, Data.PrecodeMachineDescriptor precodeMachineDescriptor)
         {
-            throw new NotImplementedException(); // TODO(cdac)
+            if (ContractVersion(PrecodeStubs) == 1)
+                throw new NotImplementedException(); // TODO(cdac)
+            else
+                return target.ReadPointer(target.ReadPointer (stubPrecodeDataAddress + /* offset of StubPrecodeData.SecretParam */ ) + /*offset of ThisPtrRetBufPrecodeData.MethodDesc*/);
         }
     }
 

src/coreclr/debug/runtimeinfo/contracts.jsonc

@@ -17,7 +17,7 @@
   "Loader": 1,
   "Object": 1,
   "PlatformMetadata": 1,
-  "PrecodeStubs": 1,
+  "PrecodeStubs": 2,
   "ReJIT": 1,
   "RuntimeTypeSystem": 1,
   "StackWalk": 1,

src/coreclr/debug/runtimeinfo/datadescriptor.h

@@ -492,10 +492,17 @@ CDAC_TYPE_END(PlatformMetadata)
 
 CDAC_TYPE_BEGIN(StubPrecodeData)
 CDAC_TYPE_INDETERMINATE(StubPrecodeData)
-CDAC_TYPE_FIELD(StubPrecodeData, /*pointer*/, MethodDesc, offsetof(StubPrecodeData, SecretParam))
+CDAC_TYPE_FIELD(StubPrecodeData, /*pointer*/, SecretParam, offsetof(StubPrecodeData, SecretParam))
 CDAC_TYPE_FIELD(StubPrecodeData, /*uint8*/, Type, offsetof(StubPrecodeData, Type))
 CDAC_TYPE_END(StubPrecodeData)
 
+#ifdef HAS_THISPTR_RETBUF_PRECODE
+CDAC_TYPE_BEGIN(ThisPtrRetBufPrecodeData)
+CDAC_TYPE_INDETERMINATE(ThisPtrRetBufPrecodeData)
+CDAC_TYPE_FIELD(ThisPtrRetBufPrecodeData, /*pointer*/, MethodDesc, offsetof(ThisPtrRetBufPrecodeData, MethodDesc))
+CDAC_TYPE_END(ThisPtrRetBufPrecodeData)
+#endif
+
 CDAC_TYPE_BEGIN(FixupPrecodeData)
 CDAC_TYPE_INDETERMINATE(FixupPrecodeData)
 CDAC_TYPE_FIELD(FixupPrecodeData, /*pointer*/, MethodDesc, offsetof(FixupPrecodeData, MethodDesc))

src/coreclr/vm/amd64/AsmHelpers.asm

@@ -497,4 +497,14 @@ NESTED_ENTRY InterpreterStub, _TEXT
 NESTED_END InterpreterStub, _TEXT
 endif ; FEATURE_INTERPRETER
 
+; rcx -This pointer
+; rdx -ReturnBuffer
+LEAF_ENTRY ThisPtrRetBufPrecodeWorker, _TEXT
+    mov  METHODDESC_REGISTER, [METHODDESC_REGISTER + ThisPtrRetBufPrecodeData__Target]
+    mov r11, rcx
+    mov rcx, rdx
+    mov rdx, r11
+    jmp METHODDESC_REGISTER
+LEAF_END ThisPtrRetBufPrecodeWorker, _TEXT
+
         end

src/coreclr/vm/amd64/asmconstants.h

@@ -34,6 +34,9 @@
 #define DynamicHelperFrameFlags_ObjectArg   1
 #define DynamicHelperFrameFlags_ObjectArg2  2
 
+#define ThisPtrRetBufPrecodeData__Target      0x00
+ASMCONSTANTS_C_ASSERT(ThisPtrRetBufPrecodeData__Target == offsetof(ThisPtrRetBufPrecodeData, Target));
+
 #define ASMCONSTANT_OFFSETOF_ASSERT(struct, member) \
 ASMCONSTANTS_C_ASSERT(OFFSETOF__##struct##__##member == offsetof(struct, member));
 

src/coreclr/vm/amd64/asmhelpers.S

@@ -352,3 +352,13 @@ NESTED_ENTRY InterpreterStub, _TEXT, NoHandler
 
 NESTED_END InterpreterStub, _TEXT
 #endif // FEATURE_INTERPRETER
+
+//rdi -This pointer
+//rsi -ReturnBuffer
+LEAF_ENTRY ThisPtrRetBufPrecodeWorker, _TEXT
+    mov  METHODDESC_REGISTER, [METHODDESC_REGISTER + ThisPtrRetBufPrecodeData__Target]
+    mov r11, rsi
+    mov rsi, rdi
+    mov rdi, r11
+    jmp METHODDESC_REGISTER
+LEAF_END ThisPtrRetBufPrecodeWorker, _TEXT

src/coreclr/vm/arm/asmconstants.h

@@ -36,6 +36,9 @@ ASMCONSTANTS_C_ASSERT(FRAMETYPE_InlinedCallFrame == (int)FrameIdentifier::Inline
 #define DynamicHelperFrameFlags_ObjectArg   1
 #define DynamicHelperFrameFlags_ObjectArg2  2
 
+#define ThisPtrRetBufPrecodeData__Target      0x00
+ASMCONSTANTS_C_ASSERT(ThisPtrRetBufPrecodeData__Target == offsetof(ThisPtrRetBufPrecodeData, Target));
+
 #define REDIRECTSTUB_SP_OFFSET_CONTEXT 0
 
 

src/coreclr/vm/arm/asmhelpers.S

@@ -977,3 +977,15 @@ LOCAL_LABEL(JIT_PollGCRarePath):
         ldr     r2, [r2]
         EPILOG_BRANCH_REG r2
     LEAF_END JIT_PollGC, _TEXT
+
+
+//x0 -This pointer
+//x1 -ReturnBuffer
+LEAF_ENTRY ThisPtrRetBufPrecodeWorker, _TEXT
+    ldr  r12, [r12, #ThisPtrRetBufPrecodeData__Target]
+    // Use XOR swap technique to set avoid the need to spill to the stack
+    eor  r0, r0, r1
+    eor  r1, r0, r1
+    eor  r0, r0, r1
+    EPILOG_BRANCH_REG r12
+LEAF_END ThisPtrRetBufPrecodeWorker, _TEXT

src/coreclr/vm/arm/cgencpu.h

@@ -962,63 +962,6 @@ inline BOOL ClrFlushInstructionCache(LPCVOID pCodeAddr, size_t sizeOfCode, bool
 // Create alias for optimized implementations of helpers provided on this platform
 //
 
-//------------------------------------------------------------------------
-//
-// Precode definitions
-//
-//------------------------------------------------------------------------
-//
-// Note: If you introduce new precode implementation below, then please
-//       update PrecodeStubManager::CheckIsStub_Internal to account for it.
-
-// Precode to shuffle this and retbuf for closed delegates over static methods with return buffer
-struct ThisPtrRetBufPrecode {
-
-    static const int Type = 0x01;
-
-    // mov r12, r0
-    // mov r0, r1
-    // mov r1, r12
-    // ldr pc, [pc, #0]     ; =m_pTarget
-    // dcd pTarget
-    // dcd pMethodDesc
-    WORD    m_rgCode[6];
-    TADDR   m_pTarget;
-    TADDR   m_pMethodDesc;
-
-    void Init(MethodDesc* pMD, LoaderAllocator *pLoaderAllocator);
-
-    TADDR GetMethodDesc()
-    {
-        LIMITED_METHOD_DAC_CONTRACT;
-
-        return m_pMethodDesc;
-    }
-
-    PCODE GetTarget()
-    {
-        LIMITED_METHOD_DAC_CONTRACT;
-        return m_pTarget;
-    }
-
-#ifndef DACCESS_COMPILE
-    BOOL SetTargetInterlocked(TADDR target, TADDR expected)
-    {
-        CONTRACTL
-        {
-            THROWS;
-            GC_TRIGGERS;
-        }
-        CONTRACTL_END;
-
-        ExecutableWriterHolder<ThisPtrRetBufPrecode> precodeWriterHolder(this, sizeof(ThisPtrRetBufPrecode));
-        return InterlockedCompareExchange((LONG*)&precodeWriterHolder.GetRW()->m_pTarget, (LONG)target, (LONG)expected) == (LONG)expected;
-    }
-#endif // !DACCESS_COMPILE
-};
-typedef DPTR(ThisPtrRetBufPrecode) PTR_ThisPtrRetBufPrecode;
-
-
 //**********************************************************************
 // Miscellaneous
 //**********************************************************************

src/coreclr/vm/arm/stubs.cpp

@@ -733,26 +733,6 @@ void HelperMethodFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool update
 
 #ifndef DACCESS_COMPILE
 
-void ThisPtrRetBufPrecode::Init(MethodDesc* pMD, LoaderAllocator *pLoaderAllocator)
-{
-    WRAPPER_NO_CONTRACT;
-
-    int n = 0;
-
-    m_rgCode[n++] = 0x4684; // mov r12, r0
-    m_rgCode[n++] = 0x4608; // mov r0, r1
-    m_rgCode[n++] = 0xea4f; // mov r1, r12
-    m_rgCode[n++] = 0x010c;
-    m_rgCode[n++] = 0xf8df; // ldr pc, [pc, #0]
-    m_rgCode[n++] = 0xf000;
-
-    _ASSERTE(n == ARRAY_SIZE(m_rgCode));
-
-    m_pTarget = GetPreStubEntryPoint();
-    m_pMethodDesc = (TADDR)pMD;
-}
-
-
 /*
 Rough pseudo-code of interface dispatching: