diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index c0de433f8328f..4e38adbe3361e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -5307,7 +5307,7 @@ struct GenTreeCall final : public GenTree #if defined(TARGET_ARMARCH) // For ARM architectures, we always use an indirection cell for R2R calls. - if (IsR2RRelativeIndir()) + if (IsR2RRelativeIndir() && !IsDelegateInvoke()) { return WellKnownArg::R2RIndirectionCell; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 98bd11f0fd2d3..741da8e6c4432 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2283,10 +2283,58 @@ void Lowering::LowerCFGCall(GenTreeCall* call) } GenTree* callTarget = call->gtCallType == CT_INDIRECT ? call->gtCallAddr : call->gtControlExpr; - if ((callTarget == nullptr) || callTarget->IsIntegralConst()) + if (callTarget == nullptr) { - // This is a direct call, no CFG check is necessary. - return; + assert((call->gtCallType != CT_INDIRECT) && (!call->IsVirtual() || call->IsVirtualStubRelativeIndir())); + if (!call->IsVirtual()) + { + // Direct call with stashed address + return; + } + + // This is a VSD call with the call target being null because we are + // supposed to load it from the indir cell. Due to CFG we will need + // this address twice, and at least on ARM64 we do not want to + // materialize the constant both times. + CallArg* indirCellArg = call->gtArgs.FindWellKnownArg(WellKnownArg::VirtualStubCell); + assert((indirCellArg != nullptr) && indirCellArg->GetNode()->OperIs(GT_PUTARG_REG)); + + GenTreeOp* putArgNode = indirCellArg->GetNode()->AsOp(); + LIR::Use indirCellArgUse(BlockRange(), &putArgNode->gtOp1, putArgNode); + + // On non-xarch, we create a local even for constants. On xarch cloning + // the constant is better since it can be contained in the load below. + bool cloneConsts = false; +#ifdef TARGET_XARCH + cloneConsts = true; +#endif + + GenTree* indirCellClone; + + if (indirCellArgUse.Def()->OperIs(GT_LCL_VAR) || (cloneConsts && indirCellArgUse.Def()->IsCnsIntOrI())) + { + indirCellClone = comp->gtClone(indirCellArgUse.Def()); + } + else + { + unsigned newLcl = indirCellArgUse.ReplaceWithLclVar(comp); + indirCellClone = comp->gtNewLclvNode(newLcl, TYP_I_IMPL); + } + + callTarget = Ind(indirCellClone); + LIR::Range controlExprRange = LIR::SeqTree(comp, callTarget); + ContainCheckRange(controlExprRange); + + BlockRange().InsertBefore(call, std::move(controlExprRange)); + call->gtControlExpr = callTarget; + } + else + { + if (callTarget->IsIntegralConst()) + { + // This is a direct call, no CFG check is necessary. + return; + } } CFGCallKind cfgKind = call->GetCFGCallKind(); @@ -5127,7 +5175,7 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) // Skip inserting the indirection node to load the address that is already // computed in the VSD stub arg register as a hidden parameter. Instead during the // codegen, just load the call target from there. - shouldOptimizeVirtualStubCall = !comp->opts.IsCFGEnabled(); + shouldOptimizeVirtualStubCall = true; #endif if (!shouldOptimizeVirtualStubCall) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d41ad4babb679..17f7cbf8f389e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2108,18 +2108,20 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call size_t addrValue = (size_t)call->gtEntryPoint.addr; GenTree* indirectCellAddress = comp->gtNewIconHandleNode(addrValue, GTF_ICON_FTN_ADDR); INDEBUG(indirectCellAddress->AsIntCon()->gtTargetHandle = (size_t)call->gtCallMethHnd); - indirectCellAddress->SetRegNum(REG_R2R_INDIRECT_PARAM); + #ifdef TARGET_ARM - // Issue #xxxx : Don't attempt to CSE this constant on ARM32 - // - // This constant has specific register requirements, and LSRA doesn't currently correctly - // handle them when the value is in a CSE'd local. + // TODO-ARM: We currently do not properly kill this register in LSRA + // (see getKillSetForCall which does so only for VSD calls). + // We should be able to remove these two workarounds once we do so, + // however when this was tried there were significant regressions. + indirectCellAddress->SetRegNum(REG_R2R_INDIRECT_PARAM); indirectCellAddress->SetDoNotCSE(); -#endif // TARGET_ARM +#endif // Push the stub address onto the list of arguments. - InsertAfterThisOrFirst(comp, - NewCallArg::Primitive(indirectCellAddress).WellKnown(WellKnownArg::R2RIndirectionCell)); + NewCallArg indirCellAddrArg = + NewCallArg::Primitive(indirectCellAddress).WellKnown(WellKnownArg::R2RIndirectionCell); + InsertAfterThisOrFirst(comp, indirCellAddrArg); } #endif @@ -7917,7 +7919,7 @@ void Compiler::fgMorphTailCallViaJitHelper(GenTreeCall* call) // call - a call that needs virtual stub dispatching. // // Return Value: -// addr tree with set resister requirements. +// addr tree // GenTree* Compiler::fgGetStubAddrArg(GenTreeCall* call) { @@ -7935,7 +7937,6 @@ GenTree* Compiler::fgGetStubAddrArg(GenTreeCall* call) INDEBUG(stubAddrArg->AsIntCon()->gtTargetHandle = (size_t)call->gtCallMethHnd); } assert(stubAddrArg != nullptr); - stubAddrArg->SetRegNum(virtualStubParamInfo->GetReg()); return stubAddrArg; } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index f32b7d79ddb0b..8e388bd521073 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9683,19 +9683,14 @@ void Compiler::fgValueNumberHelperCallFunc(GenTreeCall* call, VNFunc vnf, ValueN vnpUniq.SetBoth(vnStore->VNForExpr(compCurBB, call->TypeGet())); } -#if defined(FEATURE_READYTORUN) && (defined(TARGET_ARMARCH) || defined(TARGET_LOONGARCH64)) - if (call->IsR2RRelativeIndir()) + if (call->GetIndirectionCellArgKind() != WellKnownArg::None) { -#ifdef DEBUG - GenTree* indirectCellAddress = args->GetArgByIndex(0)->GetNode(); - assert(indirectCellAddress->IsCnsIntOrI() && indirectCellAddress->GetRegNum() == REG_R2R_INDIRECT_PARAM); -#endif // DEBUG - - // For ARM indirectCellAddress is consumed by the call itself, so it should have added as an implicit argument - // in morph. So we do not need to use EntryPointAddrAsArg0, because arg0 is already an entry point addr. + // If we are VN'ing a call with indirection cell arg (e.g. because this + // is a helper in a R2R compilation) then morph should already have + // added this arg, so we do not need to use EntryPointAddrAsArg0 + // because the indirection cell itself allows us to disambiguate. useEntryPointAddrAsArg0 = false; } -#endif // FEATURE_READYTORUN && (TARGET_ARMARCH || TARGET_LOONGARCH64) CallArg* curArg = &*args->Args().begin(); if (nArgs == 0)