From 0bdcdc37c9299e4d0a5d0efcb295f7e8665422ed Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 15:58:39 +0200 Subject: [PATCH 1/9] Add support for delegate GDV in R2R and NAOT Adds limited support for getFunctionFixedEntrypoint in R2R and NAOT and starts using it for delegate target comparisons. --- src/coreclr/jit/codegenarmarch.cpp | 29 ++++++------ src/coreclr/jit/flowgraph.cpp | 3 ++ src/coreclr/jit/gentree.cpp | 4 +- src/coreclr/jit/gentree.h | 47 ++++++++----------- src/coreclr/jit/importer.cpp | 4 ++ src/coreclr/jit/importercalls.cpp | 11 ++--- src/coreclr/jit/indirectcalltransformer.cpp | 26 +++++++++- src/coreclr/jit/lsraarmarch.cpp | 6 +-- src/coreclr/jit/morph.cpp | 13 ++--- src/coreclr/jit/objectalloc.cpp | 1 + src/coreclr/jit/rationalize.cpp | 1 + .../tools/Common/JitInterface/CorInfoImpl.cs | 3 -- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 31 ++++++++++++ .../JitInterface/CorInfoImpl.RyuJit.cs | 5 ++ 14 files changed, 116 insertions(+), 68 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 9fa35607307197..a3e352236305f1 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -3144,21 +3144,21 @@ void CodeGen::genCall(GenTreeCall* call) genConsumeReg(target); } #ifdef FEATURE_READYTORUN - else if (call->IsR2ROrVirtualStubRelativeIndir()) + else { - assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || - ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); - assert(call->gtControlExpr == nullptr); - - regNumber tmpReg = call->GetSingleTempReg(); - // Register where we save call address in should not be overridden by epilog. - assert((tmpReg & (RBM_INT_CALLEE_TRASH & ~RBM_LR)) == tmpReg); - - regNumber callAddrReg = - call->IsVirtualStubRelativeIndir() ? compiler->virtualStubParamInfo->GetReg() : REG_R2R_INDIRECT_PARAM; - GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), tmpReg, callAddrReg); - // We will use this again when emitting the jump in genCallInstruction in the epilog - call->gtRsvdRegs |= genRegMask(tmpReg); + regNumber indirCellReg = getCallIndirectionCellReg(call); + if (indirCellReg != REG_NA) + { + assert(call->gtControlExpr == nullptr); + + regNumber tmpReg = call->GetSingleTempReg(); + // Register where we save call address in should not be overridden by epilog. + assert((tmpReg & (RBM_INT_CALLEE_TRASH & ~RBM_LR)) == tmpReg); + + GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), tmpReg, indirCellReg); + // We will use this again when emitting the jump in genCallInstruction in the epilog + call->gtRsvdRegs |= genRegMask(tmpReg); + } } #endif @@ -3409,7 +3409,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call) if (callThroughIndirReg != REG_NA) { - assert(call->IsR2ROrVirtualStubRelativeIndir()); regNumber targetAddrReg = call->GetSingleTempReg(); // For fast tailcalls we have already loaded the call target when processing the call node. if (!call->IsFastTailCall()) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 413165417b76a0..976b9c7b808850 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1150,6 +1150,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer, targetObjPointers); call->setEntryPoint(pLookup.constLookup); + call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { @@ -1161,6 +1162,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer, targetObjPointers, ctxTree); call->setEntryPoint(genericLookup); + call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } } else @@ -1182,6 +1184,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, clsHnd, &entryPoint); assert(!entryPoint.lookupKind.needsRuntimeLookup); call->setEntryPoint(entryPoint.constLookup); + call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 2f511fa8807613..509e201231696d 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -9997,9 +9997,9 @@ void Compiler::gtDispNodeName(GenTree* tree) gtfType = " stub"; } #ifdef FEATURE_READYTORUN - else if (tree->AsCall()->IsR2RRelativeIndir()) + else if (tree->AsCall()->IsR2RCall()) { - gtfType = " r2r_ind"; + gtfType = " r2rcall"; } #endif // FEATURE_READYTORUN else if (tree->gtFlags & GTF_CALL_UNMANAGED) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 240a9bf7329616..f9b12f3d883cff 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4140,7 +4140,7 @@ enum GenTreeCallFlags : unsigned int // a Pinvoke but not as an unmanaged call. See impCheckForPInvokeCall() to // know when these flags are set. - GTF_CALL_M_R2R_REL_INDIRECT = 0x00002000, // ready to run call is indirected through a relative address + GTF_CALL_M_R2R_CALL = 0x00002000, // ready to run call that might need indir cell GTF_CALL_M_DOES_NOT_RETURN = 0x00004000, // call does not return GTF_CALL_M_WRAPPER_DELEGATE_INV = 0x00008000, // call is in wrapper delegate GTF_CALL_M_FAT_POINTER_CHECK = 0x00010000, // NativeAOT managed calli needs transformation, that checks @@ -5215,18 +5215,6 @@ struct GenTreeCall final : public GenTree return (gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0; } - bool IsR2ROrVirtualStubRelativeIndir() - { -#if defined(FEATURE_READYTORUN) - if (IsR2RRelativeIndir()) - { - return true; - } -#endif - - return IsVirtualStubRelativeIndir(); - } - bool HasNonStandardAddedArgs(Compiler* compiler) const; int GetNonStandardAddedArgCount(Compiler* compiler) const; @@ -5396,10 +5384,10 @@ struct GenTreeCall final : public GenTree return IsVirtualStub() && (gtCallMoreFlags & GTF_CALL_M_VIRTSTUB_REL_INDIRECT) != 0; } - bool IsR2RRelativeIndir() const + bool IsR2RCall() const { #ifdef FEATURE_READYTORUN - return (gtCallMoreFlags & GTF_CALL_M_R2R_REL_INDIRECT) != 0; + return (gtCallMoreFlags & GTF_CALL_M_R2R_CALL) != 0; #else return false; #endif @@ -5408,10 +5396,6 @@ struct GenTreeCall final : public GenTree void setEntryPoint(const CORINFO_CONST_LOOKUP& entryPoint) { gtEntryPoint = entryPoint; - if (gtEntryPoint.accessType == IAT_PVALUE) - { - gtCallMoreFlags |= GTF_CALL_M_R2R_REL_INDIRECT; - } } #endif // FEATURE_READYTORUN @@ -5536,17 +5520,26 @@ struct GenTreeCall final : public GenTree return WellKnownArg::VirtualStubCell; } -#if defined(TARGET_ARMARCH) - // For ARM architectures, we always use an indirection cell for R2R calls. - if (IsR2RRelativeIndir() && !IsDelegateInvoke()) + // TODO-R2R: Today we assume that a IAT_PVALUE entry point + // returned by R2R in all cases but getFunctionFixedEntryPoint + // means it needs indir cell. Clean this up so that R2R actually + // just tells us explicitly when an indir cell is needed. + // We optimize delegate invokes manually in lowering and do not need + // indir cells for them. + CLANG_FORMAT_COMMENT_ANCHOR; +#ifdef FEATURE_READYTORUN + if (IsR2RCall() && !IsDelegateInvoke() && (gtEntryPoint.accessType == IAT_PVALUE)) { +#if defined(TARGET_ARMARCH) + // For ARM architectures we always use an indirection cell for R2R calls. return WellKnownArg::R2RIndirectionCell; - } #elif defined(TARGET_XARCH) - // On XARCH we disassemble it from callsite except for tailcalls that need indirection cell. - if (IsR2RRelativeIndir() && IsFastTailCall()) - { - return WellKnownArg::R2RIndirectionCell; + // On XARCH we disassemble it from callsite except for tailcalls that need indirection cell. + if (IsFastTailCall()) + { + return WellKnownArg::R2RIndirectionCell; + } +#endif } #endif diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 4b9e86932f0804..5c0dbc6f3c157f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1904,6 +1904,7 @@ GenTreeCall* Compiler::impReadyToRunHelperToTree(CORINFO_RESOLVED_TOKEN* pResolv GenTreeCall* op1 = gtNewHelperCallNode(helper, type, arg1); op1->setEntryPoint(lookup); + op1->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; return op1; } @@ -3388,6 +3389,7 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr); call->setEntryPoint(pCallInfo->codePointerLookup.constLookup); + call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; return call; } @@ -4421,6 +4423,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= callFlags; op1->AsCall()->setEntryPoint(pFieldInfo->fieldLookup); + op1->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else #endif @@ -4454,6 +4457,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= callFlags; op1->AsCall()->setEntryPoint(pFieldInfo->fieldLookup); + op1->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; op1 = gtNewOperNode(GT_ADD, type, op1, gtNewIconNode(pFieldInfo->offset, innerFldSeq)); #else unreached(); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 78f05c62192fd4..5e243d5178a943 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -474,6 +474,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (opts.IsReadyToRun()) { call->AsCall()->setEntryPoint(callInfo->codePointerLookup.constLookup); + call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } #endif break; @@ -4643,13 +4644,7 @@ void Compiler::pickGDV(GenTreeCall* call, LikelyClassMethodRecord likelyMethods[maxLikelyMethods]; unsigned numberOfMethods = 0; - // TODO-GDV: R2R support requires additional work to reacquire the - // entrypoint, similar to what happens at the end of impDevirtualizeCall. - // As part of supporting this we should merge the tail of - // impDevirtualizeCall and what happens in - // GuardedDevirtualizationTransformer::CreateThen for method GDV. - // - if (!opts.IsReadyToRun() && (call->IsVirtualVtable() || call->IsDelegateInvoke())) + if (call->IsVirtualVtable() || call->IsDelegateInvoke()) { numberOfMethods = getLikelyMethods(likelyMethods, maxLikelyMethods, fgPgoSchema, fgPgoSchemaCount, fgPgoData, ilOffset); @@ -6258,8 +6253,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // Update the call. call->gtCallMoreFlags &= ~GTF_CALL_M_VIRTSTUB_REL_INDIRECT; - call->gtCallMoreFlags &= ~GTF_CALL_M_R2R_REL_INDIRECT; call->setEntryPoint(derivedCallInfo.codePointerLookup.constLookup); + call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } #endif // FEATURE_READYTORUN } diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 9879cd5e094bee..917b54fb9c60cb 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -450,10 +450,18 @@ class IndirectCallTransformer class GuardedDevirtualizationTransformer final : public Transformer { +#ifdef FEATURE_READYTORUN + CORINFO_CONST_LOOKUP m_r2rEntryPoint; +#endif + public: GuardedDevirtualizationTransformer(Compiler* compiler, BasicBlock* block, Statement* stmt) : Transformer(compiler, block, stmt), returnTemp(BAD_VAR_NUM) { +#ifdef FEATURE_READYTORUN + m_r2rEntryPoint.accessType = IAT_VALUE; + m_r2rEntryPoint.addr = nullptr; +#endif } //------------------------------------------------------------------------ @@ -607,6 +615,9 @@ class IndirectCallTransformer // path. if (origCall->IsVirtualVtable()) { + // In R2R all virtual vtable calls should go through VSD, so we should not get here. + assert(!compiler->opts.IsReadyToRun() || compiler->IsTargetAbi(CORINFO_NATIVEAOT_ABI)); + GenTree* tarTree = compiler->fgExpandVirtualVtableCallTarget(origCall); CORINFO_METHOD_HANDLE methHnd = guardedInfo->guardedMethodHandle; @@ -629,6 +640,11 @@ class IndirectCallTransformer CORINFO_CONST_LOOKUP lookup; compiler->info.compCompHnd->getFunctionFixedEntryPoint(methHnd, false, &lookup); +#ifdef FEATURE_READYTORUN + // For R2R we also need to store the entry point on the direct call node. + m_r2rEntryPoint = lookup; +#endif + GenTree* compareTarTree = CreateTreeForLookup(methHnd, lookup); compare = compiler->gtNewOperNode(GT_NE, TYP_INT, compareTarTree, tarTree); } @@ -821,8 +837,14 @@ class IndirectCallTransformer call->gtCallType = CT_USER_FUNC; call->gtCallMoreFlags |= GTF_CALL_M_DEVIRTUALIZED; call->gtCallMoreFlags &= ~GTF_CALL_M_DELEGATE_INV; - // TODO-GDV: To support R2R we need to get the entry point - // here. We should unify with the tail of impDevirtualizeCall. + +#ifdef FEATURE_READYTORUN + // We should have set this in CreateCheck. + assert(m_r2rEntryPoint.accessType != IAT_VALUE || m_r2rEntryPoint.addr != nullptr); + call->setEntryPoint(m_r2rEntryPoint); + // Call through fixed entry point does not need special R2R handling. + call->gtCallMoreFlags &= ~GTF_CALL_M_R2R_CALL; +#endif if (origCall->IsVirtual()) { diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index 2c8a16af1a8646..334567a1f8510f 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -186,10 +186,10 @@ int LinearScan::BuildCall(GenTreeCall* call) assert(ctrlExprCandidates != RBM_NONE); } } - else if (call->IsR2ROrVirtualStubRelativeIndir()) + else if (call->GetIndirectionCellArgKind() != WellKnownArg::None) { - // For R2R and VSD we have stub address in REG_R2R_INDIRECT_PARAM - // and will load call address into the temp register from this register. + // If we have indirection cell then we will load call address into the + // temp register from this register. regMaskTP candidates = RBM_NONE; if (call->IsFastTailCall()) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d4eee4b3d21c30..b6d0306c94ce56 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2115,12 +2115,8 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call // That's ok; after making something a tailcall, we will invalidate this information // and reconstruct it if necessary. The tailcalling decision does not change since // this is a non-standard arg in a register. - bool needsIndirectionCell = call->IsR2RRelativeIndir() && !call->IsDelegateInvoke(); -#if defined(TARGET_XARCH) - needsIndirectionCell &= call->IsFastTailCall(); -#endif - if (needsIndirectionCell) + if (call->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) { assert(call->gtEntryPoint.addr != nullptr); @@ -5940,7 +5936,8 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason) } #ifdef TARGET_ARM - if (callee->IsR2RRelativeIndir() || callee->HasNonStandardAddedArgs(this)) + if ((callee->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) || + callee->HasNonStandardAddedArgs(this)) { reportFastTailCallDecision( "Method with non-standard args passed in callee saved register cannot be tail called"); @@ -6664,8 +6661,8 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // For R2R we might need a different entry point for this call if we are doing a tailcall. // The reason is that the normal delay load helper uses the return address to find the indirection // cell in xarch, but now the JIT is expected to leave the indirection cell in REG_R2R_INDIRECT_PARAM: - // We optimize delegate invocations manually in the JIT so skip this for those. - if (call->IsR2RRelativeIndir() && canFastTailCall && !fastTailCallToLoop && !call->IsDelegateInvoke()) + if ((call->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) && canFastTailCall && + !fastTailCallToLoop) { info.compCompHnd->updateEntryPointForTailCall(&call->gtEntryPoint); diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 1f251f48d38d17..f369e821d3cdb9 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -475,6 +475,7 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc { assert(comp->opts.IsReadyToRun()); helperCall->AsCall()->setEntryPoint(entryPoint); + helperCall->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 7e2b2376391f8d..2f784050c85b4c 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -188,6 +188,7 @@ void Rationalizer::RewriteNodeAsCall(GenTree** use, #ifdef FEATURE_READYTORUN call->AsCall()->setEntryPoint(entryPoint); + call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; #endif call = comp->fgMorphArgs(call); diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 950266e9645bf8..618155da815641 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3210,9 +3210,6 @@ private uint getThreadTLSIndex(ref void* ppIndirection) } } - private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsafeFunctionPointer, ref CORINFO_CONST_LOOKUP pResult) - { throw new NotImplementedException("getFunctionFixedEntryPoint"); } - #pragma warning disable CA1822 // Mark members as static private CorInfoHelpFunc getLazyStringLiteralHelper(CORINFO_MODULE_STRUCT_* handle) #pragma warning restore CA1822 // Mark members as static diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index c366a6e4f08358..7b27bb2045a3ca 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1097,6 +1097,37 @@ private void getFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, ref CORINFO_CONS throw new RequiresRuntimeJitException(HandleToObject(ftn).ToString()); } + private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsafeFunctionPointer, ref CORINFO_CONST_LOOKUP pResult) + { + MethodDesc md = HandleToObject(ftn); + + if (!_compilation.CompilationModuleGroup.VersionsWithMethodBody(md)) + throw new RequiresRuntimeJitException($"{md} passed to getFunctionFixedEntryPoint is not in version bubble"); + + // This function is only used for delegate GDV and the runtime + // generally does not profile delegates involving instantiating + // stubs. The only time we can see this should thus be due to stale + // PGO data. + if (md.IsSharedByGenericInstantiations) + throw new RequiresRuntimeJitException($"{md} is shared, cannot get fixed entry point"); + + if (md.GetTypicalMethodDefinition() is not EcmaMethod ecmaMethod) + throw new RequiresRuntimeJitException($"{md} is not in metadata"); + + ModuleToken module = new ModuleToken(ecmaMethod.Module, ecmaMethod.Handle); + MethodWithToken mt = new MethodWithToken(md, module, null, false, null); + + PrepareForUseAsAFunctionPointer(md); + + pResult = + CreateConstLookupToSymbol( + _compilation.NodeFactory.MethodEntrypoint( + mt, + isInstantiatingStub: false, + isPrecodeImportRequired: true, + isJumpableImportRequired: false)); + } + private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUCT_* declaredCalleeHnd, CORINFO_METHOD_STRUCT_* exactCalleeHnd, bool fIsTailPrefix) { if (!fIsTailPrefix) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 72ee00ccd76dab..bdc2f46d4452f9 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -786,6 +786,11 @@ method.OwningType is MetadataType mdType && pResult = CreateConstLookupToSymbol(_compilation.NodeFactory.MethodEntrypoint(method)); } + private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsafeFunctionPointer, ref CORINFO_CONST_LOOKUP pResult) + { + getFunctionEntryPoint(ftn, ref pResult, CORINFO_ACCESS_FLAGS.CORINFO_ACCESS_LDFTN); + } + private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUCT_* declaredCalleeHnd, CORINFO_METHOD_STRUCT_* exactCalleeHnd, bool fIsTailPrefix) { // Assume we can tail call unless proved otherwise From 9e175a40f0e6499be6cccc7c95c79065c7c086a1 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 16:36:45 +0200 Subject: [PATCH 2/9] Fix shared generics check --- src/coreclr/jit/morph.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b6d0306c94ce56..5ca434eeb8cc5a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -6661,8 +6661,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // For R2R we might need a different entry point for this call if we are doing a tailcall. // The reason is that the normal delay load helper uses the return address to find the indirection // cell in xarch, but now the JIT is expected to leave the indirection cell in REG_R2R_INDIRECT_PARAM: - if ((call->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) && canFastTailCall && - !fastTailCallToLoop) + if (call->IsR2RCall() && canFastTailCall && !fastTailCallToLoop) { info.compCompHnd->updateEntryPointForTailCall(&call->gtEntryPoint); From 61f8ed3a3173ac175fe2485a343ffb746714ad24 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 22:06:21 +0200 Subject: [PATCH 3/9] Support it only for non-generics for now Need a R2R expert to make this work for generics --- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 7b27bb2045a3ca..f3a1d35efd719e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1104,14 +1104,11 @@ private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsa if (!_compilation.CompilationModuleGroup.VersionsWithMethodBody(md)) throw new RequiresRuntimeJitException($"{md} passed to getFunctionFixedEntryPoint is not in version bubble"); - // This function is only used for delegate GDV and the runtime - // generally does not profile delegates involving instantiating - // stubs. The only time we can see this should thus be due to stale - // PGO data. - if (md.IsSharedByGenericInstantiations) - throw new RequiresRuntimeJitException($"{md} is shared, cannot get fixed entry point"); - - if (md.GetTypicalMethodDefinition() is not EcmaMethod ecmaMethod) + // TODO: Currently generic instantiations are not supported. + if (!md.IsTypicalMethodDefinition) + throw new RequiresRuntimeJitException($"Cannot currently resolve entry point for generic method {md}"); + + if (md is not EcmaMethod ecmaMethod) throw new RequiresRuntimeJitException($"{md} is not in metadata"); ModuleToken module = new ModuleToken(ecmaMethod.Module, ecmaMethod.Handle); From 36cfc547a00d649dad3205e1f364e31ec7d9a267 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 22:06:31 +0200 Subject: [PATCH 4/9] Use regular R2R entry point in JIT --- src/coreclr/jit/indirectcalltransformer.cpp | 24 ++++----------- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 30 +++++++++++++------ 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 917b54fb9c60cb..66a1f821e12295 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -450,18 +450,10 @@ class IndirectCallTransformer class GuardedDevirtualizationTransformer final : public Transformer { -#ifdef FEATURE_READYTORUN - CORINFO_CONST_LOOKUP m_r2rEntryPoint; -#endif - public: GuardedDevirtualizationTransformer(Compiler* compiler, BasicBlock* block, Statement* stmt) : Transformer(compiler, block, stmt), returnTemp(BAD_VAR_NUM) { -#ifdef FEATURE_READYTORUN - m_r2rEntryPoint.accessType = IAT_VALUE; - m_r2rEntryPoint.addr = nullptr; -#endif } //------------------------------------------------------------------------ @@ -640,11 +632,6 @@ class IndirectCallTransformer CORINFO_CONST_LOOKUP lookup; compiler->info.compCompHnd->getFunctionFixedEntryPoint(methHnd, false, &lookup); -#ifdef FEATURE_READYTORUN - // For R2R we also need to store the entry point on the direct call node. - m_r2rEntryPoint = lookup; -#endif - GenTree* compareTarTree = CreateTreeForLookup(methHnd, lookup); compare = compiler->gtNewOperNode(GT_NE, TYP_INT, compareTarTree, tarTree); } @@ -839,11 +826,12 @@ class IndirectCallTransformer call->gtCallMoreFlags &= ~GTF_CALL_M_DELEGATE_INV; #ifdef FEATURE_READYTORUN - // We should have set this in CreateCheck. - assert(m_r2rEntryPoint.accessType != IAT_VALUE || m_r2rEntryPoint.addr != nullptr); - call->setEntryPoint(m_r2rEntryPoint); - // Call through fixed entry point does not need special R2R handling. - call->gtCallMoreFlags &= ~GTF_CALL_M_R2R_CALL; + if (compiler->opts.IsReadyToRun()) + { + CORINFO_CONST_LOOKUP entryPoint; + compiler->info.compCompHnd->getFunctionEntryPoint(methodHnd, &entryPoint); + call->setEntryPoint(entryPoint); + } #endif if (origCall->IsVirtual()) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index f3a1d35efd719e..cbdad6d0509728 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1092,17 +1092,19 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) return _compilation.NodeFactory.GetReadyToRunHelperCell(id); } - private void getFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, ref CORINFO_CONST_LOOKUP pResult, CORINFO_ACCESS_FLAGS accessFlags) - { - throw new RequiresRuntimeJitException(HandleToObject(ftn).ToString()); - } - - private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsafeFunctionPointer, ref CORINFO_CONST_LOOKUP pResult) + // Used to get entry points for methods for delegate GDV, so we only + // expect to see these used rarely. JIT needs both the prestub that is + // stored in the delegate and the R2R entry point that it can put on + // the devirtualized call. + // We could give it the prestub in both cases but in case the + // devirtualized call is not inlined the R2R entry point has better + // steady state performance. + private CORINFO_CONST_LOOKUP GetFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool prestub) { MethodDesc md = HandleToObject(ftn); if (!_compilation.CompilationModuleGroup.VersionsWithMethodBody(md)) - throw new RequiresRuntimeJitException($"{md} passed to getFunctionFixedEntryPoint is not in version bubble"); + throw new RequiresRuntimeJitException($"{md} passed to GetFunctionEntryPoint is not in version bubble"); // TODO: Currently generic instantiations are not supported. if (!md.IsTypicalMethodDefinition) @@ -1116,15 +1118,25 @@ private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsa PrepareForUseAsAFunctionPointer(md); - pResult = + return CreateConstLookupToSymbol( _compilation.NodeFactory.MethodEntrypoint( mt, isInstantiatingStub: false, - isPrecodeImportRequired: true, + isPrecodeImportRequired: prestub, isJumpableImportRequired: false)); } + private void getFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, ref CORINFO_CONST_LOOKUP pResult, CORINFO_ACCESS_FLAGS accessFlags) + { + pResult = GetFunctionEntryPoint(ftn, prestub: false); + } + + private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, bool isUnsafeFunctionPointer, ref CORINFO_CONST_LOOKUP pResult) + { + pResult = GetFunctionEntryPoint(ftn, prestub: true); + } + private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUCT_* declaredCalleeHnd, CORINFO_METHOD_STRUCT_* exactCalleeHnd, bool fIsTailPrefix) { if (!fIsTailPrefix) From 8f9fd06d1d5c42016074ec9a389300dc461e5863 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 22:12:11 +0200 Subject: [PATCH 5/9] Revert R2R_CALL changes Not necessary now that entry point is a normal R2R call. --- src/coreclr/jit/codegenarmarch.cpp | 29 +++++++++--------- src/coreclr/jit/flowgraph.cpp | 3 -- src/coreclr/jit/gentree.cpp | 4 +-- src/coreclr/jit/gentree.h | 47 +++++++++++++++++------------- src/coreclr/jit/importer.cpp | 4 --- src/coreclr/jit/importercalls.cpp | 3 +- src/coreclr/jit/lsraarmarch.cpp | 6 ++-- src/coreclr/jit/morph.cpp | 12 +++++--- src/coreclr/jit/objectalloc.cpp | 1 - src/coreclr/jit/rationalize.cpp | 1 - 10 files changed, 56 insertions(+), 54 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index a3e352236305f1..9fa35607307197 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -3144,21 +3144,21 @@ void CodeGen::genCall(GenTreeCall* call) genConsumeReg(target); } #ifdef FEATURE_READYTORUN - else + else if (call->IsR2ROrVirtualStubRelativeIndir()) { - regNumber indirCellReg = getCallIndirectionCellReg(call); - if (indirCellReg != REG_NA) - { - assert(call->gtControlExpr == nullptr); - - regNumber tmpReg = call->GetSingleTempReg(); - // Register where we save call address in should not be overridden by epilog. - assert((tmpReg & (RBM_INT_CALLEE_TRASH & ~RBM_LR)) == tmpReg); - - GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), tmpReg, indirCellReg); - // We will use this again when emitting the jump in genCallInstruction in the epilog - call->gtRsvdRegs |= genRegMask(tmpReg); - } + assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || + ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); + assert(call->gtControlExpr == nullptr); + + regNumber tmpReg = call->GetSingleTempReg(); + // Register where we save call address in should not be overridden by epilog. + assert((tmpReg & (RBM_INT_CALLEE_TRASH & ~RBM_LR)) == tmpReg); + + regNumber callAddrReg = + call->IsVirtualStubRelativeIndir() ? compiler->virtualStubParamInfo->GetReg() : REG_R2R_INDIRECT_PARAM; + GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), tmpReg, callAddrReg); + // We will use this again when emitting the jump in genCallInstruction in the epilog + call->gtRsvdRegs |= genRegMask(tmpReg); } #endif @@ -3409,6 +3409,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) if (callThroughIndirReg != REG_NA) { + assert(call->IsR2ROrVirtualStubRelativeIndir()); regNumber targetAddrReg = call->GetSingleTempReg(); // For fast tailcalls we have already loaded the call target when processing the call node. if (!call->IsFastTailCall()) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 976b9c7b808850..413165417b76a0 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1150,7 +1150,6 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer, targetObjPointers); call->setEntryPoint(pLookup.constLookup); - call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { @@ -1162,7 +1161,6 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer, targetObjPointers, ctxTree); call->setEntryPoint(genericLookup); - call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } } else @@ -1184,7 +1182,6 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call, clsHnd, &entryPoint); assert(!entryPoint.lookupKind.needsRuntimeLookup); call->setEntryPoint(entryPoint.constLookup); - call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 509e201231696d..2f511fa8807613 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -9997,9 +9997,9 @@ void Compiler::gtDispNodeName(GenTree* tree) gtfType = " stub"; } #ifdef FEATURE_READYTORUN - else if (tree->AsCall()->IsR2RCall()) + else if (tree->AsCall()->IsR2RRelativeIndir()) { - gtfType = " r2rcall"; + gtfType = " r2r_ind"; } #endif // FEATURE_READYTORUN else if (tree->gtFlags & GTF_CALL_UNMANAGED) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index f9b12f3d883cff..240a9bf7329616 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4140,7 +4140,7 @@ enum GenTreeCallFlags : unsigned int // a Pinvoke but not as an unmanaged call. See impCheckForPInvokeCall() to // know when these flags are set. - GTF_CALL_M_R2R_CALL = 0x00002000, // ready to run call that might need indir cell + GTF_CALL_M_R2R_REL_INDIRECT = 0x00002000, // ready to run call is indirected through a relative address GTF_CALL_M_DOES_NOT_RETURN = 0x00004000, // call does not return GTF_CALL_M_WRAPPER_DELEGATE_INV = 0x00008000, // call is in wrapper delegate GTF_CALL_M_FAT_POINTER_CHECK = 0x00010000, // NativeAOT managed calli needs transformation, that checks @@ -5215,6 +5215,18 @@ struct GenTreeCall final : public GenTree return (gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0; } + bool IsR2ROrVirtualStubRelativeIndir() + { +#if defined(FEATURE_READYTORUN) + if (IsR2RRelativeIndir()) + { + return true; + } +#endif + + return IsVirtualStubRelativeIndir(); + } + bool HasNonStandardAddedArgs(Compiler* compiler) const; int GetNonStandardAddedArgCount(Compiler* compiler) const; @@ -5384,10 +5396,10 @@ struct GenTreeCall final : public GenTree return IsVirtualStub() && (gtCallMoreFlags & GTF_CALL_M_VIRTSTUB_REL_INDIRECT) != 0; } - bool IsR2RCall() const + bool IsR2RRelativeIndir() const { #ifdef FEATURE_READYTORUN - return (gtCallMoreFlags & GTF_CALL_M_R2R_CALL) != 0; + return (gtCallMoreFlags & GTF_CALL_M_R2R_REL_INDIRECT) != 0; #else return false; #endif @@ -5396,6 +5408,10 @@ struct GenTreeCall final : public GenTree void setEntryPoint(const CORINFO_CONST_LOOKUP& entryPoint) { gtEntryPoint = entryPoint; + if (gtEntryPoint.accessType == IAT_PVALUE) + { + gtCallMoreFlags |= GTF_CALL_M_R2R_REL_INDIRECT; + } } #endif // FEATURE_READYTORUN @@ -5520,26 +5536,17 @@ struct GenTreeCall final : public GenTree return WellKnownArg::VirtualStubCell; } - // TODO-R2R: Today we assume that a IAT_PVALUE entry point - // returned by R2R in all cases but getFunctionFixedEntryPoint - // means it needs indir cell. Clean this up so that R2R actually - // just tells us explicitly when an indir cell is needed. - // We optimize delegate invokes manually in lowering and do not need - // indir cells for them. - CLANG_FORMAT_COMMENT_ANCHOR; -#ifdef FEATURE_READYTORUN - if (IsR2RCall() && !IsDelegateInvoke() && (gtEntryPoint.accessType == IAT_PVALUE)) - { #if defined(TARGET_ARMARCH) - // For ARM architectures we always use an indirection cell for R2R calls. + // For ARM architectures, we always use an indirection cell for R2R calls. + if (IsR2RRelativeIndir() && !IsDelegateInvoke()) + { return WellKnownArg::R2RIndirectionCell; + } #elif defined(TARGET_XARCH) - // On XARCH we disassemble it from callsite except for tailcalls that need indirection cell. - if (IsFastTailCall()) - { - return WellKnownArg::R2RIndirectionCell; - } -#endif + // On XARCH we disassemble it from callsite except for tailcalls that need indirection cell. + if (IsR2RRelativeIndir() && IsFastTailCall()) + { + return WellKnownArg::R2RIndirectionCell; } #endif diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 5c0dbc6f3c157f..4b9e86932f0804 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1904,7 +1904,6 @@ GenTreeCall* Compiler::impReadyToRunHelperToTree(CORINFO_RESOLVED_TOKEN* pResolv GenTreeCall* op1 = gtNewHelperCallNode(helper, type, arg1); op1->setEntryPoint(lookup); - op1->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; return op1; } @@ -3389,7 +3388,6 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr); call->setEntryPoint(pCallInfo->codePointerLookup.constLookup); - call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; return call; } @@ -4423,7 +4421,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= callFlags; op1->AsCall()->setEntryPoint(pFieldInfo->fieldLookup); - op1->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else #endif @@ -4457,7 +4454,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= callFlags; op1->AsCall()->setEntryPoint(pFieldInfo->fieldLookup); - op1->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; op1 = gtNewOperNode(GT_ADD, type, op1, gtNewIconNode(pFieldInfo->offset, innerFldSeq)); #else unreached(); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5e243d5178a943..8be3c77f5fbd0e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -474,7 +474,6 @@ var_types Compiler::impImportCall(OPCODE opcode, if (opts.IsReadyToRun()) { call->AsCall()->setEntryPoint(callInfo->codePointerLookup.constLookup); - call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } #endif break; @@ -6253,8 +6252,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // Update the call. call->gtCallMoreFlags &= ~GTF_CALL_M_VIRTSTUB_REL_INDIRECT; + call->gtCallMoreFlags &= ~GTF_CALL_M_R2R_REL_INDIRECT; call->setEntryPoint(derivedCallInfo.codePointerLookup.constLookup); - call->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } #endif // FEATURE_READYTORUN } diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index 334567a1f8510f..2c8a16af1a8646 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -186,10 +186,10 @@ int LinearScan::BuildCall(GenTreeCall* call) assert(ctrlExprCandidates != RBM_NONE); } } - else if (call->GetIndirectionCellArgKind() != WellKnownArg::None) + else if (call->IsR2ROrVirtualStubRelativeIndir()) { - // If we have indirection cell then we will load call address into the - // temp register from this register. + // For R2R and VSD we have stub address in REG_R2R_INDIRECT_PARAM + // and will load call address into the temp register from this register. regMaskTP candidates = RBM_NONE; if (call->IsFastTailCall()) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 5ca434eeb8cc5a..d4eee4b3d21c30 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2115,8 +2115,12 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call // That's ok; after making something a tailcall, we will invalidate this information // and reconstruct it if necessary. The tailcalling decision does not change since // this is a non-standard arg in a register. + bool needsIndirectionCell = call->IsR2RRelativeIndir() && !call->IsDelegateInvoke(); +#if defined(TARGET_XARCH) + needsIndirectionCell &= call->IsFastTailCall(); +#endif - if (call->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) + if (needsIndirectionCell) { assert(call->gtEntryPoint.addr != nullptr); @@ -5936,8 +5940,7 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason) } #ifdef TARGET_ARM - if ((callee->GetIndirectionCellArgKind() == WellKnownArg::R2RIndirectionCell) || - callee->HasNonStandardAddedArgs(this)) + if (callee->IsR2RRelativeIndir() || callee->HasNonStandardAddedArgs(this)) { reportFastTailCallDecision( "Method with non-standard args passed in callee saved register cannot be tail called"); @@ -6661,7 +6664,8 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // For R2R we might need a different entry point for this call if we are doing a tailcall. // The reason is that the normal delay load helper uses the return address to find the indirection // cell in xarch, but now the JIT is expected to leave the indirection cell in REG_R2R_INDIRECT_PARAM: - if (call->IsR2RCall() && canFastTailCall && !fastTailCallToLoop) + // We optimize delegate invocations manually in the JIT so skip this for those. + if (call->IsR2RRelativeIndir() && canFastTailCall && !fastTailCallToLoop && !call->IsDelegateInvoke()) { info.compCompHnd->updateEntryPointForTailCall(&call->gtEntryPoint); diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index f369e821d3cdb9..1f251f48d38d17 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -475,7 +475,6 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc { assert(comp->opts.IsReadyToRun()); helperCall->AsCall()->setEntryPoint(entryPoint); - helperCall->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; } else { diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 2f784050c85b4c..7e2b2376391f8d 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -188,7 +188,6 @@ void Rationalizer::RewriteNodeAsCall(GenTree** use, #ifdef FEATURE_READYTORUN call->AsCall()->setEntryPoint(entryPoint); - call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_R2R_CALL; #endif call = comp->fgMorphArgs(call); From fd2edd38734753a26404f3e5aab551243a332476 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 20 Oct 2022 22:26:39 +0200 Subject: [PATCH 6/9] Prefilter PGO data --- .../tools/Common/JitInterface/CorInfoImpl.cs | 23 +++++++------- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 30 ++++++++++++++----- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 618155da815641..f1a9662c1d67d9 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3843,7 +3843,7 @@ private PgoSchemaElem[] getPgoInstrumentationResults(MethodDesc method) private MemoryStream _cachedMemoryStream = new MemoryStream(); - public static void ComputeJitPgoInstrumentationSchema(Func objectToHandle, PgoSchemaElem[] pgoResultsSchemas, out PgoInstrumentationSchema[] nativeSchemas, MemoryStream instrumentationData, Func typeFilter = null) + public static void ComputeJitPgoInstrumentationSchema(Func objectToHandle, PgoSchemaElem[] pgoResultsSchemas, out PgoInstrumentationSchema[] nativeSchemas, MemoryStream instrumentationData, Func typeFilter = null, Func methodFilter = null) { nativeSchemas = new PgoInstrumentationSchema[pgoResultsSchemas.Length]; instrumentationData.SetLength(0); @@ -3887,11 +3887,11 @@ public static void ComputeJitPgoInstrumentationSchema(Func objec if (typeVal.AsType != null && (typeFilter == null || typeFilter(typeVal.AsType))) { - ptrVal = (IntPtr)objectToHandle(typeVal.AsType); + ptrVal = objectToHandle(typeVal.AsType); } - else if (typeVal.AsMethod != null) + else if (typeVal.AsMethod != null && (methodFilter == null || methodFilter(typeVal.AsMethod))) { - ptrVal = (IntPtr)objectToHandle(typeVal.AsMethod); + ptrVal = objectToHandle(typeVal.AsMethod); } else { @@ -3925,13 +3925,16 @@ private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref } else { -#pragma warning disable SA1001, SA1113, SA1115 // Commas should be spaced correctly - ComputeJitPgoInstrumentationSchema(ObjectToHandle, pgoResultsSchemas, out var nativeSchemas, _cachedMemoryStream -#if !READYTORUN - , _compilation.CanConstructType + Func typeFilter = null; + Func methodFilter = null; + +#if READYTORUN + methodFilter = CanGetEntryPoint; +#else + typeFilter = _compilation.CanConstructType; #endif - ); -#pragma warning restore SA1001, SA1113, SA1115 // Commas should be spaced correctly + + ComputeJitPgoInstrumentationSchema(ObjectToHandle, pgoResultsSchemas, out var nativeSchemas, _cachedMemoryStream, typeFilter, methodFilter); var instrumentationData = _cachedMemoryStream.ToArray(); pgoResults.pInstrumentationData = (byte*)GetPin(instrumentationData); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index cbdad6d0509728..69e90081cdd704 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1092,6 +1092,21 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) return _compilation.NodeFactory.GetReadyToRunHelperCell(id); } + private bool CanGetEntryPoint(MethodDesc md) + { + if (!_compilation.CompilationModuleGroup.VersionsWithMethodBody(md)) + return false; + + // TODO: Currently generic instantiations are not supported. + if (!md.IsTypicalMethodDefinition) + return false; + + if (md is not EcmaMethod ecmaMethod) + return false; + + return true; + } + // Used to get entry points for methods for delegate GDV, so we only // expect to see these used rarely. JIT needs both the prestub that is // stored in the delegate and the R2R entry point that it can put on @@ -1103,15 +1118,14 @@ private CORINFO_CONST_LOOKUP GetFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, { MethodDesc md = HandleToObject(ftn); - if (!_compilation.CompilationModuleGroup.VersionsWithMethodBody(md)) - throw new RequiresRuntimeJitException($"{md} passed to GetFunctionEntryPoint is not in version bubble"); - - // TODO: Currently generic instantiations are not supported. - if (!md.IsTypicalMethodDefinition) - throw new RequiresRuntimeJitException($"Cannot currently resolve entry point for generic method {md}"); + if (!CanGetEntryPoint(md)) + { + // We do not expect JIT to call this except for for delegate + // GDV methods, and we prefilter the PGO data we pass to JIT. + throw new RequiresRuntimeJitException($"Cannot get entry point for {md}"); + } - if (md is not EcmaMethod ecmaMethod) - throw new RequiresRuntimeJitException($"{md} is not in metadata"); + EcmaMethod ecmaMethod = (EcmaMethod)md; ModuleToken module = new ModuleToken(ecmaMethod.Module, ecmaMethod.Handle); MethodWithToken mt = new MethodWithToken(md, module, null, false, null); From 4a8391d80d79e24e13a79ac6405196b8cd335bb7 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 18 Nov 2022 14:51:37 +0100 Subject: [PATCH 7/9] Fix local name --- .../aot/ILCompiler.RyuJit/Compiler/ProfileDataManager.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/ProfileDataManager.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/ProfileDataManager.cs index 22cf86291fd107..e0690e24d8e582 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/ProfileDataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/ProfileDataManager.cs @@ -16,18 +16,18 @@ public class ProfileDataManager public ProfileDataManager(IEnumerable mibcFiles, CompilerTypeSystemContext context) { - List _inputData = new List(); + List inputData = new List(); foreach (string file in mibcFiles) { using (PEReader peReader = MIbcProfileParser.OpenMibcAsPEReader(file)) { - _inputData.Add(MIbcProfileParser.ParseMIbcFile(context, peReader, null, null)); + inputData.Add(MIbcProfileParser.ParseMIbcFile(context, peReader, null, null)); } } // Merge all data together - foreach (ProfileData profileData in _inputData) + foreach (ProfileData profileData in inputData) { ProfileData.MergeProfileData(_mergedProfileData, profileData); } From de40fbb84e040eae5335358edf69e766142fd098 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 18 Nov 2022 14:51:51 +0100 Subject: [PATCH 8/9] Allow generic signatures in PGO synthesis Otherwise we cannot synthesize for delegate calls. --- .../aot/ILCompiler.ReadyToRun/Compiler/ProfileDataManager.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ProfileDataManager.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ProfileDataManager.cs index 66525f5bed5c66..ebc246d518824f 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ProfileDataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ProfileDataManager.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; using ILCompiler.IBC; using Internal.IL; @@ -199,7 +200,7 @@ private PgoSchemaElem[] SynthesizeSchema(Compilation comp, MethodDesc method) if (targetMeth == null) continue; - if (targetMeth.Signature.IsStatic || !targetMeth.IsTypicalMethodDefinition) + if (targetMeth.Signature.IsStatic) continue; bool isDelegateInvoke = targetMeth.OwningType.IsDelegate && targetMeth.Name == "Invoke"; From a22aba86eebe872c68c99ad8fc75a0b3ce8ba863 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 18 Nov 2022 15:04:44 +0100 Subject: [PATCH 9/9] Support GT_JMP --- src/coreclr/jit/codegenarmarch.cpp | 12 +++++++++++- src/coreclr/jit/codegenloongarch64.cpp | 11 ++++++++++- src/coreclr/jit/codegenxarch.cpp | 12 +++++++++++- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 5 +++-- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 9fa35607307197..04aa780e44bc05 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -5259,7 +5259,17 @@ void CodeGen::genFnEpilog(BasicBlock* block) if (jmpEpilog && lastNode->gtOper == GT_JMP) { methHnd = (CORINFO_METHOD_HANDLE)lastNode->AsVal()->gtVal1; - compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + + if (compiler->opts.IsReadyToRun()) + { + // R2R entry points may require additional args that we do not + // handle here. + compiler->info.compCompHnd->getFunctionFixedEntryPoint(methHnd, false, &addrInfo); + } + else + { + compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + } } #ifdef TARGET_ARM diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index 12ced2641d167d..d740aa40600e29 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -1378,7 +1378,16 @@ void CodeGen::genFnEpilog(BasicBlock* block) if (jmpEpilog && (lastNode->gtOper == GT_JMP)) { methHnd = (CORINFO_METHOD_HANDLE)lastNode->AsVal()->gtVal1; - compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + if (compiler->opts.IsReadyToRun()) + { + // R2R entry points may require additional args that we do not + // handle here. + compiler->info.compCompHnd->getFunctionFixedEntryPoint(methHnd, false, &addrInfo); + } + else + { + compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + } } compiler->unwindBegEpilog(); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 5849504b0f4029..44213b581cbd50 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -9938,7 +9938,17 @@ void CodeGen::genFnEpilog(BasicBlock* block) CORINFO_METHOD_HANDLE methHnd = (CORINFO_METHOD_HANDLE)jmpNode->AsVal()->gtVal1; CORINFO_CONST_LOOKUP addrInfo; - compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + if (compiler->opts.IsReadyToRun()) + { + // R2R entry points may require additional args that we do not + // handle here. + compiler->info.compCompHnd->getFunctionFixedEntryPoint(methHnd, false, &addrInfo); + } + else + { + compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &addrInfo); + } + if (addrInfo.accessType != IAT_VALUE && addrInfo.accessType != IAT_PVALUE) { NO_WAY("Unsupported JMP indirection"); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 433bb923a3853b..ba8b785f91be51 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1126,8 +1126,9 @@ private CORINFO_CONST_LOOKUP GetFunctionEntryPoint(CORINFO_METHOD_STRUCT_* ftn, if (!CanGetEntryPoint(md)) { - // We do not expect JIT to call this except for for delegate - // GDV methods, and we prefilter the PGO data we pass to JIT. + // We only expect this to be called in rare situations: + // 1. For delegate GDV as part of the guard. We prefilter GDV data so that we do not fail it here. + // 2. For GT_JMP, which is very rare so we have no problem falling back to runtime JIT. throw new RequiresRuntimeJitException($"Cannot get entry point for {md}"); }