Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable FEATURE_MULTICASTSTUB_AS_IL for Windows x86 #104192

Merged
merged 13 commits into from
Jul 1, 2024
2 changes: 0 additions & 2 deletions src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,10 +1616,8 @@ internal static unsafe void LayoutDestroyNativeInternal(object obj, byte* pNativ
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern IntPtr GetStubContext();

#if FEATURE_MULTICASTSTUB_AS_IL
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern void MulticastDebuggerTraceHelper(object o, int count);
#endif

[Intrinsic]
[MethodImpl(MethodImplOptions.InternalCall)]
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/clr.featuredefines.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@

<PropertyGroup Condition="'$(TargetsUnix)' == 'true'">
<FeatureXplatEventSource Condition="'$(TargetOS)' == 'linux'">true</FeatureXplatEventSource>
<FeatureMulticastStubAsIL>true</FeatureMulticastStubAsIL>
<FeatureComWrappers>true</FeatureComWrappers>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetsWindows)' == 'true'">
<FeatureMulticastStubAsIL Condition="'$(Platform)' != 'x86'">true</FeatureMulticastStubAsIL>
<FeatureComWrappers>true</FeatureComWrappers>
<FeatureCominterop>true</FeatureCominterop>
<FeatureCominteropApartmentSupport>true</FeatureCominteropApartmentSupport>
Expand All @@ -30,7 +28,6 @@
</PropertyGroup>

<PropertyGroup>
<DefineConstants Condition="'$(FeatureMulticastStubAsIL)' == 'true'">$(DefineConstants);FEATURE_MULTICASTSTUB_AS_IL</DefineConstants>
<DefineConstants Condition="'$(FeatureComWrappers)' == 'true'">$(DefineConstants);FEATURE_COMWRAPPERS</DefineConstants>
<DefineConstants Condition="'$(FeatureCominterop)' == 'true'">$(DefineConstants);FEATURE_COMINTEROP</DefineConstants>
<DefineConstants Condition="'$(FeatureCominteropApartmentSupport)' == 'true'">$(DefineConstants);FEATURE_COMINTEROP_APARTMENT_SUPPORT</DefineConstants>
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/clrdefinitions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,6 @@ if(CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)
add_compile_definitions(OUT_OF_PROCESS_SETTHREADCONTEXT)
endif(CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)

# Features - please keep them alphabetically sorted
if(CLR_CMAKE_TARGET_WIN32)
if(NOT CLR_CMAKE_TARGET_ARCH_I386)
add_definitions(-DFEATURE_MULTICASTSTUB_AS_IL)
endif()
else(CLR_CMAKE_TARGET_WIN32)
add_definitions(-DFEATURE_MULTICASTSTUB_AS_IL)
endif(CLR_CMAKE_TARGET_WIN32)

if(NOT CLR_CMAKE_TARGET_ARCH_I386)
add_definitions(-DFEATURE_PORTABLE_SHUFFLE_THUNKS)
endif()
Expand Down
7 changes: 2 additions & 5 deletions src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6259,7 +6259,6 @@ void DebuggerStepper::TrapStepOut(ControllerStackInfo *info, bool fForceTraditio
_ASSERTE(IsCloserToLeaf(dbgLastFP, info->m_activeFrame.fp));
#endif

#ifdef FEATURE_MULTICASTSTUB_AS_IL
if (info->m_activeFrame.md != nullptr && info->m_activeFrame.md->IsILStub() && info->m_activeFrame.md->AsDynamicMethodDesc()->IsMulticastStub())
{
LOG((LF_CORDB, LL_INFO10000,
Expand All @@ -6286,10 +6285,8 @@ void DebuggerStepper::TrapStepOut(ControllerStackInfo *info, bool fForceTraditio
true))
break;
}
else
#endif // FEATURE_MULTICASTSTUB_AS_IL
if (info->m_activeFrame.md != nullptr && info->m_activeFrame.md->IsILStub() &&
info->m_activeFrame.md->AsDynamicMethodDesc()->GetILStubType() == DynamicMethodDesc::StubTailCallCallTarget)
else if (info->m_activeFrame.md != nullptr && info->m_activeFrame.md->IsILStub() &&
info->m_activeFrame.md->AsDynamicMethodDesc()->GetILStubType() == DynamicMethodDesc::StubTailCallCallTarget)
{
// Normally the stack trace would not include IL stubs, but we
// include this specific IL stub so that we can check if a call into
Expand Down
24 changes: 0 additions & 24 deletions src/coreclr/debug/ee/frameinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1562,9 +1562,7 @@ StackWalkAction DebuggerWalkStackProc(CrawlFrame *pCF, void *data)
{
_ASSERTE(md->IsDynamicMethod());
DynamicMethodDesc* dMD = md->AsDynamicMethodDesc();
#ifdef FEATURE_MULTICASTSTUB_AS_IL
use |= dMD->IsMulticastStub();
#endif
use |= dMD->GetILStubType() == DynamicMethodDesc::StubTailCallCallTarget;

if (use)
Expand Down Expand Up @@ -1759,28 +1757,6 @@ StackWalkAction DebuggerWalkStackProc(CrawlFrame *pCF, void *data)

break;

// Put frames we want to ignore here:
case Frame::TYPE_MULTICAST:
LOG((LF_CORDB, LL_INFO100000, "DWSP: Frame type is TYPE_MULTICAST.\n"));
if (d->ShouldIgnoreNonmethodFrames())
{
// Multicast frames exist only to gc protect the arguments
// between invocations of a delegate. They don't have code that
// we can (currently) show the user (we could change this with
// work, but why bother? It's an internal stub, and even if the
// user could see it, they can't modify it).
LOG((LF_CORDB, LL_INFO100000, "DWSP: Skipping frame 0x%x b/c it's "
"a multicast frame!\n", frame));
use = false;
}
else
{
LOG((LF_CORDB, LL_INFO100000, "DWSP: NOT Skipping frame 0x%x even thought it's "
"a multicast frame!\n", frame));
INTERNAL_FRAME_ACTION(d, use);
}
break;

default:
_ASSERTE(!"Invalid frame type!");
break;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/inc/vptr_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ VPTR_CLASS(HelperMethodFrame_PROTECTOBJ)
VPTR_CLASS(HijackFrame)
#endif
VPTR_CLASS(InlinedCallFrame)
VPTR_CLASS(MulticastFrame)
VPTR_CLASS(PInvokeCalliFrame)
VPTR_CLASS(PrestubMethodFrame)
VPTR_CLASS(ProtectByRefsFrame)
Expand Down
12 changes: 0 additions & 12 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1582,18 +1582,6 @@ StackWalkAction SystemDomain::CallersMethodCallbackWithStackMark(CrawlFrame* pCf
if (SystemDomain::IsReflectionInvocationMethod(pFunc))
return SWA_CONTINUE;

if (frame && frame->GetFrameType() == Frame::TYPE_MULTICAST)
{
// This must be either a multicast delegate invocation.

_ASSERTE(pFunc->GetMethodTable()->IsDelegate());

DELEGATEREF del = (DELEGATEREF)((MulticastFrame*)frame)->GetThis(); // This can throw.

_ASSERTE(COMDelegate::IsTrueMulticastDelegate(del));
return SWA_CONTINUE;
}

// Return the first non-reflection/remoting frame if no stack mark was
// supplied.
if (!pCaller->stackMark)
Expand Down
127 changes: 38 additions & 89 deletions src/coreclr/vm/comdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -728,9 +728,6 @@ VOID GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArray<S


ShuffleThunkCache *COMDelegate::m_pShuffleThunkCache = NULL;
#ifndef FEATURE_MULTICASTSTUB_AS_IL
MulticastStubCache *COMDelegate::m_pMulticastStubCache = NULL;
#endif

CrstStatic COMDelegate::s_DelegateToFPtrHashCrst;
PtrHashMap* COMDelegate::s_pDelegateToFPtrHash = NULL;
Expand All @@ -755,9 +752,6 @@ void COMDelegate::Init()
s_pDelegateToFPtrHash->Init(TRUE, &lock);

m_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap());
#ifndef FEATURE_MULTICASTSTUB_AS_IL
m_pMulticastStubCache = new MulticastStubCache();
#endif
}

#ifdef FEATURE_COMINTEROP
Expand Down Expand Up @@ -2139,7 +2133,6 @@ FCIMPL1(MethodDesc*, COMDelegate::GetInvokeMethod, Object* refThisIn)
}
FCIMPLEND

#ifdef FEATURE_MULTICASTSTUB_AS_IL
FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
{
FCALL_CONTRACT;
Expand All @@ -2166,38 +2159,41 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)

ILCodeStream *pCode = sl.NewCodeStream(ILStubLinker::kDispatch);

DWORD dwInvocationCountNum = pCode->NewLocal(ELEMENT_TYPE_I4);
DWORD dwLoopCounterNum = pCode->NewLocal(ELEMENT_TYPE_I4);

DWORD dwReturnValNum = -1;
if(fReturnVal)
dwReturnValNum = pCode->NewLocal(sig.GetRetTypeHandleNT());

ILCodeLabel *nextDelegate = pCode->NewCodeLabel();
ILCodeLabel *endOfMethod = pCode->NewCodeLabel();

// Get count of delegates
pCode->EmitLoadThis();
pCode->EmitLDFLD(pCode->GetToken(CoreLibBinder::GetField(FIELD__MULTICAST_DELEGATE__INVOCATION_COUNT)));
pCode->EmitSTLOC(dwInvocationCountNum);
ILCodeLabel *checkCount = pCode->NewCodeLabel();

// initialize counter
pCode->EmitLDC(0);
pCode->EmitSTLOC(dwLoopCounterNum);

// Make the shape of the loop similar to what C# compiler emits
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new shape seems to be missing the invocation of the trace helper at the end. E.g. if the invocation count is 2, the trace helper should be called 3 times. It is only called 2 times if I am reading the code correctly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Result codegen:

; Assembly listing for method System.Action:IL_STUB_MulticastDelegate_Invoke():this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; optimized using Synthesized PGO
; rsp based frame
; partially interruptible
; with Synthesized PGO: fgCalledCount is 100
; No PGO data

G_M000_IG01:                ;; offset=0x0000
       push     rsi
       push     rbx
       sub      rsp, 40
       mov      rbx, rcx

G_M000_IG02:                ;; offset=0x0009
       xor      esi, esi
       jmp      SHORT G_M000_IG04

G_M000_IG03:                ;; offset=0x000D
       mov      rcx, gword ptr [rbx+0x28]
       cmp      esi, dword ptr [rcx+0x08]
       jae      SHORT G_M000_IG08
       mov      rax, gword ptr [rcx+8*rsi+0x10]
       mov      rcx, gword ptr [rax+0x08]
       call     [rax+0x18]System.Action:Invoke():this
       inc      esi

G_M000_IG04:                ;; offset=0x0024
       test     dword ptr [(reloc 0x7ff84ea05210)], 512
       jne      SHORT G_M000_IG06

G_M000_IG05:                ;; offset=0x0030
       movsxd   rcx, esi
       cmp      rcx, qword ptr [rbx+0x30]
       jl       SHORT G_M000_IG03
       jmp      SHORT G_M000_IG07

G_M000_IG06:                ;; offset=0x003B
       mov      rcx, rbx
       mov      edx, esi
       call     System.StubHelpers.StubHelpers:MulticastDebuggerTraceHelper(System.Object,int)
       jmp      SHORT G_M000_IG05

G_M000_IG07:                ;; offset=0x0047
       add      rsp, 40
       pop      rbx
       pop      rsi
       ret

G_M000_IG08:                ;; offset=0x004E
       call     CORINFO_HELP_RNGCHKFAIL
       int3

; Total bytes of code 84

JIT reorders the debugging block in front of the ret. Benchmark shows no regression. It's just a bit strange with the unconditional jumps.

pCode->EmitBR(checkCount);

//Label_nextDelegate:
pCode->EmitLabel(nextDelegate);

#ifdef DEBUGGING_SUPPORTED
pCode->EmitLoadThis();
pCode->EmitLDLOC(dwLoopCounterNum);
pCode->EmitCALL(METHOD__STUBHELPERS__MULTICAST_DEBUGGER_TRACE_HELPER, 2, 0);
#endif // DEBUGGING_SUPPORTED
ILCodeLabel *invokeTraceHelper = pCode->NewCodeLabel();
ILCodeLabel *debuggerCheckEnd = pCode->NewCodeLabel();

// compare LoopCounter with InvocationCount. If equal then branch to Label_endOfMethod
pCode->EmitLDLOC(dwLoopCounterNum);
pCode->EmitLDLOC(dwInvocationCountNum);
pCode->EmitBEQ(endOfMethod);
// Call MulticastDebuggerTraceHelper only if any debugger is attached
pCode->EmitLDC((DWORD_PTR)&g_CORDebuggerControlFlags);
pCode->EmitCONV_I();
pCode->EmitLDIND_I4();

// (g_CORDebuggerControlFlags & DBCF_ATTACHED) != 0
pCode->EmitLDC(DBCF_ATTACHED);
pCode->EmitAND();
pCode->EmitBRTRUE(invokeTraceHelper);

pCode->EmitLabel(debuggerCheckEnd);
#endif // DEBUGGING_SUPPORTED

// Load next delegate from array using LoopCounter as index
pCode->EmitLoadThis();
Expand All @@ -2206,9 +2202,8 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
pCode->EmitLDELEM_REF();

// Load the arguments
UINT paramCount = 0;
while(paramCount < sig.NumFixedArgs())
pCode->EmitLDARG(paramCount++);
for (UINT paramCount = 0; paramCount < sig.NumFixedArgs(); paramCount++)
pCode->EmitLDARG(paramCount);

// call the delegate
pCode->EmitCALL(pCode->GetToken(pMD), sig.NumFixedArgs(), fReturnVal);
Expand All @@ -2223,11 +2218,14 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
pCode->EmitADD();
pCode->EmitSTLOC(dwLoopCounterNum);

// branch to next delegate
pCode->EmitBR(nextDelegate);
//Label_checkCount
pCode->EmitLabel(checkCount);

//Label_endOfMethod
pCode->EmitLabel(endOfMethod);
// compare LoopCounter with InvocationCount. If less then branch to nextDelegate
pCode->EmitLDLOC(dwLoopCounterNum);
pCode->EmitLoadThis();
pCode->EmitLDFLD(pCode->GetToken(CoreLibBinder::GetField(FIELD__MULTICAST_DELEGATE__INVOCATION_COUNT)));
pCode->EmitBLT(nextDelegate);

// load the return value. return value from the last delegate call is returned
if(fReturnVal)
Expand All @@ -2236,6 +2234,17 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
// return
pCode->EmitRET();

#ifdef DEBUGGING_SUPPORTED
// Emit debugging support at the end of the method for better perf
pCode->EmitLabel(invokeTraceHelper);

pCode->EmitLoadThis();
pCode->EmitLDLOC(dwLoopCounterNum);
pCode->EmitCALL(METHOD__STUBHELPERS__MULTICAST_DEBUGGER_TRACE_HELPER, 2, 0);

pCode->EmitBR(debuggerCheckEnd);
#endif // DEBUGGING_SUPPORTED

PCCOR_SIGNATURE pSig;
DWORD cbSig;

Expand All @@ -2260,66 +2269,6 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
}
FCIMPLEND

#else // FEATURE_MULTICASTSTUB_AS_IL

FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, Object* refThisIn)
{
FCALL_CONTRACT;

OBJECTREF refThis = ObjectToOBJECTREF(refThisIn);
MethodTable *pDelegateMT = refThis->GetMethodTable();

DelegateEEClass *delegateEEClass = ((DelegateEEClass*)(pDelegateMT->GetClass()));
Stub *pStub = delegateEEClass->m_pMultiCastInvokeStub;
if (pStub == NULL)
{
MethodDesc* pMD = delegateEEClass->GetInvokeMethod();

HELPER_METHOD_FRAME_BEGIN_RET_0();

GCX_PREEMP();

MetaSig sig(pMD);

UINT_PTR hash = CPUSTUBLINKER::HashMulticastInvoke(&sig);

pStub = m_pMulticastStubCache->GetStub(hash);
if (!pStub)
{
CPUSTUBLINKER sl;

LOG((LF_CORDB,LL_INFO10000, "COMD::GIMS making a multicast delegate\n"));

sl.EmitMulticastInvoke(hash);

// The cache is process-wide, based on signature. It never unloads
Stub *pCandidate = sl.Link(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_MULTICAST);

Stub *pWinner = m_pMulticastStubCache->AttemptToSetStub(hash,pCandidate);
ExecutableWriterHolder<Stub> candidateWriterHolder(pCandidate, sizeof(Stub));
candidateWriterHolder.GetRW()->DecRef();

if (!pWinner)
COMPlusThrowOM();

LOG((LF_CORDB,LL_INFO10000, "Putting a MC stub at 0x%p (code:0x%p)\n",
pWinner, (BYTE*)pWinner+sizeof(Stub)));

pStub = pWinner;
}

// we don't need to do an InterlockedCompareExchange here - the m_pMulticastStubCache->AttemptToSetStub
// will make sure all threads racing here will get the same stub, so they'll all store the same value
delegateEEClass->m_pMultiCastInvokeStub = pStub;

HELPER_METHOD_FRAME_END();
}

return pStub->GetEntryPoint();
}
FCIMPLEND
#endif // FEATURE_MULTICASTSTUB_AS_IL

PCODE COMDelegate::GetWrapperInvoke(MethodDesc* pMD)
{
CONTRACTL
Expand Down
11 changes: 0 additions & 11 deletions src/coreclr/vm/comdelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ class ShuffleThunkCache;
#include "dllimportcallback.h"
#include "stubcache.h"

#ifndef FEATURE_MULTICASTSTUB_AS_IL
typedef ArgBasedStubCache MulticastStubCache;
#endif

VOID GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, struct ShuffleEntry * pShuffleEntryArray, size_t nEntries);

enum class ShuffleComputationType
Expand All @@ -34,15 +30,8 @@ BOOL GenerateShuffleArrayPortable(MethodDesc* pMethodSrc, MethodDesc *pMethodDst
class COMDelegate
{
private:
// friend VOID CPUSTUBLINKER::EmitMulticastInvoke(...);
// friend VOID CPUSTUBLINKER::EmitShuffleThunk(...);
friend class CPUSTUBLINKER;
friend BOOL MulticastFrame::TraceFrame(Thread *thread, BOOL fromPatch,
TraceDestination *trace, REGDISPLAY *regs);

#ifndef FEATURE_MULTICASTSTUB_AS_IL
static MulticastStubCache* m_pMulticastStubCache;
#endif

static CrstStatic s_DelegateToFPtrHashCrst; // Lock for the following hash.
static PtrHashMap* s_pDelegateToFPtrHash; // Hash table containing the Delegate->FPtr pairs
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -938,9 +938,7 @@ DEFINE_METHOD(STUBHELPERS, PROFILER_BEGIN_TRANSITION_CALLBACK, Profiler
DEFINE_METHOD(STUBHELPERS, PROFILER_END_TRANSITION_CALLBACK, ProfilerEndTransitionCallback, SM_IntPtr_IntPtr_RetVoid)
#endif

#ifdef FEATURE_MULTICASTSTUB_AS_IL
DEFINE_METHOD(STUBHELPERS, MULTICAST_DEBUGGER_TRACE_HELPER, MulticastDebuggerTraceHelper, SM_Obj_Int_RetVoid)
#endif

DEFINE_CLASS(CLEANUP_WORK_LIST_ELEMENT, StubHelpers, CleanupWorkListElement)

Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/vm/dllimport.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ enum ILStubTypes
ILSTUB_ARRAYOP_GET = 0x80000001,
ILSTUB_ARRAYOP_SET = 0x80000002,
ILSTUB_ARRAYOP_ADDRESS = 0x80000003,
#ifdef FEATURE_MULTICASTSTUB_AS_IL
ILSTUB_MULTICASTDELEGATE_INVOKE = 0x80000004,
#endif
#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL
ILSTUB_UNBOXINGILSTUB = 0x80000005,
ILSTUB_INSTANTIATINGSTUB = 0x80000006,
Expand Down Expand Up @@ -231,9 +229,7 @@ inline bool SF_IsArrayOpStub (DWORD dwStubFlags) { LIMITED_METHOD_CONT
(dwStubFlags == ILSTUB_ARRAYOP_SET) ||
(dwStubFlags == ILSTUB_ARRAYOP_ADDRESS)); }

#ifdef FEATURE_MULTICASTSTUB_AS_IL
inline bool SF_IsMulticastDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_MULTICASTDELEGATE_INVOKE); }
#endif

inline bool SF_IsWrapperDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_WRAPPERDELEGATE_INVOKE); }
#ifdef FEATURE_INSTANTIATINGSTUB_AS_IL
Expand Down
Loading