From cbb1e455f070b8974f19810162aab6fa46358bb5 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 21 May 2020 07:27:05 -0700 Subject: [PATCH 1/9] Optimize virtual call stub for R2R ARM/ARM64 scenarios --- src/coreclr/src/jit/codegenarmarch.cpp | 5 +++-- src/coreclr/src/jit/lower.cpp | 4 ++++ src/coreclr/src/jit/lsraarmarch.cpp | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index cacfb1a6182db2..120b7f04ab0b29 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2521,11 +2521,12 @@ void CodeGen::genCallInstruction(GenTreeCall* call) retSize MULTIREG_HAS_SECOND_GC_RET_ONLY_ARG(secondRetSize), ilOffset, target->GetRegNum()); } #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir()) + else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) { // Generate a direct call to a non-virtual user defined or helper method assert(callType == CT_HELPER || callType == CT_USER_FUNC); - assert(call->gtEntryPoint.accessType == IAT_PVALUE); + assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || + ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); assert(call->gtControlExpr == nullptr); regNumber tmpReg = call->GetSingleTempReg(); diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 2c093a67d048bb..13639919587591 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -4529,7 +4529,11 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) } else { +#if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) + result = nullptr; +#else result = Ind(addr); +#endif // defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) } } diff --git a/src/coreclr/src/jit/lsraarmarch.cpp b/src/coreclr/src/jit/lsraarmarch.cpp index 64d18bf6d80bde..575d9a227beb9f 100644 --- a/src/coreclr/src/jit/lsraarmarch.cpp +++ b/src/coreclr/src/jit/lsraarmarch.cpp @@ -183,7 +183,7 @@ int LinearScan::BuildCall(GenTreeCall* call) } } #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir()) + else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) { buildInternalIntRegisterDefForNode(call); } From 424dd1cbc8683a696390ca92cbca708cda964a67 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 21 May 2020 16:09:41 -0700 Subject: [PATCH 2/9] Only optimize when EntryPointAdd is not null --- src/coreclr/src/jit/codegenarmarch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index 120b7f04ab0b29..73e8f0708f9a21 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2521,7 +2521,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) retSize MULTIREG_HAS_SECOND_GC_RET_ONLY_ARG(secondRetSize), ilOffset, target->GetRegNum()); } #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) + else if (call->IsR2RRelativeIndir() || (call->IsVirtualStubRelativeIndir() && call->gtEntryPoint.addr != NULL)) { // Generate a direct call to a non-virtual user defined or helper method assert(callType == CT_HELPER || callType == CT_USER_FUNC); From c427fdb8f82a78aea6cb29073926528231286164 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 21 May 2020 15:25:35 -0700 Subject: [PATCH 3/9] logging --- src/coreclr/src/jit/codegenarmarch.cpp | 4 ++++ src/coreclr/src/jit/lower.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index 73e8f0708f9a21..fe6d73081c3fc5 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2326,6 +2326,10 @@ void CodeGen::genCodeForInitBlkHelper(GenTreeBlk* initBlkNode) // void CodeGen::genCallInstruction(GenTreeCall* call) { + if (this->compiler->compMethodID == 749) + { + printf("genCallInstruction : %s\n", this->compiler->info.compMethodName); + } gtCallTypes callType = (gtCallTypes)call->gtCallType; IL_OFFSETX ilOffset = BAD_IL_OFFSET; diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 13639919587591..63509da3e25d48 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -1518,6 +1518,10 @@ GenTree* Lowering::AddrGen(void* addr) // void Lowering::LowerCall(GenTree* node) { + if (this->comp->compMethodID == 749) + { + printf("genCallInstruction : %s\n", this->comp->info.compMethodName); + } GenTreeCall* call = node->AsCall(); JITDUMP("lowering call (before):\n"); From 3af72ea47cc7598ba388963545788c5f0eb4bb7a Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 21 May 2020 16:15:52 -0700 Subject: [PATCH 4/9] Revert "Only optimize when EntryPointAdd is not null" This reverts commit 441d866609b406c57ed586a79f5ed27e99c31194. --- src/coreclr/src/jit/codegenarmarch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index fe6d73081c3fc5..fd86ac9f93e7a1 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2525,7 +2525,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) retSize MULTIREG_HAS_SECOND_GC_RET_ONLY_ARG(secondRetSize), ilOffset, target->GetRegNum()); } #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir() || (call->IsVirtualStubRelativeIndir() && call->gtEntryPoint.addr != NULL)) + else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) { // Generate a direct call to a non-virtual user defined or helper method assert(callType == CT_HELPER || callType == CT_USER_FUNC); From 5d2ba71715f63455e1b61364c18da917a3dae66d Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 21 May 2020 23:59:31 -0700 Subject: [PATCH 5/9] Include check for GTF_CALL_VIRT_STUB --- src/coreclr/src/jit/codegenarmarch.cpp | 4 +--- src/coreclr/src/jit/gentree.h | 10 ++++++++++ src/coreclr/src/jit/lsraarmarch.cpp | 4 +--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index fd86ac9f93e7a1..44902e1db09dc9 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2524,8 +2524,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) INDEBUG_LDISASM_COMMA(sigInfo) nullptr, // addr retSize MULTIREG_HAS_SECOND_GC_RET_ONLY_ARG(secondRetSize), ilOffset, target->GetRegNum()); } -#if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) + else if (call->IsR2ROrVirtualStubRelativeIndir()) { // Generate a direct call to a non-virtual user defined or helper method assert(callType == CT_HELPER || callType == CT_USER_FUNC); @@ -2545,7 +2544,6 @@ void CodeGen::genCallInstruction(GenTreeCall* call) INDEBUG_LDISASM_COMMA(sigInfo) nullptr, // addr retSize MULTIREG_HAS_SECOND_GC_RET_ONLY_ARG(secondRetSize), ilOffset, tmpReg); } -#endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH else { // Generate a direct call to a non-virtual user defined or helper method diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index c43227bd663111..31880288cfd624 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -4202,6 +4202,16 @@ struct GenTreeCall final : public GenTree return (gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0; } + bool IsR2ROrVirtualStubRelativeIndir() + { +#if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) + return (IsR2RRelativeIndir() || + ((gtFlags & GTF_CALL_VIRT_KIND_MASK) == GTF_CALL_VIRT_STUB) && (IsVirtualStubRelativeIndir())); +#else + return false; +#endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH + } + bool HasNonStandardAddedArgs(Compiler* compiler) const; int GetNonStandardAddedArgCount(Compiler* compiler) const; diff --git a/src/coreclr/src/jit/lsraarmarch.cpp b/src/coreclr/src/jit/lsraarmarch.cpp index 575d9a227beb9f..bcc987ca62e132 100644 --- a/src/coreclr/src/jit/lsraarmarch.cpp +++ b/src/coreclr/src/jit/lsraarmarch.cpp @@ -182,12 +182,10 @@ int LinearScan::BuildCall(GenTreeCall* call) ctrlExprCandidates = RBM_FASTTAILCALL_TARGET; } } -#if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - else if (call->IsR2RRelativeIndir() || call->IsVirtualStubRelativeIndir()) + else if (call->IsR2ROrVirtualStubRelativeIndir()) { buildInternalIntRegisterDefForNode(call); } -#endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH #ifdef TARGET_ARM else { From 3d301832c7763dadbae69dea92154f5645ef8e6f Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 22 May 2020 00:00:29 -0700 Subject: [PATCH 6/9] Revert "logging" This reverts commit 0183e76e83d5ba1057b95ba37204cc525e295bf5. --- src/coreclr/src/jit/codegenarmarch.cpp | 4 ---- src/coreclr/src/jit/lower.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index 44902e1db09dc9..6408057505f501 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2326,10 +2326,6 @@ void CodeGen::genCodeForInitBlkHelper(GenTreeBlk* initBlkNode) // void CodeGen::genCallInstruction(GenTreeCall* call) { - if (this->compiler->compMethodID == 749) - { - printf("genCallInstruction : %s\n", this->compiler->info.compMethodName); - } gtCallTypes callType = (gtCallTypes)call->gtCallType; IL_OFFSETX ilOffset = BAD_IL_OFFSET; diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 63509da3e25d48..13639919587591 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -1518,10 +1518,6 @@ GenTree* Lowering::AddrGen(void* addr) // void Lowering::LowerCall(GenTree* node) { - if (this->comp->compMethodID == 749) - { - printf("genCallInstruction : %s\n", this->comp->info.compMethodName); - } GenTreeCall* call = node->AsCall(); JITDUMP("lowering call (before):\n"); From ead40bb635f81a69412829c23138269002b955a3 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 22 May 2020 00:41:28 -0700 Subject: [PATCH 7/9] fix the conditions --- src/coreclr/src/jit/gentree.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index 31880288cfd624..ec0cbee87192e4 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -4205,8 +4205,8 @@ struct GenTreeCall final : public GenTree bool IsR2ROrVirtualStubRelativeIndir() { #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - return (IsR2RRelativeIndir() || - ((gtFlags & GTF_CALL_VIRT_KIND_MASK) == GTF_CALL_VIRT_STUB) && (IsVirtualStubRelativeIndir())); + bool isVirtualStub = (gtFlags & GTF_CALL_VIRT_KIND_MASK) == GTF_CALL_VIRT_STUB; + return ((IsR2RRelativeIndir()) || (isVirtualStub && (IsVirtualStubRelativeIndir()))); #else return false; #endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH From c82c2779be76cc864f9886c8cbbe8197d764e39f Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 22 May 2020 14:02:50 -0700 Subject: [PATCH 8/9] Should not optimize if TailCall --- src/coreclr/src/jit/codegenarmarch.cpp | 1 + src/coreclr/src/jit/lower.cpp | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/jit/codegenarmarch.cpp b/src/coreclr/src/jit/codegenarmarch.cpp index 6408057505f501..9ca28936f24bf9 100644 --- a/src/coreclr/src/jit/codegenarmarch.cpp +++ b/src/coreclr/src/jit/codegenarmarch.cpp @@ -2527,6 +2527,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) assert(((call->IsR2RRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_PVALUE)) || ((call->IsVirtualStubRelativeIndir()) && (call->gtEntryPoint.accessType == IAT_VALUE))); assert(call->gtControlExpr == nullptr); + assert(!call->IsTailCall()); regNumber tmpReg = call->GetSingleTempReg(); GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), tmpReg, REG_R2R_INDIRECT_PARAM); diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 13639919587591..553b7b07a263b1 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -4529,11 +4529,16 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) } else { + + bool shouldOptimizeVirtualStubCall = false; #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) - result = nullptr; -#else - result = Ind(addr); -#endif // defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) + shouldOptimizeVirtualStubCall = !call->IsTailCall(); +#endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH + + if (!shouldOptimizeVirtualStubCall) + { + result = Ind(addr); + } } } From e2d8d34d0cb06d67138463b020d81f072b02fdbe Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 27 May 2020 17:11:12 -0700 Subject: [PATCH 9/9] Added a comment --- src/coreclr/src/jit/lower.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 553b7b07a263b1..75d63a18661f7d 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -3489,6 +3489,9 @@ GenTree* Lowering::LowerDirectCall(GenTreeCall* call) { bool isR2RRelativeIndir = false; #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) + // Skip inserting the indirection node to load the address that is already + // computed in REG_R2R_INDIRECT_PARAM as a hidden parameter. Instead during the + // codegen, just load the call target from REG_R2R_INDIRECT_PARAM. isR2RRelativeIndir = call->IsR2RRelativeIndir(); #endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH @@ -4532,6 +4535,11 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) bool shouldOptimizeVirtualStubCall = false; #if defined(FEATURE_READYTORUN_COMPILER) && defined(TARGET_ARMARCH) + // Skip inserting the indirection node to load the address that is already + // computed in REG_R2R_INDIRECT_PARAM as a hidden parameter. Instead during the + // codegen, just load the call target from REG_R2R_INDIRECT_PARAM. + // However, for tail calls, the call target is always computed in RBM_FASTTAILCALL_TARGET + // and so do not optimize virtual stub calls for such cases. shouldOptimizeVirtualStubCall = !call->IsTailCall(); #endif // FEATURE_READYTORUN_COMPILER && TARGET_ARMARCH