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.

Sign up for GitHub

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

Do not encode safe points with -1 offset. #110845

Merged
merged 6 commits into from
Dec 22, 2024
Merged

Do not encode safe points with -1 offset. #110845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89455a5
Do not record safe points with -1 adjustmnt
VSadov Jun 30, 2024
79718be
NORMALIZE_CODE_OFFSET on RISC
VSadov Jul 3, 2024
a5b967b
denormalize code offsets in ILCompiler.Reflection.ReadyToRun.Amd64
VSadov Jul 16, 2024
e27805f
bump the min R2R version and GCInfo version
VSadov Dec 21, 2024
856ebd2
Do not record Return Kind
VSadov Dec 21, 2024
f212ed2
m_ReturnKind is not needed in non-legacy GC encoder, regardless X86 o…
VSadov Dec 22, 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
62 changes: 10 additions & 52 deletions src/coreclr/gcinfo/gcinfoencoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,13 +490,6 @@ GcInfoEncoder::GcInfoEncoder(
m_IsSlotTableFrozen = FALSE;
#endif //_DEBUG

#ifndef TARGET_X86
// If the compiler doesn't set the GCInfo, report RT_Unset.
// This is used for compatibility with JITs that aren't updated to use the new API.
m_ReturnKind = RT_Unset;
#else
m_ReturnKind = RT_Illegal;
#endif // TARGET_X86
m_CodeLength = 0;
#ifdef FIXED_STACK_PARAMETER_SCRATCH_AREA
m_SizeOfStackOutgoingAndScratchArea = -1;
Expand Down Expand Up @@ -776,13 +769,6 @@ void GcInfoEncoder::SetReversePInvokeFrameSlot(INT32 spOffset)
m_ReversePInvokeFrameSlot = spOffset;
}

void GcInfoEncoder::SetReturnKind(ReturnKind returnKind)
{
_ASSERTE(IsValidReturnKind(returnKind));

m_ReturnKind = returnKind;
}

struct GcSlotDescAndId
{
GcSlotDesc m_SlotDesc;
Expand Down Expand Up @@ -1045,16 +1031,15 @@ void GcInfoEncoder::Build()
BOOL slimHeader = (!m_IsVarArg && !hasGSCookie && (m_PSPSymStackSlot == NO_PSP_SYM) &&
!hasContextParamType && (m_InterruptibleRanges.Count() == 0) && !hasReversePInvokeFrame &&
((m_StackBaseRegister == NO_STACK_BASE_REGISTER) || (NORMALIZE_STACK_BASE_REGISTER(m_StackBaseRegister) == 0))) &&
(m_SizeOfEditAndContinuePreservedArea == NO_SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA) &&
#ifdef TARGET_AMD64
!m_WantsReportOnlyLeaf &&
#elif defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
!m_HasTailCalls &&
#endif // TARGET_AMD64
!IsStructReturnKind(m_ReturnKind);
(m_SizeOfEditAndContinuePreservedArea == NO_SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA);

// All new code is generated for the latest GCINFO_VERSION.
// So, always encode RetunrKind and encode ReversePInvokeFrameSlot where applicable.
// So, always encode ReversePInvokeFrameSlot where applicable.
if (slimHeader)
{
// Slim encoding means nothing special, partially interruptible, maybe a default frame register
Expand All @@ -1065,8 +1050,6 @@ void GcInfoEncoder::Build()
assert(m_StackBaseRegister == 8 || 2 == m_StackBaseRegister);
#endif
GCINFO_WRITE(m_Info1, (m_StackBaseRegister == NO_STACK_BASE_REGISTER) ? 0 : 1, 1, FlagsSize);

GCINFO_WRITE(m_Info1, m_ReturnKind, SIZE_OF_RETURN_KIND_IN_SLIM_HEADER, RetKindSize);
}
else
{
Expand All @@ -1089,8 +1072,6 @@ void GcInfoEncoder::Build()
#endif // TARGET_AMD64
GCINFO_WRITE(m_Info1, ((m_SizeOfEditAndContinuePreservedArea != NO_SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA) ? 1 : 0), 1, FlagsSize);
GCINFO_WRITE(m_Info1, (hasReversePInvokeFrame ? 1 : 0), 1, FlagsSize);

GCINFO_WRITE(m_Info1, m_ReturnKind, SIZE_OF_RETURN_KIND_IN_FAT_HEADER, RetKindSize);
}

_ASSERTE( m_CodeLength > 0 );
Expand Down Expand Up @@ -1217,42 +1198,19 @@ void GcInfoEncoder::Build()

///////////////////////////////////////////////////////////////////////
// Normalize call sites
// Eliminate call sites that fall inside interruptible ranges
///////////////////////////////////////////////////////////////////////

_ASSERTE(m_NumCallSites == 0 || numInterruptibleRanges == 0);

UINT32 numCallSites = 0;
for(UINT32 callSiteIndex = 0; callSiteIndex < m_NumCallSites; callSiteIndex++)
{
UINT32 callSite = m_pCallSites[callSiteIndex];
// There's a contract with the EE that says for non-leaf stack frames, where the
// method is stopped at a call site, the EE will not query with the return PC, but
// rather the return PC *minus 1*.
// The reason is that variable/register liveness may change at the instruction immediately after the
// call, so we want such frames to appear as if they are "within" the call.
// Since we use "callSite" as the "key" when we search for the matching descriptor, also subtract 1 here
// (after, of course, adding the size of the call instruction to get the return PC).
callSite += m_pCallSiteSizes[callSiteIndex] - 1;
callSite += m_pCallSiteSizes[callSiteIndex];

_ASSERTE(DENORMALIZE_CODE_OFFSET(NORMALIZE_CODE_OFFSET(callSite)) == callSite);
UINT32 normOffset = NORMALIZE_CODE_OFFSET(callSite);

BOOL keepIt = TRUE;

for(UINT32 intRangeIndex = 0; intRangeIndex < numInterruptibleRanges; intRangeIndex++)
{
InterruptibleRange *pRange = &pRanges[intRangeIndex];
if(pRange->NormStopOffset > normOffset)
{
if(pRange->NormStartOffset <= normOffset)
{
keepIt = FALSE;
}
break;
}
}

if(keepIt)
m_pCallSites[numCallSites++] = normOffset;
m_pCallSites[numCallSites++] = normOffset;
}

GCINFO_WRITE_VARL_U(m_Info1, NORMALIZE_NUM_SAFE_POINTS(numCallSites), NUM_SAFE_POINTS_ENCBASE, NumCallSitesSize);
Expand Down Expand Up @@ -1419,7 +1377,7 @@ void GcInfoEncoder::Build()

for(pCurrent = pTransitions; pCurrent < pEndTransitions; )
{
if(pCurrent->CodeOffset > callSite)
if(pCurrent->CodeOffset >= callSite)
{
couldBeLive |= liveState;

Expand Down Expand Up @@ -1774,7 +1732,7 @@ void GcInfoEncoder::Build()
{
for(pCurrent = pTransitions; pCurrent < pEndTransitions; )
{
if(pCurrent->CodeOffset > callSite)
if(pCurrent->CodeOffset >= callSite)
{
// Time to record the call site

Expand Down Expand Up @@ -1873,7 +1831,7 @@ void GcInfoEncoder::Build()

for(pCurrent = pTransitions; pCurrent < pEndTransitions; )
{
if(pCurrent->CodeOffset > callSite)
if(pCurrent->CodeOffset >= callSite)
{
// Time to encode the call site

Expand Down Expand Up @@ -1920,7 +1878,7 @@ void GcInfoEncoder::Build()

for(pCurrent = pTransitions; pCurrent < pEndTransitions; )
{
if(pCurrent->CodeOffset > callSite)
if(pCurrent->CodeOffset >= callSite)
{
// Time to encode the call site
GCINFO_WRITE_VECTOR(m_Info1, liveState, CallSiteStateSize);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/gcinfo.h
View file
Open in desktop
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 3
#define GCINFO_VERSION 4

//-----------------------------------------------------------------------------
// GCInfoToken: A wrapper that contains the GcInfo data and version number.
Expand Down
8 changes: 0 additions & 8 deletions src/coreclr/inc/gcinfoencoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,6 @@ class GcInfoEncoder
GcSlotState slotState
);


//------------------------------------------------------------------------
// ReturnKind
//------------------------------------------------------------------------

void SetReturnKind(ReturnKind returnKind);

//------------------------------------------------------------------------
// Miscellaneous method information
//------------------------------------------------------------------------
Expand Down Expand Up @@ -509,7 +502,6 @@ class GcInfoEncoder
INT32 m_PSPSymStackSlot;
INT32 m_GenericsInstContextStackSlot;
GENERIC_CONTEXTPARAM_TYPE m_contextParamType;
ReturnKind m_ReturnKind;
UINT32 m_CodeLength;
UINT32 m_StackBaseRegister;
UINT32 m_SizeOfEditAndContinuePreservedArea;
Expand Down
34 changes: 11 additions & 23 deletions src/coreclr/inc/gcinfotypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,6 @@ void FASTCALL decodeCallPattern(int pattern,
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 6
#define GS_COOKIE_STACK_SLOT_ENCBASE 6
#define CODE_LENGTH_ENCBASE 8
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 4
#define STACK_BASE_REGISTER_ENCBASE 3
#define SIZE_OF_STACK_AREA_ENCBASE 3
#define SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA_ENCBASE 4
Expand Down Expand Up @@ -679,8 +677,8 @@ void FASTCALL decodeCallPattern(int pattern,
#define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>2)
#define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<2)
#define CODE_OFFSETS_NEED_NORMALIZATION 1
#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 2/4 bytes long in Thumb/ARM states,
#define DENORMALIZE_CODE_OFFSET(x) (x) // but the safe-point offsets are encoded with a -1 adjustment.
#define NORMALIZE_CODE_OFFSET(x) ((x)>>1) // Instructions are 2/4 bytes long in Thumb/ARM states,
#define DENORMALIZE_CODE_OFFSET(x) ((x)<<1)
#define NORMALIZE_REGISTER(x) (x)
#define DENORMALIZE_REGISTER(x) (x)
#define NORMALIZE_NUM_SAFE_POINTS(x) (x)
Expand All @@ -695,8 +693,6 @@ void FASTCALL decodeCallPattern(int pattern,
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 5
#define GS_COOKIE_STACK_SLOT_ENCBASE 5
#define CODE_LENGTH_ENCBASE 7
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 2
#define STACK_BASE_REGISTER_ENCBASE 1
#define SIZE_OF_STACK_AREA_ENCBASE 3
#define SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA_ENCBASE 3
Expand Down Expand Up @@ -735,9 +731,9 @@ void FASTCALL decodeCallPattern(int pattern,
#define DENORMALIZE_STACK_BASE_REGISTER(x) ((x)^29)
#define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3)
#define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3)
#define CODE_OFFSETS_NEED_NORMALIZATION 0
#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point
#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment.
#define CODE_OFFSETS_NEED_NORMALIZATION 1
#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long
#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2)
#define NORMALIZE_REGISTER(x) (x)
#define DENORMALIZE_REGISTER(x) (x)
#define NORMALIZE_NUM_SAFE_POINTS(x) (x)
Expand All @@ -750,8 +746,6 @@ void FASTCALL decodeCallPattern(int pattern,
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 6
#define GS_COOKIE_STACK_SLOT_ENCBASE 6
#define CODE_LENGTH_ENCBASE 8
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 4
#define STACK_BASE_REGISTER_ENCBASE 2 // FP encoded as 0, SP as 2.
#define SIZE_OF_STACK_AREA_ENCBASE 3
#define SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA_ENCBASE 4
Expand Down Expand Up @@ -790,9 +784,9 @@ void FASTCALL decodeCallPattern(int pattern,
#define DENORMALIZE_STACK_BASE_REGISTER(x) ((x) == 0 ? 22 : 3)
#define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3)
#define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3)
#define CODE_OFFSETS_NEED_NORMALIZATION 0
#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point
#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment.
#define CODE_OFFSETS_NEED_NORMALIZATION 1
#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long
#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2)
#define NORMALIZE_REGISTER(x) (x)
#define DENORMALIZE_REGISTER(x) (x)
#define NORMALIZE_NUM_SAFE_POINTS(x) (x)
Expand All @@ -805,8 +799,6 @@ void FASTCALL decodeCallPattern(int pattern,
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 6
#define GS_COOKIE_STACK_SLOT_ENCBASE 6
#define CODE_LENGTH_ENCBASE 8
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 4
// FP/SP encoded as 0 or 1.
#define STACK_BASE_REGISTER_ENCBASE 2
#define SIZE_OF_STACK_AREA_ENCBASE 3
Expand Down Expand Up @@ -845,9 +837,9 @@ void FASTCALL decodeCallPattern(int pattern,
#define DENORMALIZE_STACK_BASE_REGISTER(x) ((x) == 0 ? 8 : 2)
#define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3)
#define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3)
#define CODE_OFFSETS_NEED_NORMALIZATION 0
#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point
#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment.
#define CODE_OFFSETS_NEED_NORMALIZATION 1
#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long
#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2)
#define NORMALIZE_REGISTER(x) (x)
#define DENORMALIZE_REGISTER(x) (x)
#define NORMALIZE_NUM_SAFE_POINTS(x) (x)
Expand All @@ -860,8 +852,6 @@ void FASTCALL decodeCallPattern(int pattern,
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 6
#define GS_COOKIE_STACK_SLOT_ENCBASE 6
#define CODE_LENGTH_ENCBASE 8
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 4
#define STACK_BASE_REGISTER_ENCBASE 2
// FP encoded as 0, SP as 1
#define SIZE_OF_STACK_AREA_ENCBASE 3
Expand Down Expand Up @@ -924,8 +914,6 @@ PORTABILITY_WARNING("Please specialize these definitions for your platform!")
#define SECURITY_OBJECT_STACK_SLOT_ENCBASE 6
#define GS_COOKIE_STACK_SLOT_ENCBASE 6
#define CODE_LENGTH_ENCBASE 6
#define SIZE_OF_RETURN_KIND_IN_SLIM_HEADER 2
#define SIZE_OF_RETURN_KIND_IN_FAT_HEADER 2
#define STACK_BASE_REGISTER_ENCBASE 3
#define SIZE_OF_STACK_AREA_ENCBASE 6
#define SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA_ENCBASE 3
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/inc/readytorun.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
// src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
// If you update this, ensure you run `git grep MINIMUM_READYTORUN_MAJOR_VERSION`
// and handle pending work.
#define READYTORUN_MAJOR_VERSION 10
#define READYTORUN_MINOR_VERSION 0x0001
#define READYTORUN_MAJOR_VERSION 11
#define READYTORUN_MINOR_VERSION 0x0000

#define MINIMUM_READYTORUN_MAJOR_VERSION 10
#define MINIMUM_READYTORUN_MAJOR_VERSION 11

// R2R Version 2.1 adds the InliningInfo section
// R2R Version 2.2 adds the ProfileDataInfo section
Expand All @@ -38,6 +38,7 @@
// uses GCInfo v3, which makes safe points in partially interruptible code interruptible.
// R2R Version 10.0 adds support for the statics being allocated on a per type basis instead of on a per module basis, disable support for LogMethodEnter helper
// R2R Version 10.1 adds Unbox_TypeTest helper
// R2R Version 11 uses GCInfo v4, which encodes safe points without -1 offset and does not track return kinds in GCInfo

struct READYTORUN_CORE_HEADER
{
Expand Down
11 changes: 0 additions & 11 deletions src/coreclr/jit/gcencode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3709,15 +3709,6 @@ class GcInfoEncoderWithLogging
}
}

void SetReturnKind(ReturnKind returnKind)
{
m_gcInfoEncoder->SetReturnKind(returnKind);
if (m_doLogging)
{
printf("Set ReturnKind to %s.\n", ReturnKindToString(returnKind));
}
}

void SetStackBaseRegister(UINT32 registerNumber)
{
m_gcInfoEncoder->SetStackBaseRegister(registerNumber);
Expand Down Expand Up @@ -3832,8 +3823,6 @@ void GCInfo::gcInfoBlockHdrSave(GcInfoEncoder* gcInfoEncoder, unsigned methodSiz

gcInfoEncoderWithLog->SetCodeLength(methodSize);

gcInfoEncoderWithLog->SetReturnKind(getReturnKind());

if (compiler->isFramePointerUsed())
{
gcInfoEncoderWithLog->SetStackBaseRegister(REG_FPBASE);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ struct ReadyToRunHeaderConstants
{
static const uint32_t Signature = 0x00525452; // 'RTR'

static const uint32_t CurrentMajorVersion = 10;
static const uint32_t CurrentMinorVersion = 1;
static const uint32_t CurrentMajorVersion = 11;
static const uint32_t CurrentMinorVersion = 0;
};

struct ReadyToRunHeader
Expand Down
30 changes: 2 additions & 28 deletions src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,44 +213,18 @@ void UnixNativeCodeManager::EnumGcRefs(MethodInfo * pMethodInfo,

#ifdef TARGET_ARM
// Ensure that code offset doesn't have the Thumb bit set. We need
// it to be aligned to instruction start to make the !isActiveStackFrame
// branch below work.
// it to be aligned to instruction start
ASSERT(((uintptr_t)codeOffset & 1) == 0);
#endif

bool executionAborted = ((UnixNativeMethodInfo*)pMethodInfo)->executionAborted;

if (!isActiveStackFrame && !executionAborted)
{
// the reasons for this adjustment are explained in EECodeManager::EnumGcRefs
codeOffset--;
}

GcInfoDecoder decoder(
GCInfoToken(gcInfo),
GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG),
codeOffset
);

if (isActiveStackFrame)
{
// CONSIDER: We can optimize this by remembering the need to adjust in IsSafePoint and propagating into here.
// Or, better yet, maybe we should change the decoder to not require this adjustment.
// The scenario that adjustment tries to handle (fallthrough into BB with random liveness)
// does not seem possible.
if (!decoder.HasInterruptibleRanges())
{
decoder = GcInfoDecoder(
GCInfoToken(gcInfo),
GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG),
codeOffset - 1
);

assert(decoder.IsSafePoint());
}
}

ICodeManagerFlags flags = (ICodeManagerFlags)0;
bool executionAborted = ((UnixNativeMethodInfo*)pMethodInfo)->executionAborted;
if (executionAborted)
flags = ICodeManagerFlags::ExecutionAborted;

Expand Down
Loading
Loading