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

[Test Only] async safe point interrupts before NOP change #101152

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
5cd729c
allow async interruptions on safepoints
VSadov Dec 4, 2023
2434466
ARM64 TODO
VSadov Dec 4, 2023
cbaafbe
report GC ref/byref returns at partially interruptible callsites
VSadov Dec 9, 2023
ac41f7b
enable on all platforms
VSadov Dec 9, 2023
590a165
tweak
VSadov Dec 10, 2023
5b15ad6
fix after rebasing
VSadov Jan 27, 2024
372f268
do not record tailcalls
VSadov Feb 16, 2024
77aca7b
IsInterruptibleSafePoint
VSadov Feb 16, 2024
eed56d9
update gccover
VSadov Feb 17, 2024
8d85757
turn on new behavior on a gcinfo version
VSadov Feb 17, 2024
f7b7fc0
tailcalls tweak
VSadov Feb 18, 2024
8c7dd48
do not report unused returns
VSadov Feb 21, 2024
250f985
CORINFO_HELP_FAIL_FAST should not be a safepoint
VSadov Feb 21, 2024
1d78a87
treat tailcalls as emitNoGChelper
VSadov Feb 21, 2024
b9fccd4
versioning tweak
VSadov Feb 21, 2024
86a4fa4
enable in CoreCLR (not just for GC stress scenarios)
VSadov Feb 21, 2024
ccd5cc5
fix x86 build
VSadov Feb 22, 2024
fc0352a
other architectures
VSadov Feb 22, 2024
efe24c8
added a knob DOTNET_InterruptibleCallSites
VSadov Feb 22, 2024
1bab7f2
moved DOTNET_InterruptibleCallSites check to the code manager
VSadov Feb 23, 2024
dc56a7a
JIT_StackProbe should not be a safepoint (stack is not cleaned yet)
VSadov Feb 23, 2024
0cc6a4c
Hooked up GCInfo version to R2R file version
VSadov Feb 23, 2024
59a4baa
formatting
VSadov Feb 24, 2024
35d8e63
GCStress support for RISC architectures
VSadov Feb 25, 2024
55a06e1
Update src/coreclr/inc/gcinfo.h
VSadov Feb 25, 2024
4004d6d
make InterruptibleSafePointsEnabled static
VSadov Feb 25, 2024
c5b7296
fix linux-x86 build.
VSadov Feb 25, 2024
11425c5
ARM32 actually can`t return 2 GC references, so can filter out R1 early
VSadov Feb 27, 2024
0743c7b
revert unnecessary change
VSadov Feb 28, 2024
ef760e5
Update src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/G…
VSadov Mar 2, 2024
854c3aa
removed GCINFO_VERSION cons from GcInfo.cs
VSadov Mar 2, 2024
4a117b1
Use RBM_INTRET/RBM_INTRET_1
VSadov Mar 2, 2024
5ae3e4b
Update src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/G…
VSadov Mar 4, 2024
cae83d9
do not skip safe points twice (stress failure)
VSadov Mar 4, 2024
411f204
revert unnecessary change in gccover.
VSadov Mar 5, 2024
e0a31e1
fix after rebase
VSadov Mar 6, 2024
d68e094
make sure to check `idIsNoGC` on all codepaths in `emitOutputInstr`
VSadov Mar 7, 2024
7ed82b3
make CORINFO_HELP_CHECK_OBJ a no-gc helper (because we can)
VSadov Mar 7, 2024
a149e6a
mark a test that tests WaitForPendingFinalizers as GCStressIncompatible
VSadov Mar 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 39 additions & 5 deletions src/coreclr/gcinfo/gcinfoencoder.cpp
Expand Up @@ -922,7 +922,11 @@ void GcInfoEncoder::FinalizeSlotIds()
#endif
}

bool GcInfoEncoder::IsAlwaysScratch(GcSlotDesc &slotDesc)
#ifdef PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED

// tells whether a slot cannot contain an object reference
// at call instruction or right after returning
bool GcInfoEncoder::DoNotTrackInPartiallyInterruptible(GcSlotDesc &slotDesc)
{
#if defined(TARGET_ARM)

Expand All @@ -933,7 +937,31 @@ bool GcInfoEncoder::IsAlwaysScratch(GcSlotDesc &slotDesc)
_ASSERTE(regNum >= 0 && regNum <= 14);
_ASSERTE(regNum != 13); // sp

return ((regNum <= 3) || (regNum >= 12)); // R12 and R14/LR are both scratch registers
return ((regNum <= 3) || (regNum >= 12)) // R12 is volatile and SP/LR can't contain objects around calls
&& regNum != 0 // R0 can contain return value
;
}
else if (!slotDesc.IsUntracked() && (slotDesc.Slot.Stack.Base == GC_SP_REL) &&
((UINT32)slotDesc.Slot.Stack.SpOffset < m_SizeOfStackOutgoingAndScratchArea))
{
return TRUE;
}
else
return FALSE;

#elif defined(TARGET_ARM64)

_ASSERTE(m_SizeOfStackOutgoingAndScratchArea != (UINT32)-1);
if (slotDesc.IsRegister())
{
int regNum = (int)slotDesc.Slot.RegisterNumber;
_ASSERTE(regNum >= 0 && regNum <= 30);
_ASSERTE(regNum != 18);

return (regNum <= 17 || regNum >= 29) // X0 through X17 are scratch, FP/LR can't be used for objects around calls
&& regNum != 0 // X0 can contain return value
&& regNum != 1 // X1 can contain return value
;
}
else if (!slotDesc.IsUntracked() && (slotDesc.Slot.Stack.Base == GC_SP_REL) &&
((UINT32)slotDesc.Slot.Stack.SpOffset < m_SizeOfStackOutgoingAndScratchArea))
Expand All @@ -953,7 +981,7 @@ bool GcInfoEncoder::IsAlwaysScratch(GcSlotDesc &slotDesc)
_ASSERTE(regNum != 4); // rsp

UINT16 PreservedRegMask =
(1 << 3) // rbx
(1 << 3) // rbx
| (1 << 5) // rbp
#ifndef UNIX_AMD64_ABI
| (1 << 6) // rsi
Expand All @@ -962,7 +990,12 @@ bool GcInfoEncoder::IsAlwaysScratch(GcSlotDesc &slotDesc)
| (1 << 12) // r12
| (1 << 13) // r13
| (1 << 14) // r14
| (1 << 15); // r15
| (1 << 15) // r15
| (1 << 0) // rax - may contain return value
#ifdef UNIX_AMD64_ABI
| (1 << 2) // rdx - may contain return value
#endif
;

return !(PreservedRegMask & (1 << regNum));
}
Expand All @@ -978,6 +1011,7 @@ bool GcInfoEncoder::IsAlwaysScratch(GcSlotDesc &slotDesc)
return FALSE;
#endif
}
#endif // PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED

void GcInfoEncoder::Build()
{
Expand Down Expand Up @@ -1396,7 +1430,7 @@ void GcInfoEncoder::Build()
else
{
UINT32 slotIndex = pCurrent->SlotId;
if(!IsAlwaysScratch(m_SlotTable[slotIndex]))
if(!DoNotTrackInPartiallyInterruptible(m_SlotTable[slotIndex]))
{
BYTE becomesLive = pCurrent->BecomesLive;
_ASSERTE((liveState.ReadBit(slotIndex) && !becomesLive)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/inc/clrconfigvalues.h
Expand Up @@ -505,6 +505,7 @@ CONFIG_DWORD_INFO(INTERNAL_DiagnosticSuspend, W("DiagnosticSuspend"), 0, "")
CONFIG_DWORD_INFO(INTERNAL_SuspendDeadlockTimeout, W("SuspendDeadlockTimeout"), 40000, "")
CONFIG_DWORD_INFO(INTERNAL_SuspendThreadDeadlockTimeoutMs, W("SuspendThreadDeadlockTimeoutMs"), 2000, "")
RETAIL_CONFIG_DWORD_INFO(INTERNAL_ThreadSuspendInjection, W("INTERNAL_ThreadSuspendInjection"), 1, "Specifies whether to inject activations for thread suspension on Unix")
RETAIL_CONFIG_DWORD_INFO(INTERNAL_InterruptibleCallSites, W("InterruptibleCallSites"), 1, "Specifies whether to allow asynchronous thread interruptions at call sites (requires GCInfo v3)")

///
/// Thread (miscellaneous)
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/eetwain.h
Expand Up @@ -457,6 +457,9 @@ virtual
bool IsGcSafe( EECodeInfo *pCodeInfo,
DWORD dwRelOffset);

static
bool InterruptibleSafePointsEnabled();

#if defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
virtual
bool HasTailCalls(EECodeInfo *pCodeInfo);
Expand Down
13 changes: 10 additions & 3 deletions src/coreclr/inc/gcinfo.h
Expand Up @@ -36,7 +36,7 @@ const unsigned this_OFFSET_FLAG = 0x2; // the offset is "this"
// The current GCInfo Version
//-----------------------------------------------------------------------------

#define GCINFO_VERSION 2
#define GCINFO_VERSION 3

//-----------------------------------------------------------------------------
// GCInfoToken: A wrapper that contains the GcInfo data and version number.
Expand Down Expand Up @@ -65,9 +65,16 @@ struct GCInfoToken
}
#endif

static uint32_t ReadyToRunVersionToGcInfoVersion(uint32_t readyToRunMajorVersion)
static uint32_t ReadyToRunVersionToGcInfoVersion(uint32_t readyToRunMajorVersion, uint32_t readyToRunMinorVersion)
{
// GcInfo version is current from ReadyToRun version 2.0
// Once MINIMUM_READYTORUN_MAJOR_VERSION is bumped to 10+
// delete the following and just return GCINFO_VERSION
//
// R2R 9.0 and 9.1 use GCInfo v2
// R2R 9.2 uses GCInfo v3
if (readyToRunMajorVersion == 9 && readyToRunMinorVersion < 2)
return 2;

return GCINFO_VERSION;
}
};
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/inc/gcinfodecoder.h
Expand Up @@ -502,12 +502,17 @@ class GcInfoDecoder
//------------------------------------------------------------------------

bool IsInterruptible();
bool HasInterruptibleRanges();

#ifdef PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED
bool IsSafePoint();
bool AreSafePointsInterruptible();
bool IsInterruptibleSafePoint();

// This is used for gccoverage
bool IsSafePoint(UINT32 codeOffset);

typedef void EnumerateSafePointsCallback (UINT32 offset, void * hCallback);
typedef void EnumerateSafePointsCallback (GcInfoDecoder* decoder, UINT32 offset, void * hCallback);
void EnumerateSafePoints(EnumerateSafePointsCallback * pCallback, void * hCallback);

#endif
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/inc/gcinfoencoder.h
Expand Up @@ -542,7 +542,9 @@ class GcInfoEncoder
void SizeofSlotStateVarLengthVector(const BitArray& vector, UINT32 baseSkip, UINT32 baseRun, UINT32 * pSizeofSimple, UINT32 * pSizeofRLE, UINT32 * pSizeofRLENeg);
UINT32 WriteSlotStateVarLengthVector(BitStreamWriter &writer, const BitArray& vector, UINT32 baseSkip, UINT32 baseRun);

bool IsAlwaysScratch(GcSlotDesc &slot);
#ifdef PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED
bool DoNotTrackInPartiallyInterruptible(GcSlotDesc &slot);
#endif // PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED

// Assumes that "*ppTransitions" is has size "numTransitions", is sorted by CodeOffset then by SlotId,
// and that "*ppEndTransitions" points one beyond the end of the array. If "*ppTransitions" contains
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/readytorun.h
Expand Up @@ -33,8 +33,8 @@
// R2R Version 8.0 Changes the alignment of the Int128 type
// R2R Version 9.0 adds support for the Vector512 type
// R2R Version 9.1 adds new helpers to allocate objects on frozen segments
// R2R Version 9.2 adds MemZero and NativeMemSet helpers

// R2R Version 9.2 adds MemZero and NativeMemSet helpers,
// uses GCInfo v3, which makes safe points in partially interruptible code interruptible.

struct READYTORUN_CORE_HEADER
{
Expand Down
34 changes: 22 additions & 12 deletions src/coreclr/jit/codegenarmarch.cpp
Expand Up @@ -3503,25 +3503,35 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
// unused values are of no interest to GC.
if (!call->IsUnusedValue())
{
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else
{
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
if (call->HasMultiRegRetVal())
{
retSize = EA_GCREF;
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else if (call->gtType == TYP_BYREF)
else
{
retSize = EA_BYREF;
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
{
retSize = EA_GCREF;
}
else if (call->gtType == TYP_BYREF)
{
retSize = EA_BYREF;
}
}
}

#ifdef TARGET_ARM
// ARM32 support multireg returns, but only to return 64bit primitives.
assert(secondRetSize != EA_GCREF);
assert(secondRetSize != EA_BYREF);
#endif

DebugInfo di;
// We need to propagate the debug information to the call instruction, so we can emit
// an IL to native mapping record for the call, to support managed return value debugging.
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenlinear.cpp
Expand Up @@ -714,8 +714,7 @@ void CodeGen::genCodeForBBlist()
}
// Do likewise for blocks that end in DOES_NOT_RETURN calls
// that were not caught by the above rules. This ensures that
// gc register liveness doesn't change across call instructions
// in fully-interruptible mode.
// gc register liveness doesn't change to some random state after call instructions
else
{
GenTree* call = block->lastNode();
Expand Down
28 changes: 16 additions & 12 deletions src/coreclr/jit/codegenloongarch64.cpp
Expand Up @@ -6485,22 +6485,26 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
// unused values are of no interest to GC.
if (!call->IsUnusedValue())
{
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else
{
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
if (call->HasMultiRegRetVal())
{
retSize = EA_GCREF;
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else if (call->gtType == TYP_BYREF)
else
{
retSize = EA_BYREF;
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
{
retSize = EA_GCREF;
}
else if (call->gtType == TYP_BYREF)
{
retSize = EA_BYREF;
}
}
}

Expand Down
28 changes: 16 additions & 12 deletions src/coreclr/jit/codegenriscv64.cpp
Expand Up @@ -6561,22 +6561,26 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
// unused values are of no interest to GC.
if (!call->IsUnusedValue())
{
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else
{
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
if (call->HasMultiRegRetVal())
{
retSize = EA_GCREF;
retSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(pRetTypeDesc->GetReturnRegType(1));
}
else if (call->gtType == TYP_BYREF)
else
{
retSize = EA_BYREF;
assert(call->gtType != TYP_STRUCT);

if (call->gtType == TYP_REF)
{
retSize = EA_GCREF;
}
else if (call->gtType == TYP_BYREF)
{
retSize = EA_BYREF;
}
}
}

Expand Down
28 changes: 16 additions & 12 deletions src/coreclr/jit/codegenxarch.cpp
Expand Up @@ -6212,22 +6212,26 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA
emitAttr retSize = EA_PTRSIZE;
emitAttr secondRetSize = EA_UNKNOWN;

if (call->HasMultiRegRetVal())
// unused values are of no interest to GC.
if (!call->IsUnusedValue())
{
retSize = emitTypeSize(retTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(retTypeDesc->GetReturnRegType(1));
}
else
{
assert(!varTypeIsStruct(call));

if (call->gtType == TYP_REF)
if (call->HasMultiRegRetVal())
{
retSize = EA_GCREF;
retSize = emitTypeSize(retTypeDesc->GetReturnRegType(0));
secondRetSize = emitTypeSize(retTypeDesc->GetReturnRegType(1));
}
else if (call->gtType == TYP_BYREF)
else
{
retSize = EA_BYREF;
assert(!varTypeIsStruct(call));

if (call->gtType == TYP_REF)
{
retSize = EA_GCREF;
}
else if (call->gtType == TYP_BYREF)
{
retSize = EA_BYREF;
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/jit/emit.cpp
Expand Up @@ -2834,6 +2834,11 @@ bool emitter::emitNoGChelper(CorInfoHelpFunc helpFunc)

case CORINFO_HELP_INIT_PINVOKE_FRAME:

case CORINFO_HELP_FAIL_FAST:
case CORINFO_HELP_STACK_PROBE:

case CORINFO_HELP_CHECK_OBJ:

case CORINFO_HELP_VALIDATE_INDIRECT_CALL:
return true;

Expand Down Expand Up @@ -10008,7 +10013,7 @@ void emitter::emitStackPopLargeStk(BYTE* addr, bool isCall, unsigned char callIn

// We make a bitmask whose bits correspond to callee-saved register indices (in the sequence
// of callee-saved registers only).
for (unsigned calleeSavedRegIdx = 0; calleeSavedRegIdx < CNT_CALLEE_SAVED; calleeSavedRegIdx++)
for (unsigned calleeSavedRegIdx = 0; calleeSavedRegIdx < CNT_CALL_GC_REGS; calleeSavedRegIdx++)
{
regMaskTP calleeSavedRbm = raRbmCalleeSaveOrder[calleeSavedRegIdx];
if (emitThisGCrefRegs & calleeSavedRbm)
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/emitarm.cpp
Expand Up @@ -4758,7 +4758,8 @@ void emitter::emitIns_Call(EmitCallType callType,
emitThisGCrefRegs = gcrefRegs;
emitThisByrefRegs = byrefRegs;

id->idSetIsNoGC(emitNoGChelper(methHnd));
// for the purpose of GC safepointing tail-calls are not real calls
id->idSetIsNoGC(isJump || emitNoGChelper(methHnd));

/* Set the instruction - special case jumping a function */
instruction ins;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/emitarm64.cpp
Expand Up @@ -9024,7 +9024,8 @@ void emitter::emitIns_Call(EmitCallType callType,
emitThisGCrefRegs = gcrefRegs;
emitThisByrefRegs = byrefRegs;

id->idSetIsNoGC(emitNoGChelper(methHnd));
// for the purpose of GC safepointing tail-calls are not real calls
id->idSetIsNoGC(isJump || emitNoGChelper(methHnd));

/* Set the instruction - special case jumping a function */
instruction ins;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/emitloongarch64.cpp
Expand Up @@ -2468,7 +2468,8 @@ void emitter::emitIns_Call(EmitCallType callType,
emitThisGCrefRegs = gcrefRegs;
emitThisByrefRegs = byrefRegs;

id->idSetIsNoGC(emitNoGChelper(methHnd));
// for the purpose of GC safepointing tail-calls are not real calls
id->idSetIsNoGC(isJump || emitNoGChelper(methHnd));

/* Set the instruction - special case jumping a function */
instruction ins;
Expand Down