Skip to content

Commit

Permalink
Simplify making IL base JIT Helpers (#108349)
Browse files Browse the repository at this point in the history
- Currently, for best performance, one needs to know to update a switch statement in jitinterface.cpp whenever a helper is moved to managed
- Instead, with this change, if the helper has a specified managed method implementation, that check will just work
- Also, if there is a native implementation, then the native implementation will take precedence. (This is to allow for helpers to have alternative assembly based implementations)
- NOTE, it is also possible to disable using an indirection, as that produces slightly smaller code, which is useful for helpers which are used only in cold code paths. This is done by using `DYNAMICJITHELPER_NOINDIRECT` to define the helper.

This should help fix a regression #108203 introduced with my recent work around generics lookups.
  • Loading branch information
davidwrighton authored Oct 18, 2024
1 parent 023686e commit 1fa61b9
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 45 deletions.
32 changes: 19 additions & 13 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
#define DYNAMICJITHELPER(code,fn,binderId) JITHELPER(code,fn,binderId)
#endif

#ifndef DYNAMICJITHELPER_NOINDIRECT
//I should try to call these methods via a non-indirect call, as those are typically smaller
//and that is an improvement for these dynamic helpers.
#define DYNAMICJITHELPER_NOINDIRECT(code,fn,binderId) DYNAMICJITHELPER(code,fn,binderId)
#endif

// pfnHelper is set to NULL if it is a stubbed helper.
// It will be set in InitJITHelpers2

Expand Down Expand Up @@ -119,11 +125,11 @@
DYNAMICJITHELPER(CORINFO_HELP_THROW, IL_Throw, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_RETHROW, IL_Rethrow, METHOD__NIL)
JITHELPER(CORINFO_HELP_USER_BREAKPOINT, JIT_UserBreakpoint, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_RNGCHKFAIL, NULL, METHOD__THROWHELPERS__THROWINDEXOUTOFRANGEEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_OVERFLOW, NULL, METHOD__THROWHELPERS__THROWOVERFLOWEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROWDIVZERO, NULL, METHOD__THROWHELPERS__THROWDIVIDEBYZEROEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROWNULLREF, NULL, METHOD__THROWHELPERS__THROWNULLREFEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_VERIFICATION, NULL, METHOD__THROWHELPERS__THROWVERIFICATIONEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_RNGCHKFAIL, NULL, METHOD__THROWHELPERS__THROWINDEXOUTOFRANGEEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_OVERFLOW, NULL, METHOD__THROWHELPERS__THROWOVERFLOWEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROWDIVZERO, NULL, METHOD__THROWHELPERS__THROWDIVIDEBYZEROEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROWNULLREF, NULL, METHOD__THROWHELPERS__THROWNULLREFEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_VERIFICATION, NULL, METHOD__THROWHELPERS__THROWVERIFICATIONEXCEPTION)
JITHELPER(CORINFO_HELP_FAIL_FAST, JIT_FailFast, METHOD__NIL)
JITHELPER(CORINFO_HELP_METHOD_ACCESS_EXCEPTION,JIT_ThrowMethodAccessException, METHOD__NIL)
JITHELPER(CORINFO_HELP_FIELD_ACCESS_EXCEPTION,JIT_ThrowFieldAccessException, METHOD__NIL)
Expand Down Expand Up @@ -285,13 +291,13 @@
JITHELPER(CORINFO_HELP_LOOP_CLONE_CHOICE_ADDR, JIT_LoopCloneChoiceAddr, METHOD__NIL)
JITHELPER(CORINFO_HELP_DEBUG_LOG_LOOP_CLONING, JIT_DebugLogLoopCloning, METHOD__NIL)

DYNAMICJITHELPER(CORINFO_HELP_THROW_ARGUMENTEXCEPTION, NULL, METHOD__THROWHELPERS__THROWARGUMENTEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION, NULL, METHOD__THROWHELPERS__THROWARGUMENTOUTOFRANGEEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROW_NOT_IMPLEMENTED, NULL, METHOD__THROWHELPERS__THROWNOTIMPLEMENTEDEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, NULL, METHOD__THROWHELPERS__THROWPLATFORMNOTSUPPORTEDEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, NULL, METHOD__THROWHELPERS__THROWTYPENOTSUPPORTED)
DYNAMICJITHELPER(CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, NULL, METHOD__THROWHELPERS__THROWAMBIGUOUSRESOLUTIONEXCEPTION)
DYNAMICJITHELPER(CORINFO_HELP_THROW_ENTRYPOINT_NOT_FOUND_EXCEPTION, NULL, METHOD__THROWHELPERS__THROWENTRYPOINTNOTFOUNDEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_ARGUMENTEXCEPTION, NULL, METHOD__THROWHELPERS__THROWARGUMENTEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION, NULL, METHOD__THROWHELPERS__THROWARGUMENTOUTOFRANGEEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_NOT_IMPLEMENTED, NULL, METHOD__THROWHELPERS__THROWNOTIMPLEMENTEDEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, NULL, METHOD__THROWHELPERS__THROWPLATFORMNOTSUPPORTEDEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, NULL, METHOD__THROWHELPERS__THROWTYPENOTSUPPORTED)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, NULL, METHOD__THROWHELPERS__THROWAMBIGUOUSRESOLUTIONEXCEPTION)
DYNAMICJITHELPER_NOINDIRECT(CORINFO_HELP_THROW_ENTRYPOINT_NOT_FOUND_EXCEPTION, NULL, METHOD__THROWHELPERS__THROWENTRYPOINTNOTFOUNDEXCEPTION)

JITHELPER(CORINFO_HELP_JIT_PINVOKE_BEGIN, JIT_PInvokeBegin, METHOD__NIL)
JITHELPER(CORINFO_HELP_JIT_PINVOKE_END, JIT_PInvokeEnd, METHOD__NIL)
Expand Down Expand Up @@ -338,4 +344,4 @@
#undef JITHELPER
#undef DYNAMICJITHELPER
#undef JITHELPER
#undef DYNAMICJITHELPER
#undef DYNAMICJITHELPER_NOINDIRECT
15 changes: 11 additions & 4 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5179,10 +5179,17 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize,

callTarget = callTargetReg;

// adrp + add with relocations will be emitted
GetEmitter()->emitIns_R_AI(INS_adrp, EA_PTR_DSP_RELOC, callTarget,
(ssize_t)pAddr DEBUGARG((size_t)compiler->eeFindHelper(helper))
DEBUGARG(GTF_ICON_METHOD_HDL));
if (compiler->opts.compReloc)
{
// adrp + add with relocations will be emitted
GetEmitter()->emitIns_R_AI(INS_adrp, EA_PTR_DSP_RELOC, callTarget,
(ssize_t)pAddr DEBUGARG((size_t)compiler->eeFindHelper(helper))
DEBUGARG(GTF_ICON_METHOD_HDL));
}
else
{
instGen_Set_Reg_To_Imm(EA_PTRSIZE, callTarget, (ssize_t)addr);
}
GetEmitter()->emitIns_R_R(INS_ldr, EA_PTRSIZE, callTarget, callTarget);
callType = emitter::EC_INDIR_R;
}
Expand Down
30 changes: 29 additions & 1 deletion src/coreclr/vm/jithelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4441,7 +4441,7 @@ VMHELPDEF hlpDynamicFuncTable[DYNAMIC_CORINFO_HELP_COUNT] =
static const BinderMethodID hlpDynamicToBinderMap[DYNAMIC_CORINFO_HELP_COUNT] =
{
#define JITHELPER(code, pfnHelper, binderId)
#define DYNAMICJITHELPER(code, pfnHelper, binderId) (BinderMethodID)binderId,
#define DYNAMICJITHELPER(code, pfnHelper, binderId) (pfnHelper != NULL) ? (BinderMethodID)METHOD__NIL : (BinderMethodID)binderId, // If pre-compiled code is provided for a jit helper, prefer that over the IL implementation
#include "jithelpers.h"
};

Expand Down Expand Up @@ -4504,6 +4504,34 @@ VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** metho
return hlpDynamicFuncTable[ftnNum];
}

bool HasILBasedDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum)
{
STANDARD_VM_CONTRACT;

_ASSERTE(ftnNum < DYNAMIC_CORINFO_HELP_COUNT);

return (METHOD__NIL != hlpDynamicToBinderMap[ftnNum]);
}

bool IndirectionAllowedForJitHelper(CorInfoHelpFunc ftnNum)
{
STANDARD_VM_CONTRACT;

_ASSERTE(ftnNum < CORINFO_HELP_COUNT);

if (
#define DYNAMICJITHELPER(code,fn,binderId)
#define JITHELPER(code,fn,binderId)
#define DYNAMICJITHELPER_NOINDIRECT(code,fn,binderId) (code == ftnNum) ||
#include "jithelpers.h"
false)
{
return false;
}

return true;
}

/*********************************************************************/
// Initialize the part of the JIT helpers that require much of the
// EE infrastructure to be in place.
Expand Down
37 changes: 10 additions & 27 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10701,6 +10701,7 @@ void* CEEJitInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN */
_ASSERTE(ppIndirection != NULL);
_ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != NULL); // Confirm the helper is non-null and doesn't require lazy loading.
*ppIndirection = &hlpDynamicFuncTable[dynamicFtnNum].pfnHelper;
_ASSERTE(IndirectionAllowedForJitHelper(ftnNum));
result = NULL;
goto exit;
}
Expand All @@ -10715,28 +10716,7 @@ void* CEEJitInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN */
goto exit;
}

if (dynamicFtnNum == DYNAMIC_CORINFO_HELP_ISINSTANCEOFINTERFACE ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_ISINSTANCEOFANY ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_ISINSTANCEOFARRAY ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_ISINSTANCEOFCLASS ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_CHKCASTANY ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_CHKCASTARRAY ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_CHKCASTINTERFACE ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_CHKCASTCLASS ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_CHKCASTCLASS_SPECIAL ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_UNBOX ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_ARRADDR_ST ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_LDELEMA_REF ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_MEMSET ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_MEMZERO ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_MEMCPY ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_BULK_WRITEBARRIER ||
IN_TARGET_32BIT(dynamicFtnNum == DYNAMIC_CORINFO_HELP_LMUL_OVF ||)
IN_TARGET_32BIT(dynamicFtnNum == DYNAMIC_CORINFO_HELP_ULMUL_OVF ||)
dynamicFtnNum == DYNAMIC_CORINFO_HELP_DBL2INT_OVF ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_DBL2LNG_OVF ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_DBL2UINT_OVF ||
dynamicFtnNum == DYNAMIC_CORINFO_HELP_DBL2ULNG_OVF)
if (HasILBasedDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum))
{
MethodDesc* helperMD = NULL;
(void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, &helperMD);
Expand Down Expand Up @@ -10774,11 +10754,14 @@ void* CEEJitInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN */
}
}

Precode* pPrecode = helperMD->GetPrecode();
_ASSERTE(pPrecode->GetType() == PRECODE_FIXUP);
*ppIndirection = ((FixupPrecode*)pPrecode)->GetTargetSlot();
result = NULL;
goto exit;
if (IndirectionAllowedForJitHelper(ftnNum))
{
Precode* pPrecode = helperMD->GetPrecode();
_ASSERTE(pPrecode->GetType() == PRECODE_FIXUP);
*ppIndirection = ((FixupPrecode*)pPrecode)->GetTargetSlot();
result = NULL;
goto exit;
}
}

pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum).pfnHelper;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,8 @@ GARY_DECL(VMHELPDEF, hlpDynamicFuncTable, DYNAMIC_CORINFO_HELP_COUNT);
void _SetJitHelperFunction(DynamicCorInfoHelpFunc ftnNum, void * pFunc);

VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc = NULL);
bool HasILBasedDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum);
bool IndirectionAllowedForJitHelper(CorInfoHelpFunc ftnNum);

void *GenFastGetSharedStaticBase(bool bCheckCCtor);

Expand Down

0 comments on commit 1fa61b9

Please sign in to comment.