From e8cb39239add3b843817a172788bc8b77800d885 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 8 Jan 2026 14:26:59 -0800 Subject: [PATCH 1/3] A quick analysis of the current use of PInvokeArgIterator leads me to believe that we would use it to calculate a value we never actually used. This PR changes the implementation of the runtime to remove those uses which were tied to contruction of function pointers from delegates where the delegate parameters were a structure which required marshalling. Should be a small code size win for Unix X64 platforms along with a negligible perf boost. This work is not removing the PInvokeArgIterator itself, as it appears to be necessary for the interpreter call stub generator (although it also needs to be fixed to be correct in those cases as well.) --- src/coreclr/vm/clrtocomcall.cpp | 2 ++ src/coreclr/vm/comdelegate.cpp | 2 ++ src/coreclr/vm/dllimport.cpp | 8 +++++++- src/coreclr/vm/method.cpp | 3 ++- src/coreclr/vm/method.hpp | 35 ++++++++++++--------------------- 5 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/coreclr/vm/clrtocomcall.cpp b/src/coreclr/vm/clrtocomcall.cpp index 0e6add81fc3d87..7520a200483239 100644 --- a/src/coreclr/vm/clrtocomcall.cpp +++ b/src/coreclr/vm/clrtocomcall.cpp @@ -69,7 +69,9 @@ CLRToCOMCallInfo *CLRToCOMCall::PopulateCLRToCOMCallMethodDesc(MethodDesc* pMD, LoaderHeap *pHeap = pMD->GetLoaderAllocator()->GetHighFrequencyHeap(); CLRToCOMCallInfo *pTemp = (CLRToCOMCallInfo *)(void *)pHeap->AllocMem(S_SIZE_T(sizeof(CLRToCOMCallInfo))); +#ifdef TARGET_X86 pTemp->InitStackArgumentSize(); +#endif // TARGET_X86 InterlockedCompareExchangeT(&pCMD->m_pCLRToCOMCallInfo, pTemp, NULL); } diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index 75dd154611d003..8160d2b0197985 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -812,7 +812,9 @@ CLRToCOMCallInfo * COMDelegate::PopulateCLRToCOMCallInfo(MethodTable * pDelMT) CLRToCOMCallInfo *pTemp = (CLRToCOMCallInfo *)(void *)pHeap->AllocMem(S_SIZE_T(sizeof(CLRToCOMCallInfo))); pTemp->m_cachedComSlot = ComMethodTable::GetNumExtraSlots(ifVtable); +#ifdef TARGET_X86 pTemp->InitStackArgumentSize(); +#endif // TARGET_X86 InterlockedCompareExchangeT(&pClass->m_pCLRToCOMCallInfo, pTemp, NULL); } diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index af8a66a0a992fc..057421fc0705f3 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -491,7 +491,7 @@ class ILStubState : public StubState pStubMD->SetStatic(); } -#ifndef TARGET_X86 +#if !defined(TARGET_X86) && defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE) // we store the real managed argument stack size in the stub MethodDesc on non-X86 UINT stackSize = pStubMD->SizeOfNativeArgStack(); @@ -3477,6 +3477,7 @@ BOOL PInvoke::MarshalingRequired( if (!FitsInU2(dwStackSize)) return TRUE; +#ifdef TARGET_X86 // do not set the stack size for varargs - the number is call site specific if (pMD != NULL && !pMD->IsVarArg()) { @@ -3492,6 +3493,7 @@ BOOL PInvoke::MarshalingRequired( } #endif // FEATURE_COMINTEROP } +#endif // TARGET_X86 return FALSE; } @@ -3886,11 +3888,13 @@ static void CreatePInvokeStubWorker(StubState* pss, if (!FitsInU2(nativeStackSize)) COMPlusThrow(kMarshalDirectiveException, IDS_EE_SIGTOOCOMPLEX); +#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); if (fStubNeedsCOM) pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); +#endif } // FinishEmit needs to know the native stack arg size so we call it after the number @@ -3984,11 +3988,13 @@ static void CreateStructStub(ILStubState* pss, pss->MarshalField(&mlInfo, managedOffset, externalOffset, pFD); } +#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE) if (pMD->IsDynamicMethod()) { DynamicMethodDesc* pDMD = pMD->AsDynamicMethodDesc(); pDMD->SetNativeStackArgSize(4 * TARGET_POINTER_SIZE); // The native stack arg size is constant since the signature for struct stubs is constant. } +#endif // FinishEmit needs to know the native stack arg size so we call it after the number // has been set in the stub MD (code:DynamicMethodDesc.SetNativeStackArgSize) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index dda289de1ed9f2..e5b5e5e249159e 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1728,7 +1728,7 @@ UINT MethodDesc::SizeOfArgStack() return argit.SizeOfArgStack(); } - +#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE UINT MethodDesc::SizeOfNativeArgStack() { #ifndef UNIX_AMD64_ABI @@ -1740,6 +1740,7 @@ UINT MethodDesc::SizeOfNativeArgStack() return argit.SizeOfArgStack(); #endif } +#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE #ifdef TARGET_X86 //******************************************************************************* diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index c63f9e344d87d0..8e31fa2baf5a1e 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -60,6 +60,10 @@ EXTERN_C VOID STDCALL PInvokeImportThunk(); #define METHOD_TOKEN_RANGE_BIT_COUNT (24 - METHOD_TOKEN_REMAINDER_BIT_COUNT) #define METHOD_TOKEN_RANGE_MASK ((1 << METHOD_TOKEN_RANGE_BIT_COUNT) - 1) +#if defined(TARGET_X86) || defined(FEATURE_COMINTEROP) +#define FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#endif + enum class AsyncMethodFlags { // Method uses CORINFO_CALLCONV_ASYNCCALL call convention. @@ -858,9 +862,11 @@ class MethodDesc // arguments passed in registers. UINT SizeOfArgStack(); +#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE // Returns the # of bytes of stack used by arguments in a call from native to this function. // Does not include arguments passed in registers. UINT SizeOfNativeArgStack(); +#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE // Returns the # of bytes to pop after a call. Not necessary the // same as SizeOfArgStack()! @@ -2887,23 +2893,25 @@ class DynamicMethodDesc : public StoredSigMethodDesc return asMetadata; } +#if defined(TARGET_X86) || defined(FEATURE_COMINTEROP) WORD GetNativeStackArgSize() { LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(IsILStub()); return (WORD)((m_dwExtendedFlags & StackArgSizeMask) >> 16); } - + void SetNativeStackArgSize(WORD cbArgSize) { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); -#if !defined(OSX_ARM64_ABI) + #if !defined(OSX_ARM64_ABI) _ASSERTE((cbArgSize % TARGET_POINTER_SIZE) == 0); -#endif + #endif m_dwExtendedFlags = (m_dwExtendedFlags & ~StackArgSizeMask) | ((DWORD)cbArgSize << 16); } - +#endif // TARGET_X86 || FEATURE_COMINTEROP + bool IsReversePInvokeStub() const { LIMITED_METHOD_DAC_CONTRACT; @@ -3333,11 +3341,11 @@ class PInvokeMethodDesc : public MethodDesc #endif public: +#if defined(TARGET_X86) void SetStackArgumentSize(WORD cbDstBuffer, CorInfoCallConvExtension unmgdCallConv) { LIMITED_METHOD_CONTRACT; -#if defined(TARGET_X86) // thiscall passes the this pointer in ECX if (unmgdCallConv == CorInfoCallConvExtension::Thiscall) { @@ -3354,10 +3362,8 @@ class PInvokeMethodDesc : public MethodDesc { _ASSERTE(m_cbStackArgumentSize == cbDstBuffer); } -#endif // defined(TARGET_X86) } -#if defined(TARGET_X86) void EnsureStackArgumentSize(); WORD GetStackArgumentSize() const @@ -3475,16 +3481,6 @@ struct CLRToCOMCallInfo _ASSERTE(m_cbStackArgumentSize != 0xFFFF); return m_cbStackArgumentSize; } -#else // TARGET_X86 - void InitStackArgumentSize() - { - LIMITED_METHOD_CONTRACT; - } - - void SetStackArgumentSize(WORD cbDstBuffer) - { - LIMITED_METHOD_CONTRACT; - } #endif // TARGET_X86 }; @@ -3554,11 +3550,6 @@ class CLRToCOMCallMethodDesc : public MethodDesc LIMITED_METHOD_CONTRACT; m_pCLRToCOMCallInfo->m_cbStackPop = (WORD)CbStackPop(); } -#else // TARGET_X86 - void SetStackArgumentSize(WORD cbDstBuffer) - { - LIMITED_METHOD_CONTRACT; - } #endif // TARGET_X86 }; #endif // FEATURE_COMINTEROP From 90c7019ab0c137431ef712558fde2c81ab818609 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 12 Jan 2026 10:26:43 -0800 Subject: [PATCH 2/3] Fix ifdef --- src/coreclr/vm/method.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 8e31fa2baf5a1e..0f1aa9f75189bd 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2905,9 +2905,9 @@ class DynamicMethodDesc : public StoredSigMethodDesc { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); - #if !defined(OSX_ARM64_ABI) +#if !defined(OSX_ARM64_ABI) _ASSERTE((cbArgSize % TARGET_POINTER_SIZE) == 0); - #endif +#endif m_dwExtendedFlags = (m_dwExtendedFlags & ~StackArgSizeMask) | ((DWORD)cbArgSize << 16); } #endif // TARGET_X86 || FEATURE_COMINTEROP From 150c26bfcae19456e94ce804a11c38b9e873cc89 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 12 Jan 2026 10:29:58 -0800 Subject: [PATCH 3/3] Remove excess spaces --- src/coreclr/vm/method.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 0f1aa9f75189bd..016e25bdad2ab2 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2900,7 +2900,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc _ASSERTE(IsILStub()); return (WORD)((m_dwExtendedFlags & StackArgSizeMask) >> 16); } - + void SetNativeStackArgSize(WORD cbArgSize) { LIMITED_METHOD_CONTRACT;