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

Allow async interruptions on safepoints #95565

Merged
merged 44 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
800db91
allow async interruptions on safepoints
VSadov Dec 4, 2023
72b015f
ARM64 TODO
VSadov Dec 4, 2023
b2cf275
report GC ref/byref returns at partially interruptible callsites
VSadov Dec 9, 2023
8cf3e0c
enable on all platforms
VSadov Dec 9, 2023
76b4053
tweak
VSadov Dec 10, 2023
fbaf77e
fix after rebasing
VSadov Jan 27, 2024
0a13211
do not record tailcalls
VSadov Feb 16, 2024
30a8524
IsInterruptibleSafePoint
VSadov Feb 16, 2024
df581c6
update gccover
VSadov Feb 17, 2024
d08552b
turn on new behavior on a gcinfo version
VSadov Feb 17, 2024
45f74af
tailcalls tweak
VSadov Feb 18, 2024
f656ba4
do not report unused returns
VSadov Feb 21, 2024
5487818
CORINFO_HELP_FAIL_FAST should not be a safepoint
VSadov Feb 21, 2024
604f864
treat tailcalls as emitNoGChelper
VSadov Feb 21, 2024
ee6129c
versioning tweak
VSadov Feb 21, 2024
ee75d35
enable in CoreCLR (not just for GC stress scenarios)
VSadov Feb 21, 2024
bb98cd9
fix x86 build
VSadov Feb 22, 2024
a8d9b43
other architectures
VSadov Feb 22, 2024
5a332b0
added a knob DOTNET_InterruptibleCallSites
VSadov Feb 22, 2024
3f6c088
moved DOTNET_InterruptibleCallSites check to the code manager
VSadov Feb 23, 2024
4d1895d
JIT_StackProbe should not be a safepoint (stack is not cleaned yet)
VSadov Feb 23, 2024
93eede9
Hooked up GCInfo version to R2R file version
VSadov Feb 23, 2024
8fa6a6f
formatting
VSadov Feb 24, 2024
5ac10c5
GCStress support for RISC architectures
VSadov Feb 25, 2024
43fb760
Update src/coreclr/inc/gcinfo.h
VSadov Feb 25, 2024
b17b42e
make InterruptibleSafePointsEnabled static
VSadov Feb 25, 2024
855dd1f
fix linux-x86 build.
VSadov Feb 25, 2024
8fb65ba
ARM32 actually can`t return 2 GC references, so can filter out R1 early
VSadov Feb 27, 2024
cb84de7
revert unnecessary change
VSadov Feb 28, 2024
d1cb779
Update src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/G…
VSadov Mar 2, 2024
7ad04bc
removed GCINFO_VERSION cons from GcInfo.cs
VSadov Mar 2, 2024
3702386
Use RBM_INTRET/RBM_INTRET_1
VSadov Mar 2, 2024
7b6506d
Update src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/G…
VSadov Mar 4, 2024
a816dc0
do not skip safe points twice (stress failure)
VSadov Mar 4, 2024
ef9bd7d
revert unnecessary change in gccover.
VSadov Mar 5, 2024
d6b2b9f
fix after rebase
VSadov Mar 6, 2024
06d681e
make sure to check `idIsNoGC` on all codepaths in `emitOutputInstr`
VSadov Mar 7, 2024
52e2fd6
make CORINFO_HELP_CHECK_OBJ a no-gc helper (because we can)
VSadov Mar 7, 2024
0a938e0
mark a test that tests WaitForPendingFinalizers as GCStressIncompatible
VSadov Mar 9, 2024
dc12379
NOP
VSadov Apr 9, 2024
c8f1ba1
these helpers should not form GC safe points
VSadov Apr 20, 2024
e99529c
require that the new block has BBF_HAS_LABEL
VSadov Apr 21, 2024
53c99d3
Apply suggestions from code review
VSadov May 1, 2024
40e4526
updated JITEEVersionIdentifier GUID
VSadov May 6, 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
44 changes: 39 additions & 5 deletions src/coreclr/gcinfo/gcinfoencoder.cpp
Original file line number Diff line number Diff line change
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
Copy link
Member Author

@VSadov VSadov Jan 27, 2024

Choose a reason for hiding this comment

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

ARM/ARM64 has more changes than x64 here because some of this is just to get the filtering to the same level as on x64. (looks like we had some NYIs for ARM)

Otherwise, it would be just removing X0/X1 from being filtered out unconditionally.

&& regNum != 0 // R0 can contain return value
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that R1 can contain a part of 64bit return value as well, however R1 never returns an object/byref

;
}
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,9 @@ virtual
bool IsGcSafe( EECodeInfo *pCodeInfo,
DWORD dwRelOffset);

static
bool InterruptibleSafePointsEnabled();
VSadov marked this conversation as resolved.
Show resolved Hide resolved

#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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* bd8c41d4-8531-49c1-a600-0ae9bfe05de1 */
0xbd8c41d4,
0x8531,
0x49c1,
{0xa6, 0x00, 0x0a, 0xe9, 0xbf, 0xe0, 0x5d, 0xe1}
constexpr GUID JITEEVersionIdentifier = { /* 43854594-cd60-45df-a89f-5b7697586f46 */
0x43854594,
0xcd60,
0x45df,
{0xa8, 0x9f, 0x5b, 0x76, 0x97, 0x58, 0x6f, 0x46}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/readytorun.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
// 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.3 adds BulkWriteBarrier helper

// uses GCInfo v3, which makes safe points in partially interruptible code interruptible.
Copy link
Member Author

@VSadov VSadov Feb 25, 2024

Choose a reason for hiding this comment

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

MemZero stuff has made it first to revving the minor version to 9.2.
We have not shipped anything with that though, so I will also use 9.2 and just added a comment here


struct READYTORUN_CORE_HEADER
{
Expand Down
34 changes: 22 additions & 12 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
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
4 changes: 2 additions & 2 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -997,8 +997,8 @@ void CodeGen::genLogLabel(BasicBlock* bb)
void CodeGen::genDefineTempLabel(BasicBlock* label)
{
genLogLabel(label);
label->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur,
gcInfo.gcRegByrefSetCur DEBUG_ARG(label));
label->bbEmitCookie =
GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur);
}

// genDefineInlineTempLabel: Define an inline label that does not affect the GC
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ void CodeGen::genCodeForBBlist()
// Mark a label and update the current set of live GC refs

block->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur,
gcInfo.gcRegByrefSetCur DEBUG_ARG(block));
gcInfo.gcRegByrefSetCur, block->Prev());
}

if (block->IsFirstColdBlock(compiler))
Expand Down 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
Original file line number Diff line number Diff line change
Expand Up @@ -6356,22 +6356,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
Original file line number Diff line number Diff line change
Expand Up @@ -6500,22 +6500,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
Original file line number Diff line number Diff line change
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
Loading
Loading