From 9e174586de7fcfd8457a03e80c46fd6dc3125066 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 17 Jul 2024 19:48:44 +0300 Subject: [PATCH] Move the NoGC helper classification to HelperCallProperties Fixes a TODO and centralizes the assumptions about helpers in one place. --- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/emit.cpp | 81 ++--------------------------------- src/coreclr/jit/emit.h | 3 +- src/coreclr/jit/optimizer.cpp | 2 +- src/coreclr/jit/utils.cpp | 56 ++++++++++++++++++------ src/coreclr/jit/utils.h | 8 ++++ 6 files changed, 58 insertions(+), 94 deletions(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 69e95d60d58ae2..9035d6b367aa75 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -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; diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 206066200a47cd..d91b822afa11a9 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -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. // @@ -2867,6 +2791,7 @@ 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); @@ -2874,7 +2799,7 @@ bool emitter::emitNoGChelper(CORINFO_METHOD_HANDLE methHnd) { return false; } - return emitNoGChelper(helpFunc); + return Compiler::s_helperCallProperties.IsNoGC(helpFunc); } /***************************************************************************** @@ -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) { diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 83bcd1e5dfe694..83c08395118eff 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -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? diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index d388e9cc7017f5..8fb5ca04935662 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -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)) { diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index 81d426939d3dfa..ae174c24780303 100644 --- a/src/coreclr/jit/utils.cpp +++ b/src/coreclr/jit/utils.cpp @@ -1520,6 +1520,7 @@ 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) { @@ -1527,6 +1528,8 @@ void HelperCallProperties::init() 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: @@ -1538,7 +1541,6 @@ void HelperCallProperties::init() case CORINFO_HELP_DBLREM: case CORINFO_HELP_FLTROUND: case CORINFO_HELP_DBLROUND: - isPure = true; noThrow = true; break; @@ -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: @@ -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; @@ -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: @@ -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. @@ -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; } @@ -1836,6 +1865,7 @@ void HelperCallProperties::init() m_mutatesHeap[helper] = mutatesHeap; m_mayRunCctor[helper] = mayRunCctor; m_isNoEscape[helper] = isNoEscape; + m_isNoGC[helper] = isNoGC; } } diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h index 29a1d7d29c933c..b0b86736bc0b7b 100644 --- a/src/coreclr/jit/utils.h +++ b/src/coreclr/jit/utils.h @@ -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(); @@ -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]; + } }; //*****************************************************************************