From ba3d8d51789eb9ef2ae51565e98bf26649bde583 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 16 Apr 2022 02:09:17 +0200 Subject: [PATCH 1/2] ARM32: stop allocating a register for null checks on instance calls For interface tail calls we currently end up allocating two internal registers where one of them is forced into R12. LSRA currently does not always handle the low amount of freedom, in particular with some jitstressregs scenarios. Since LR is always free going into a call we can just stop allocating a register for the null check and use LR always to avoid the problem and also give LSRA a little less work to do. Fix #66563 --- src/coreclr/jit/codegenarmarch.cpp | 34 +++++++++--------------------- src/coreclr/jit/lsraarmarch.cpp | 5 ----- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 3f030275432e78..195dad43d92dd6 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -3182,17 +3182,17 @@ void CodeGen::genCall(GenTreeCall* call) } } + GenTree* target = getCallTarget(call, nullptr); + + if (target != nullptr) + { + genConsumeReg(target); + } + // Insert a null check on "this" pointer if asked. if (call->NeedsNullCheck()) { - const regNumber regThis = genGetThisArgReg(call); - -#if defined(TARGET_ARM) - const regNumber tmpReg = call->ExtractTempReg(); - GetEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, tmpReg, regThis, 0); -#elif defined(TARGET_ARM64) - GetEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, REG_ZR, regThis, 0); -#endif // TARGET* + GetEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, REG_LR, genGetThisArgReg(call), 0); } // If fast tail call, then we are done here, we just have to load the call @@ -3200,16 +3200,10 @@ void CodeGen::genCall(GenTreeCall* call) // into a volatile register that won't be restored by epilog sequence. if (call->IsFastTailCall()) { - GenTree* target = getCallTarget(call, nullptr); - - if (target != nullptr) - { - // Indirect fast tail calls materialize call target either in gtControlExpr or in gtCallAddr. - genConsumeReg(target); - } #ifdef FEATURE_READYTORUN - else if (call->IsR2ROrVirtualStubRelativeIndir()) + if (call->IsR2ROrVirtualStubRelativeIndir()) { + assert(target == nullptr); assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); assert(call->gtControlExpr == nullptr); @@ -3431,14 +3425,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call) // A call target can not be a contained indirection assert(!target->isContainedIndir()); - // For fast tailcall we have already consumed the target. We ensure in - // RA that the target was allocated into a volatile register that will - // not be messed up by epilog sequence. - if (!call->IsFastTailCall()) - { - genConsumeReg(target); - } - // We have already generated code for gtControlExpr evaluating it into a register. // We just need to emit "call reg" in this case. // diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index 0abcf59287978e..9959f06671f0f5 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -205,11 +205,6 @@ int LinearScan::BuildCall(GenTreeCall* call) buildInternalIntRegisterDefForNode(call); } - if (call->NeedsNullCheck()) - { - buildInternalIntRegisterDefForNode(call); - } - #endif // TARGET_ARM RegisterType registerType = call->TypeGet(); From 270856ef8c74bf5c13215becd3023db50af65ac0 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 16 Apr 2022 03:32:02 +0200 Subject: [PATCH 2/2] Fix target check, unify with xarch as well --- src/coreclr/jit/codegenarmarch.cpp | 8 ++--- src/coreclr/jit/codegenxarch.cpp | 52 +++++++++++------------------- 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 195dad43d92dd6..28c4ee9b9a96d1 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -3196,14 +3196,14 @@ void CodeGen::genCall(GenTreeCall* call) } // If fast tail call, then we are done here, we just have to load the call - // target into the right registers. We ensure in RA that target is loaded - // into a volatile register that won't be restored by epilog sequence. + // target into the right registers if we used an internal register for it. + // We ensure in RA that target is loaded into a volatile register that + // won't be restored by epilog sequence. if (call->IsFastTailCall()) { #ifdef FEATURE_READYTORUN - if (call->IsR2ROrVirtualStubRelativeIndir()) + if ((target == nullptr) && call->IsR2ROrVirtualStubRelativeIndir()) { - assert(target == nullptr); assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); assert(call->gtControlExpr == nullptr); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index ffd72c5fd2ba2f..6449caebff4516 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -5405,6 +5405,21 @@ void CodeGen::genCall(GenTreeCall* call) } #endif // defined(TARGET_X86) || defined(UNIX_AMD64_ABI) + GenTree* target = getCallTarget(call, nullptr); + + if (target != nullptr) + { + if (target->isContainedIndir()) + { + genConsumeAddress(target->AsIndir()->Addr()); + } + else + { + assert(!target->isContained()); + genConsumeReg(target); + } + } + // Insert a null check on "this" pointer if asked. if (call->NeedsNullCheck()) { @@ -5412,26 +5427,12 @@ void CodeGen::genCall(GenTreeCall* call) GetEmitter()->emitIns_AR_R(INS_cmp, EA_4BYTE, regThis, regThis, 0); } - // If fast tail call, then we are done here, we just have to load the call - // target into the right registers. We ensure in RA that the registers used - // for the target (e.g. contained indir) are loaded into volatile registers - // that won't be restored by epilog sequence. + // If fast tail call, then we are done here as we loaded the call target + // above. We ensure in RA that the registers used for the target (e.g. + // contained indir) are loaded into volatile registers that won't be + // restored by epilog sequence. if (call->IsFastTailCall()) { - GenTree* target = getCallTarget(call, nullptr); - if (target != nullptr) - { - if (target->isContainedIndir()) - { - genConsumeAddress(target->AsIndir()->Addr()); - } - else - { - assert(!target->isContained()); - genConsumeReg(target); - } - } - return; } @@ -5745,7 +5746,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA GenTree* addr = target->AsIndir()->Addr(); assert(addr->isUsedFromReg()); - genConsumeReg(addr); genCopyRegIfNeeded(addr, REG_VIRTUAL_STUB_TARGET); GetEmitter()->emitIns_Nop(3); @@ -5794,13 +5794,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA } else { - // For fast tailcalls this is happening in epilog, so we should - // have already consumed target in genCall. - if (!call->IsFastTailCall()) - { - genConsumeAddress(target->AsIndir()->Addr()); - } - // clang-format off genEmitCallIndir(emitter::EC_INDIR_ARD, methHnd, @@ -5820,13 +5813,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA // We just need to emit "call reg" in this case. assert(genIsValidIntReg(target->GetRegNum())); - // For fast tailcalls this is happening in epilog, so we should - // have already consumed target in genCall. - if (!call->IsFastTailCall()) - { - genConsumeReg(target); - } - // clang-format off genEmitCall(emitter::EC_INDIR_R, methHnd,