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

Gc poll without HELPER_METHOD_FRAME #109378

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0fbff89
Part1 Of implement InitClass helpers
davidwrighton Sep 19, 2024
7d6a205
Merge branch 'main' of github.com:dotnet/runtime into initclass_managed
davidwrighton Sep 19, 2024
51c69d5
It builds and works
davidwrighton Sep 19, 2024
d1fccda
Revert unnecessary changes
davidwrighton Sep 19, 2024
08fe963
Ooops we did need to keep the refactoring that put the debug only Met…
davidwrighton Sep 19, 2024
12c9de2
Merge branch 'initclass_managed' into thread_static_helpers
davidwrighton Sep 19, 2024
2c0cb78
Initial work which does static helpers
davidwrighton Sep 24, 2024
0fb7f4a
Fix build break
davidwrighton Sep 24, 2024
0cb8f78
Thread statics basically implemented...
davidwrighton Sep 24, 2024
aacdb2a
Merge branch 'main' of https://github.com/dotnet/runtime into thread_…
davidwrighton Oct 17, 2024
00b6df4
Finish merging in changes from main, and fix up some oopses... and it…
davidwrighton Oct 18, 2024
34ebcaf
Restructure static access so that we don't need a helper call to acce…
davidwrighton Oct 19, 2024
f740618
Tweaks for failures noted in PR testing
davidwrighton Oct 21, 2024
e2498bb
Fix comments on Unix builds for Arm64 and RISCV
davidwrighton Oct 21, 2024
0d03a51
Tail call the right helper
davidwrighton Oct 21, 2024
367e95b
Force volatile loads for the statics ref, to fix the arm64 statics ra…
davidwrighton Oct 22, 2024
ccfafe1
Get rid of extra null check
davidwrighton Oct 22, 2024
5ff66b4
Fix error around fcall class ordering
davidwrighton Oct 22, 2024
0586455
Attempt to fix Unix assembly code, and fix fcall registration
davidwrighton Oct 24, 2024
0f6290a
Properly thread through using the "optimized" helper 2 for thread sta…
davidwrighton Oct 24, 2024
8f5448f
Remove unnecessary changes from the PR
davidwrighton Oct 25, 2024
2152544
Merge branch 'main' of https://github.com/dotnet/runtime into thread_…
davidwrighton Oct 28, 2024
2c9bc7d
Rewrite gc polling as a managed helper and remove the stress helper
davidwrighton Oct 28, 2024
1d4b19a
Remove polling logic
davidwrighton Oct 30, 2024
82fcb9f
Merge branch 'main' of https://github.com/dotnet/runtime into gc_poll…
davidwrighton Oct 30, 2024
2f7c7aa
More merging of the statics change
davidwrighton Oct 30, 2024
b56404e
Merge branch 'main' into gc_poll_without_hmf
davidwrighton Oct 30, 2024
edd2dbd
Apply suggestions from code review
davidwrighton Nov 13, 2024
c40d2f9
Add assembly paths for checking the g_TrapReturningThreads flag effic…
davidwrighton Nov 14, 2024
ec56ef6
Remove the helper method frames from P/Invoke suspension
davidwrighton Dec 6, 2024
04ab3fc
Merge branch 'main' of https://github.com/dotnet/runtime into gc_poll…
davidwrighton Dec 6, 2024
0001a6d
Merge branch 'main' of https://github.com/dotnet/runtime into gc_poll…
davidwrighton Dec 6, 2024
b926e5d
Attempt to fix assembly build errors
davidwrighton Dec 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -490,5 +490,37 @@ private void ResetFinalizerThreadSlow()
Priority = ThreadPriority.Highest;
}
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_PollGC")]
private static partial void ThreadNative_PollGC();

// GC Suspension is done by simply dropping into native code via p/invoke, and we reuse the p/invoke
// mechanism for suspension. On all architectures we should have the actual stub used for the check be implemented
// as a small assembly stub which checks the global g_TrapReturningThreads flag and tail-call to this helper
private static unsafe void PollGC()
{
NativeThreadState catchAtSafePoint = ((NativeThreadClass*)Thread.DirectOnThreadLocalData.pNativeThread)->m_State & NativeThreadState.TS_CatchAtSafePoint;
if (catchAtSafePoint != NativeThreadState.None)
{
ThreadNative_PollGC();
}
}

[StructLayout(LayoutKind.Sequential)]
private struct NativeThreadClass
{
public NativeThreadState m_State;
}

private enum NativeThreadState
{
None = 0,
TS_AbortRequested = 0x00000001, // Abort the thread
TS_DebugSuspendPending = 0x00000008, // Is the debugger suspending threads?
TS_GCOnTransitions = 0x00000010, // Force a GC on stub transitions (GCStress only)

// We require (and assert) that the following bits are less than 0x100.
TS_CatchAtSafePoint = (TS_AbortRequested | TS_DebugSuspendPending | TS_GCOnTransitions),
};
}
}
1 change: 0 additions & 1 deletion src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,6 @@ enum CorInfoHelpFunc
CORINFO_HELP_STOP_FOR_GC, // Call GC (force a GC)
CORINFO_HELP_POLL_GC, // Ask GC if it wants to collect

CORINFO_HELP_STRESS_GC, // Force a GC, but then update the JITTED code to be a noop call
CORINFO_HELP_CHECK_OBJ, // confirm that ECX is a valid object pointer (debugging only)

/* GC Write barrier support */
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,8 @@

// GC support
DYNAMICJITHELPER(CORINFO_HELP_STOP_FOR_GC, JIT_RareDisableHelper, METHOD__NIL)
JITHELPER(CORINFO_HELP_POLL_GC, JIT_PollGC, METHOD__NIL)
JITHELPER(CORINFO_HELP_STRESS_GC, JIT_StressGC, METHOD__NIL)

DYNAMICJITHELPER(CORINFO_HELP_POLL_GC, JIT_PollGC, METHOD__THREAD__POLLGC)

JITHELPER(CORINFO_HELP_CHECK_OBJ, JIT_CheckObj, METHOD__NIL)

// GC Write barrier support
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ which is the right helper to use to allocate an object of a given type. */
CORINFO_HELP_STOP_FOR_GC, // Call GC (force a GC)
CORINFO_HELP_POLL_GC, // Ask GC if it wants to collect

CORINFO_HELP_STRESS_GC, // Force a GC, but then update the JITTED code to be a noop call
CORINFO_HELP_CHECK_OBJ, // confirm that ECX is a valid object pointer (debugging only)

/* GC Write barrier support */
Expand Down
12 changes: 11 additions & 1 deletion src/coreclr/vm/amd64/AsmHelpers.asm
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ extern ProfileTailcall:proc
extern OnHijackWorker:proc
extern JIT_RareDisableHelperWorker:proc

extern g_pPollGC:QWORD
extern g_TrapReturningThreads:DWORD

; EXTERN_C int __fastcall HelperMethodFrameRestoreState(
; INDEBUG_COMMA(HelperMethodFrame *pFrame)
Expand Down Expand Up @@ -447,5 +449,13 @@ NESTED_END OnCallCountThresholdReachedStub, _TEXT

endif ; FEATURE_TIERED_COMPILATION

end
LEAF_ENTRY JIT_PollGC, _TEXT
cmp [g_TrapReturningThreads], 0
jnz RarePath
ret
RarePath:
Comment on lines +454 to +456
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
jnz RarePath
ret
RarePath:
jnz JIT_PollGCRarePath
ret
JIT_PollGCRarePath:

mov rax, g_pPollGC
TAILJMP_RAX
LEAF_END JIT_PollGC, _TEXT

end
11 changes: 11 additions & 0 deletions src/coreclr/vm/amd64/asmhelpers.S
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,14 @@ LEAF_ENTRY GetTlsIndexObjectDescOffset, _TEXT
int 3
LEAF_END GetTlsIndexObjectDescOffset, _TEXT
#endif

LEAF_ENTRY JIT_PollGC, _TEXT
PREPARE_EXTERNAL_VAR g_TrapReturningThreads, rax
cmp dword ptr [rax], 0
jnz LOCAL_LABEL(JIT_PollGCRarePath)
ret
LOCAL_LABEL(JIT_PollGCRarePath):
PREPARE_EXTERNAL_VAR g_pPollGC, rax
mov rax, [rax]
jmp rax
LEAF_END JIT_PollGC, _TEXT
1 change: 1 addition & 0 deletions src/coreclr/vm/amd64/cgencpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -606,5 +606,6 @@ inline BOOL ClrFlushInstructionCache(LPCVOID pCodeAddr, size_t sizeOfCode, bool
//
#define JIT_GetDynamicGCStaticBase JIT_GetDynamicGCStaticBase_SingleAppDomain
#define JIT_GetDynamicNonGCStaticBase JIT_GetDynamicNonGCStaticBase_SingleAppDomain
#define JIT_PollGC JIT_PollGC
Copy link
Member

Choose a reason for hiding this comment

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

Why are we defining the name itself?


#endif // __cgencpu_h__
3 changes: 3 additions & 0 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,8 @@ extern "C" PCODE g_pGetGCStaticBase;
PCODE g_pGetGCStaticBase;
extern "C" PCODE g_pGetNonGCStaticBase;
PCODE g_pGetNonGCStaticBase;
extern "C" PCODE g_pPollGC;
PCODE g_pPollGC;

void SystemDomain::LoadBaseSystemClasses()
{
Expand Down Expand Up @@ -1144,6 +1146,7 @@ void SystemDomain::LoadBaseSystemClasses()

g_pGetGCStaticBase = CoreLibBinder::GetMethod(METHOD__STATICSHELPERS__GET_GC_STATIC)->GetMultiCallableAddrOfCode();
g_pGetNonGCStaticBase = CoreLibBinder::GetMethod(METHOD__STATICSHELPERS__GET_NONGC_STATIC)->GetMultiCallableAddrOfCode();
g_pPollGC = CoreLibBinder::GetMethod(METHOD__THREAD__POLLGC)->GetMultiCallableAddrOfCode();

#ifdef PROFILING_SUPPORTED
// Note that g_profControlBlock.fBaseSystemClassesLoaded must be set to TRUE only after
Expand Down
24 changes: 24 additions & 0 deletions src/coreclr/vm/arm/asmhelpers.S
Original file line number Diff line number Diff line change
Expand Up @@ -914,3 +914,27 @@ ProbeLoop:
NESTED_END OnCallCountThresholdReachedStub, _TEXT

#endif // FEATURE_TIERED_COMPILATION

LEAF_ENTRY JIT_PollGC, _TEXT
#if defined(__clang__)
ldr r2, =g_TrapReturningThreads-(1f+4)
1:
add r2, pc
#else
ldr r2, =g_TrapReturningThreads
#endif
PREPARE_EXTERNAL_VAR g_TrapReturningThreads, r0
ldr r2, [r2]
cbnz r2, LOCAL_LABEL(JIT_PollGCRarePath)
ret
LOCAL_LABEL(JIT_PollGCRarePath):
#if defined(__clang__)
ldr r2, =g_pPollGC-(1f+4)
1:
add r2, pc
#else
ldr r2, =g_pPollGC
#endif
ldr r2, [r2]
EPILOG_BRANCH_REG r2
LEAF_END JIT_PollGC, _TEXT
2 changes: 2 additions & 0 deletions src/coreclr/vm/arm/cgencpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ EXTERN_C void setFPReturn(int fpSize, INT64 retVal);

#define FLOAT_REGISTER_SIZE 4 // each register in FloatArgumentRegisters is 4 bytes.

#define JIT_PollGC JIT_PollGC
Copy link
Member

Choose a reason for hiding this comment

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

Same question as above. Why same name?


//**********************************************************************
// Parameter size
//**********************************************************************
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/vm/arm64/asmhelpers.S
Original file line number Diff line number Diff line change
Expand Up @@ -806,3 +806,14 @@ LEAF_ENTRY GetTLSResolverAddress, _TEXT
LEAF_END GetTLSResolverAddress, _TEXT
// ------------------------------------------------------------------
#endif // !TARGET_OSX

LEAF_ENTRY JIT_PollGC, _TEXT
PREPARE_EXTERNAL_VAR g_TrapReturningThreads, x9
ldr w9, [x9]
cbnz w9, LOCAL_LABEL(JIT_PollGCRarePath)
ret
LOCAL_LABEL(JIT_PollGCRarePath):
PREPARE_EXTERNAL_VAR g_pPollGC, x9
ldr x9, [x9]
br x9
LEAF_END JIT_PollGC, _TEXT
14 changes: 14 additions & 0 deletions src/coreclr/vm/arm64/asmhelpers.asm
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
IMPORT g_pGetGCStaticBase
IMPORT g_pGetNonGCStaticBase

IMPORT g_pPollGC
IMPORT g_TrapReturningThreads

#ifdef WRITE_BARRIER_CHECK
SETALIAS g_GCShadow, ?g_GCShadow@@3PEAEEA
SETALIAS g_GCShadowEnd, ?g_GCShadowEnd@@3PEAEEA
Expand Down Expand Up @@ -1179,6 +1182,17 @@ __HelperNakedFuncName SETS "$helper":CC:"Naked"

#endif ; FEATURE_SPECIAL_USER_MODE_APC

LEAF_ENTRY JIT_PollGC
ldr x9, =g_TrapReturningThreads
ldr w9, [x9]
cbnz w9, RarePath
ret
RarePath
Comment on lines +1188 to +1190
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cbnz w9, RarePath
ret
RarePath
cbnz w9, JIT_PollGCRarePath
ret
JIT_PollGCRarePath

ldr x9, =g_pPollGC
ldr x9, [x9]
br x9
LEAF_END


; Must be at very end of file
END
1 change: 1 addition & 0 deletions src/coreclr/vm/arm64/cgencpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ inline unsigned StackElemSize(unsigned parmSize, bool isValueType, bool isFloatH
//
#define JIT_GetDynamicGCStaticBase JIT_GetDynamicGCStaticBase_SingleAppDomain
#define JIT_GetDynamicNonGCStaticBase JIT_GetDynamicNonGCStaticBase_SingleAppDomain
#define JIT_PollGC JIT_PollGC

//**********************************************************************
// Frames
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/comsynchronizable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,12 @@ extern "C" void QCALLTYPE ThreadNative_DisableComObjectEagerCleanup(QCall::Threa
}
#endif //FEATURE_COMINTEROP

extern "C" void QCALLTYPE ThreadNative_PollGC()
{
// This is an intentional no-op. The call is made to ensure that the thread goes through a GC transition
// and is thus marked as a GC safe point, and that the p/invoke rare path will kick in
}

extern "C" BOOL QCALLTYPE ThreadNative_YieldThread()
{
QCALL_CONTRACT;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/comsynchronizable.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ extern "C" BOOL QCALLTYPE ThreadNative_GetIsBackground(QCall::ThreadHandle threa
extern "C" void QCALLTYPE ThreadNative_SetIsBackground(QCall::ThreadHandle thread, BOOL value);
extern "C" void QCALLTYPE ThreadNative_InformThreadNameChange(QCall::ThreadHandle thread, LPCWSTR name, INT32 len);
extern "C" BOOL QCALLTYPE ThreadNative_YieldThread();
extern "C" void QCALLTYPE ThreadNative_PollGC();
extern "C" UINT64 QCALLTYPE ThreadNative_GetCurrentOSThreadId();
extern "C" void QCALLTYPE ThreadNative_Initialize(QCall::ObjectHandleOnStack t);
extern "C" INT32 QCALLTYPE ThreadNative_GetThreadState(QCall::ThreadHandle thread);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,8 @@ DEFINE_CLASS(DIRECTONTHREADLOCALDATA, Threading, Thread+DirectOnThreadLocalData)

DEFINE_CLASS(THREAD, Threading, Thread)
DEFINE_METHOD(THREAD, START_CALLBACK, StartCallback, IM_RetVoid)
DEFINE_METHOD(THREAD, POLLGC, PollGC, NoSig)

#ifdef FEATURE_OBJCMARSHAL
DEFINE_CLASS(AUTORELEASEPOOL, Threading, AutoreleasePool)
DEFINE_METHOD(AUTORELEASEPOOL, CREATEAUTORELEASEPOOL, CreateAutoreleasePool, SM_RetVoid)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ class Frame : public FrameBase
friend class TailCallFrame;
friend class AppDomain;
friend VOID RealCOMPlusThrow(OBJECTREF);
friend FCDECL0(VOID, JIT_StressGC);
#ifdef _DEBUG
friend LONG WINAPI CLRVectoredExceptionHandlerShim(PEXCEPTION_POINTERS pExceptionInfo);
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/i386/PInvokeStubs.asm
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ extern _s_gsCookie:DWORD
extern ??_7InlinedCallFrame@@6B@:DWORD
extern _g_TrapReturningThreads:DWORD

extern @JIT_PInvokeEndRarePath@0:proc
extern _JIT_PInvokeEndRarePath@0:proc

.686P
.XMM
Expand Down Expand Up @@ -103,7 +103,7 @@ _JIT_PInvokeEnd@4 PROC public
ret

RarePath:
jmp @JIT_PInvokeEndRarePath@0
jmp _JIT_PInvokeEndRarePath@0

_JIT_PInvokeEnd@4 ENDP

Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/i386/cgencpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -482,5 +482,8 @@ inline BOOL ClrFlushInstructionCache(LPCVOID pCodeAddr, size_t sizeOfCode, bool
// #define JIT_GetSharedNonGCStaticBase
// #define JIT_GetSharedGCStaticBaseNoCtor
// #define JIT_GetSharedNonGCStaticBaseNoCtor
#ifdef TARGET_WINDOWS
#define JIT_PollGC JIT_PollGC
#endif

#endif // __cgenx86_h__
16 changes: 16 additions & 0 deletions src/coreclr/vm/i386/jithelp.asm
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ JIT_TailCallVSDLeave TEXTEQU <_JIT_TailCallVSDLeave@0>
JIT_TailCallHelper TEXTEQU <_JIT_TailCallHelper@4>
JIT_TailCallReturnFromVSD TEXTEQU <_JIT_TailCallReturnFromVSD@0>

g_pPollGC TEXTEQU <_g_pPollGC>
g_TrapReturningThreads TEXTEQU <_g_TrapReturningThreads>

EXTERN g_ephemeral_low:DWORD
EXTERN g_ephemeral_high:DWORD
EXTERN g_lowest_address:DWORD
Expand All @@ -59,6 +62,10 @@ EXTERN _g_TailCallFrameVptr:DWORD
EXTERN @JIT_FailFast@0:PROC
EXTERN _s_gsCookie:DWORD

EXTERN g_pPollGC:DWORD
EXTERN g_TrapReturningThreads:DWORD


ifdef WRITE_BARRIER_CHECK
; Those global variables are always defined, but should be 0 for Server GC
g_GCShadow TEXTEQU <?g_GCShadow@@3PAEA>
Expand Down Expand Up @@ -1149,4 +1156,13 @@ _JIT_StackProbe_End@0 PROC
ret
_JIT_StackProbe_End@0 ENDP

@JIT_PollGC@0 PROC public
cmp [g_TrapReturningThreads], 0
jnz RarePath
ret
RarePath:
Comment on lines +1161 to +1163
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
jnz RarePath
ret
RarePath:
jnz JIT_PollGCRarePath
ret
JIT_PollGCRarePath:

mov eax, g_pPollGC
jmp eax
@JIT_PollGC@0 ENDP

end
Loading
Loading