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

Move the NoGC helper classification to HelperCallProperties #105040

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Move the NoGC helper classification to HelperCallProperties #105040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3322,7 +3322,7 @@ inline bool Compiler::IsPotentialGCSafePoint(GenTree* tree) const
if (((tree->gtFlags & GTF_CALL) != 0))
{
// if this is not a No-GC helper
if (!tree->IsCall() || !emitter::emitNoGChelper(tree->AsCall()->GetHelperNum()))
if (!tree->IsHelperCall() || !s_helperCallProperties.IsNoGC(tree->AsCall()->GetHelperNum()))
{
// assume that we have a safe point.
return true;
Expand Down
81 changes: 3 additions & 78 deletions src/coreclr/jit/emit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2780,82 +2780,6 @@ emitter::instrDesc* emitter::emitNewInstrCnsDsp(emitAttr size, target_ssize_t cn
}
}

//------------------------------------------------------------------------
// emitNoGChelper: Returns true if garbage collection won't happen within the helper call.
//
// Notes:
// There is no need to record live pointers for such call sites.
//
// Arguments:
// helpFunc - a helper signature for the call, can be CORINFO_HELP_UNDEF, that means that the call is not a helper.
//
// Return value:
// true if GC can't happen within this call, false otherwise.
bool emitter::emitNoGChelper(CorInfoHelpFunc helpFunc)
{
// TODO-Throughput: Make this faster (maybe via a simple table of bools?)

switch (helpFunc)
{
case CORINFO_HELP_UNDEF:
return false;

case CORINFO_HELP_PROF_FCN_LEAVE:
case CORINFO_HELP_PROF_FCN_ENTER:
case CORINFO_HELP_PROF_FCN_TAILCALL:
case CORINFO_HELP_LLSH:
case CORINFO_HELP_LRSH:
case CORINFO_HELP_LRSZ:

// case CORINFO_HELP_LMUL:
// case CORINFO_HELP_LDIV:
// case CORINFO_HELP_LMOD:
// case CORINFO_HELP_ULDIV:
// case CORINFO_HELP_ULMOD:

#ifdef TARGET_X86
case CORINFO_HELP_ASSIGN_REF_EAX:
case CORINFO_HELP_ASSIGN_REF_ECX:
case CORINFO_HELP_ASSIGN_REF_EBX:
case CORINFO_HELP_ASSIGN_REF_EBP:
case CORINFO_HELP_ASSIGN_REF_ESI:
case CORINFO_HELP_ASSIGN_REF_EDI:

case CORINFO_HELP_CHECKED_ASSIGN_REF_EAX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_ECX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EBX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EBP:
case CORINFO_HELP_CHECKED_ASSIGN_REF_ESI:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EDI:
#endif

case CORINFO_HELP_ASSIGN_REF:
case CORINFO_HELP_CHECKED_ASSIGN_REF:
case CORINFO_HELP_ASSIGN_BYREF:

case CORINFO_HELP_GET_GCSTATIC_BASE_NOCTOR:
case CORINFO_HELP_GET_NONGCSTATIC_BASE_NOCTOR:

case CORINFO_HELP_INIT_PINVOKE_FRAME:

case CORINFO_HELP_FAIL_FAST:
case CORINFO_HELP_STACK_PROBE:

case CORINFO_HELP_CHECK_OBJ:

// never present on stack at the time of GC
case CORINFO_HELP_TAILCALL:
case CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER:
case CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER_TRACK_TRANSITIONS:

case CORINFO_HELP_VALIDATE_INDIRECT_CALL:
return true;

default:
return false;
}
}

//------------------------------------------------------------------------
// emitNoGChelper: Returns true if garbage collection won't happen within the helper call.
//
Expand All @@ -2867,14 +2791,15 @@ bool emitter::emitNoGChelper(CorInfoHelpFunc helpFunc)
//
// Return value:
// true if GC can't happen within this call, false otherwise.
//
bool emitter::emitNoGChelper(CORINFO_METHOD_HANDLE methHnd)
{
CorInfoHelpFunc helpFunc = Compiler::eeGetHelperNum(methHnd);
if (helpFunc == CORINFO_HELP_UNDEF)
{
return false;
}
return emitNoGChelper(helpFunc);
return Compiler::s_helperCallProperties.IsNoGC(helpFunc);
}

/*****************************************************************************
Expand Down Expand Up @@ -10430,7 +10355,7 @@ regMaskTP emitter::emitGetGCRegsSavedOrModified(CORINFO_METHOD_HANDLE methHnd)
//
regMaskTP emitter::emitGetGCRegsKilledByNoGCCall(CorInfoHelpFunc helper)
{
assert(emitNoGChelper(helper));
assert(emitNoGChelper(Compiler::eeFindHelper(helper)));
regMaskTP result;
switch (helper)
{
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/emit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3159,13 +3159,14 @@ class emitter
/* The following is used to distinguish helper vs non-helper calls */
/************************************************************************/

static bool emitNoGChelper(CorInfoHelpFunc helpFunc);
private:
static bool emitNoGChelper(CORINFO_METHOD_HANDLE methHnd);

/************************************************************************/
/* The following logic keeps track of live GC ref values */
/************************************************************************/

public:
bool emitFullArgInfo; // full arg info (including non-ptr arg)?
bool emitFullGCinfo; // full GC pointer maps?
bool emitFullyInt; // fully interruptible code?
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6104,7 +6104,7 @@ void Compiler::optRemoveRedundantZeroInits()
{
// If compMethodRequiresPInvokeFrame() returns true, lower may later
// insert a call to CORINFO_HELP_INIT_PINVOKE_FRAME but that is not a gc-safe point.
assert(emitter::emitNoGChelper(CORINFO_HELP_INIT_PINVOKE_FRAME));
assert(s_helperCallProperties.IsNoGC(CORINFO_HELP_INIT_PINVOKE_FRAME));

if (!lclDsc->HasGCPtr() || (!GetInterruptible() && !hasGCSafePoint))
{
Expand Down
56 changes: 43 additions & 13 deletions src/coreclr/jit/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,13 +1520,16 @@ void HelperCallProperties::init()
bool mutatesHeap = false; // true if any previous heap objects [are|can be] modified
bool mayRunCctor = false; // true if the helper call may cause a static constructor to be run.
bool isNoEscape = false; // true if none of the GC ref arguments can escape
bool isNoGC = false; // true is the helper cannot trigger GC

switch (helper)
{
// Arithmetic helpers that cannot throw
case CORINFO_HELP_LLSH:
case CORINFO_HELP_LRSH:
case CORINFO_HELP_LRSZ:
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_LMUL:
case CORINFO_HELP_LNG2DBL:
case CORINFO_HELP_ULNG2DBL:
Copy link
Member

Choose a reason for hiding this comment

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

just randomly checked CORINFO_HELP_ULNG2DBL and it also looks nogc, but I presume your PR just cleans things up

Copy link
Contributor Author

@SingleAccretion SingleAccretion Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Right, this is intentionally meant to be no-diff. Additionally, these no-gc annotations are a bit scary to maintain: they must match between CoreCLR and NAOT, and only CoreCLR is tested with GCStress.

Expand All @@ -1538,7 +1541,6 @@ void HelperCallProperties::init()
case CORINFO_HELP_DBLREM:
case CORINFO_HELP_FLTROUND:
case CORINFO_HELP_DBLROUND:

isPure = true;
noThrow = true;
break;
Expand Down Expand Up @@ -1729,6 +1731,8 @@ void HelperCallProperties::init()

case CORINFO_HELP_GET_GCSTATIC_BASE_NOCTOR:
case CORINFO_HELP_GET_NONGCSTATIC_BASE_NOCTOR:
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_GETDYNAMIC_GCSTATIC_BASE_NOCTOR:
case CORINFO_HELP_GETDYNAMIC_NONGCSTATIC_BASE_NOCTOR:
case CORINFO_HELP_GETPINNED_GCSTATIC_BASE_NOCTOR:
Expand All @@ -1749,14 +1753,29 @@ void HelperCallProperties::init()
nonNullReturn = true;
break;

#ifdef TARGET_X86
case CORINFO_HELP_ASSIGN_REF_EAX:
case CORINFO_HELP_ASSIGN_REF_ECX:
case CORINFO_HELP_ASSIGN_REF_EBX:
case CORINFO_HELP_ASSIGN_REF_EBP:
case CORINFO_HELP_ASSIGN_REF_ESI:
case CORINFO_HELP_ASSIGN_REF_EDI:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EAX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_ECX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EBX:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EBP:
case CORINFO_HELP_CHECKED_ASSIGN_REF_ESI:
case CORINFO_HELP_CHECKED_ASSIGN_REF_EDI:
#endif
// GC Write barrier support
// TODO-ARM64-Bug?: Can these throw or not?
case CORINFO_HELP_ASSIGN_REF:
case CORINFO_HELP_CHECKED_ASSIGN_REF:
case CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP:
case CORINFO_HELP_ASSIGN_BYREF:
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP:
case CORINFO_HELP_BULK_WRITEBARRIER:

mutatesHeap = true;
break;

Expand All @@ -1768,11 +1787,13 @@ void HelperCallProperties::init()
case CORINFO_HELP_SETFIELDFLOAT:
case CORINFO_HELP_SETFIELDDOUBLE:
case CORINFO_HELP_ARRADDR_ST:

mutatesHeap = true;
break;

// These helper calls always throw an exception
case CORINFO_HELP_FAIL_FAST:
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_OVERFLOW:
case CORINFO_HELP_VERIFICATION:
case CORINFO_HELP_RNGCHKFAIL:
Expand All @@ -1785,17 +1806,14 @@ void HelperCallProperties::init()
case CORINFO_HELP_THROW_NOT_IMPLEMENTED:
case CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED:
case CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED:
case CORINFO_HELP_FAIL_FAST:
case CORINFO_HELP_METHOD_ACCESS_EXCEPTION:
case CORINFO_HELP_FIELD_ACCESS_EXCEPTION:
case CORINFO_HELP_CLASS_ACCESS_EXCEPTION:

alwaysThrow = true;
break;

// These helper calls may throw an exception
case CORINFO_HELP_MON_EXIT_STATIC:

break;

// This is a debugging aid; it simply returns a constant address.
Expand All @@ -1804,26 +1822,37 @@ void HelperCallProperties::init()
noThrow = true;
break;

case CORINFO_HELP_INIT_PINVOKE_FRAME:
case CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER: // Never present on stack at the time of GC.
case CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER_TRACK_TRANSITIONS:
isNoGC = true;
FALLTHROUGH;
case CORINFO_HELP_DBG_IS_JUST_MY_CODE:
case CORINFO_HELP_BBT_FCN_ENTER:
case CORINFO_HELP_POLL_GC:
case CORINFO_HELP_MON_ENTER:
case CORINFO_HELP_MON_EXIT:
case CORINFO_HELP_MON_ENTER_STATIC:
case CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER:
case CORINFO_HELP_JIT_REVERSE_PINVOKE_EXIT:
case CORINFO_HELP_GETFIELDADDR:
case CORINFO_HELP_INIT_PINVOKE_FRAME:
case CORINFO_HELP_JIT_PINVOKE_BEGIN:
case CORINFO_HELP_JIT_PINVOKE_END:

noThrow = true;
break;

// Not sure how to handle optimization involving the rest of these helpers
default:
case CORINFO_HELP_TAILCALL: // Never present on stack at the time of GC.
case CORINFO_HELP_STACK_PROBE:
case CORINFO_HELP_CHECK_OBJ:
case CORINFO_HELP_VALIDATE_INDIRECT_CALL:
case CORINFO_HELP_PROF_FCN_LEAVE:
case CORINFO_HELP_PROF_FCN_ENTER:
case CORINFO_HELP_PROF_FCN_TAILCALL:
isNoGC = true;
mutatesHeap = true; // Conservatively.
break;

// The most pessimistic results are returned for these helpers
default:
// The most pessimistic results are returned for these helpers.
mutatesHeap = true;
break;
}
Expand All @@ -1836,6 +1865,7 @@ void HelperCallProperties::init()
m_mutatesHeap[helper] = mutatesHeap;
m_mayRunCctor[helper] = mayRunCctor;
m_isNoEscape[helper] = isNoEscape;
m_isNoGC[helper] = isNoGC;
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/jit/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ class HelperCallProperties
bool m_mutatesHeap[CORINFO_HELP_COUNT];
bool m_mayRunCctor[CORINFO_HELP_COUNT];
bool m_isNoEscape[CORINFO_HELP_COUNT];
bool m_isNoGC[CORINFO_HELP_COUNT];

void init();

Expand Down Expand Up @@ -655,6 +656,13 @@ class HelperCallProperties
assert(helperId < CORINFO_HELP_COUNT);
return m_isNoEscape[helperId];
}

bool IsNoGC(CorInfoHelpFunc helperId)
{
assert(helperId > CORINFO_HELP_UNDEF);
assert(helperId < CORINFO_HELP_COUNT);
return m_isNoGC[helperId];
}
};

//*****************************************************************************
Expand Down
Loading