From 4a90d512be37b7e5e785d1178482cfe49b79ba7c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 16:26:07 +0200 Subject: [PATCH 01/26] Disable Comparer_get_Default test on win-arm64-crossgen --- src/tests/issues.targets | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 1ceab90a73d64d..3db369bcb95ab5 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -673,6 +673,13 @@ https://github.com/dotnet/runtime/issues/85663 + + + + + https://github.com/dotnet/runtime/issues/104927 + + From 76c6f4d2383eb2d4765c192a4c9da48acac0d74f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 18:51:41 +0200 Subject: [PATCH 02/26] Keep 'this' alive for delegate invoke --- src/coreclr/jit/lower.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index d57d1b893d68af..b5931940e5afb2 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5551,6 +5551,11 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); + // Keep 'this' alive + GenTree* thisClone = comp->gtCloneExpr(base); + GenTree* keepAlive = comp->gtNewKeepAliveNode(thisClone); + BlockRange().InsertAfter(call, thisClone, keepAlive); + ContainCheckIndir(newThis->AsIndir()); // the control target is From 49b10a67c218a8a54e8efd0fb9b1f730a4bb2058 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 18 Jul 2024 18:54:58 +0200 Subject: [PATCH 03/26] Update issues.targets --- src/tests/issues.targets | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 3db369bcb95ab5..1ceab90a73d64d 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -673,13 +673,6 @@ https://github.com/dotnet/runtime/issues/85663 - - - - - https://github.com/dotnet/runtime/issues/104927 - - From 8bf8365cd9aa2a909d5687f79b4cde891a53cdaf Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 18 Jul 2024 19:14:17 +0200 Subject: [PATCH 04/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index b5931940e5afb2..57e0f33500d6f4 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5551,10 +5551,10 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); - // Keep 'this' alive - GenTree* thisClone = comp->gtCloneExpr(base); - GenTree* keepAlive = comp->gtNewKeepAliveNode(thisClone); - BlockRange().InsertAfter(call, thisClone, keepAlive); + // Keep delegate alive + GenTree* baseClone = comp->gtCloneExpr(base); + GenTree* keepBaseAlive = comp->gtNewKeepAliveNode(baseClone); + BlockRange().InsertAfter(call, baseClone, keepBaseAlive); ContainCheckIndir(newThis->AsIndir()); From 96543b327c0bc9b9e2fe88228c08f4be5ad39d82 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 22:07:32 +0200 Subject: [PATCH 05/26] less conservative version --- src/coreclr/jit/lower.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 57e0f33500d6f4..db140d5652154b 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5551,10 +5551,16 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); - // Keep delegate alive - GenTree* baseClone = comp->gtCloneExpr(base); - GenTree* keepBaseAlive = comp->gtNewKeepAliveNode(baseClone); - BlockRange().InsertAfter(call, baseClone, keepBaseAlive); +#if defined(TARGET_XARCH) + // Keep delegate alive if the target can't do an indirect call with an immediate operand + // and only in fully interruptible code. + if (comp->GetInterruptible()) + { + GenTree* baseClone = comp->gtCloneExpr(base); + GenTree* keepBaseAlive = comp->gtNewKeepAliveNode(baseClone); + BlockRange().InsertBefore(call, baseClone, keepBaseAlive); + } +#endif ContainCheckIndir(newThis->AsIndir()); From 5db07f80c5d68ed6c5c96e1bc9842ea9c27be4fd Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 22:09:13 +0200 Subject: [PATCH 06/26] fix define --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index db140d5652154b..b48a1727065eb8 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5551,7 +5551,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); -#if defined(TARGET_XARCH) +#if !defined(TARGET_XARCH) // Keep delegate alive if the target can't do an indirect call with an immediate operand // and only in fully interruptible code. if (comp->GetInterruptible()) From 118c8ad3ef0d3ae16891786fa511d22e13103d8a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 19 Jul 2024 01:15:57 +0200 Subject: [PATCH 07/26] Apply Jakob's suggestion --- src/coreclr/jit/lower.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index b48a1727065eb8..cdfbf9acfa34be 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5552,13 +5552,15 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); #if !defined(TARGET_XARCH) - // Keep delegate alive if the target can't do an indirect call with an immediate operand - // and only in fully interruptible code. if (comp->GetInterruptible()) { - GenTree* baseClone = comp->gtCloneExpr(base); - GenTree* keepBaseAlive = comp->gtNewKeepAliveNode(baseClone); - BlockRange().InsertBefore(call, baseClone, keepBaseAlive); + // If the target's backend doesn't support indirect calls with immediate operands (contained) + // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. + // to keep the delegate object alive while we're obtaining the function pointer. + // Since the actual call is effectively a safe point, we don't need to insert any additional + // NOPs to prevent the GC starvation in single-block methods. + GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + BlockRange().InsertBefore(newThisAddr, startNonGCNode); } #endif From ffe5317c17f9f0f2a74e8ae82d5463acf24f9d47 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 19 Jul 2024 02:16:43 +0200 Subject: [PATCH 08/26] GT_STOP_NOGC --- src/coreclr/jit/codegenarmarch.cpp | 4 ++++ src/coreclr/jit/codegenloongarch64.cpp | 4 ++++ src/coreclr/jit/codegenriscv64.cpp | 4 ++++ src/coreclr/jit/codegenxarch.cpp | 4 ++++ src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/compiler.hpp | 1 + src/coreclr/jit/gentree.cpp | 3 +++ src/coreclr/jit/gtlist.h | 1 + src/coreclr/jit/liveness.cpp | 1 + src/coreclr/jit/lower.cpp | 19 +++++++++++++++---- src/coreclr/jit/lower.h | 2 +- src/coreclr/jit/lsraarm.cpp | 1 + src/coreclr/jit/lsraarm64.cpp | 1 + src/coreclr/jit/lsraloongarch64.cpp | 1 + src/coreclr/jit/lsrariscv64.cpp | 1 + src/coreclr/jit/lsraxarch.cpp | 1 + 16 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 16cad5618112b7..c4dbab9726d878 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -160,6 +160,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) GetEmitter()->emitDisableGC(); break; + case GT_STOP_NONGC: + GetEmitter()->emitEnableGC(); + break; + case GT_START_PREEMPTGC: // Kill callee saves GC registers, and create a label // so that information gets propagated to the emitter. diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index 3bb0a2927444c2..7ce8af071f46ac 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -4206,6 +4206,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) GetEmitter()->emitDisableGC(); break; + case GT_STOP_NONGC: + GetEmitter()->emitEnableGC(); + break; + case GT_START_PREEMPTGC: // Kill callee saves GC registers, and create a label // so that information gets propagated to the emitter. diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 611cdf3b2e3e79..4fc57510abb6ca 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -4285,6 +4285,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) GetEmitter()->emitDisableGC(); break; + case GT_STOP_NONGC: + GetEmitter()->emitEnableGC(); + break; + case GT_START_PREEMPTGC: // Kill callee saves GC registers, and create a label // so that information gets propagated to the emitter. diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index dc9dd7c8a249bf..43fed90159171b 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1873,6 +1873,10 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_START_NONGC: GetEmitter()->emitDisableGC(); break; + + case GT_STOP_NONGC: + GetEmitter()->emitEnableGC(); + break; #endif // !defined(JIT32_GCENCODER) case GT_START_PREEMPTGC: diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e7822dc591597b..9017abf333a683 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11708,6 +11708,7 @@ class GenTreeVisitor case GT_SETCC: case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: #if defined(FEATURE_EH_WINDOWS_X86) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index e4ab163452a164..1a407e7aa2687f 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4365,6 +4365,7 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_SETCC: case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: #if defined(FEATURE_EH_WINDOWS_X86) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 759dde9b584fb9..9dd7f28fbf6084 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -6475,6 +6475,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) case GT_SETCC: case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: #if defined(FEATURE_EH_WINDOWS_X86) @@ -10011,6 +10012,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) case GT_SETCC: case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: #if defined(FEATURE_EH_WINDOWS_X86) @@ -12158,6 +12160,7 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) case GT_NOP: case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: case GT_CATCH_ARG: diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index e1e1f909896276..6c7e7b3851deb8 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -275,6 +275,7 @@ GTNODE(SWITCH , GenTreeOp ,0,1,GTK_UNOP|GTK_NOVALUE) GTNODE(NO_OP , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE) // A NOP that cannot be deleted. GTNODE(START_NONGC , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTHIR) // Starts a new instruction group that will be non-gc interruptible. +GTNODE(STOP_NONGC , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTHIR) // Tries to end the non-gc interruptible instruction group GTNODE(START_PREEMPTGC , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTHIR) // Starts a new instruction group where preemptive GC is enabled. GTNODE(PROF_HOOK , GenTree ,0,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTHIR) // Profiler Enter/Leave/TailCall hook. diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index d0ce193114529b..de20afa7aaeaf3 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1438,6 +1438,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR case GT_SWITCH: case GT_RETFILT: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_START_PREEMPTGC: case GT_PROF_HOOK: #if defined(FEATURE_EH_WINDOWS_X86) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index cdfbf9acfa34be..c4321668bf75f5 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2691,6 +2691,7 @@ GenTree* Lowering::LowerCall(GenTree* node) // note that everything generated from this point might run AFTER the outgoing args are placed GenTree* controlExpr = nullptr; bool callWasExpandedEarly = false; + bool endNogcBeforeCall = false; // for x86, this is where we record ESP for checking later to make sure stack is balanced @@ -2699,7 +2700,7 @@ GenTree* Lowering::LowerCall(GenTree* node) // an indirect call. if (call->IsDelegateInvoke()) { - controlExpr = LowerDelegateInvoke(call); + controlExpr = LowerDelegateInvoke(call, &endNogcBeforeCall); } else { @@ -2807,6 +2808,13 @@ GenTree* Lowering::LowerCall(GenTree* node) } ContainCheckCallOperands(call); + + if (endNogcBeforeCall) + { + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + BlockRange().InsertBefore(call, stopNonGCNode); + } + JITDUMP("lowering call (after):\n"); DISPTREERANGE(BlockRange(), call); JITDUMP("\n"); @@ -5491,8 +5499,10 @@ GenTree* Lowering::LowerDirectCall(GenTreeCall* call) return result; } -GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) +GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call, bool* shouldEndNogcBeforeCall) { + assert(shouldEndNogcBeforeCall != nullptr); + *shouldEndNogcBeforeCall = false; noway_assert(call->gtCallType == CT_USER_FUNC); assert((comp->info.compCompHnd->getMethodAttribs(call->gtCallMethHnd) & @@ -5557,10 +5567,11 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. - // Since the actual call is effectively a safe point, we don't need to insert any additional - // NOPs to prevent the GC starvation in single-block methods. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); BlockRange().InsertBefore(newThisAddr, startNonGCNode); + + // We should try to end the non-GC region just before the actual call. + *shouldEndNogcBeforeCall = true; } #endif diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 81e337abdaf668..f143e0d6e65215 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -165,7 +165,7 @@ class Lowering final : public Phase #if !defined(WINDOWS_AMD64_ABI) GenTreeLclVar* SpillStructCallResult(GenTreeCall* call) const; #endif // WINDOWS_AMD64_ABI - GenTree* LowerDelegateInvoke(GenTreeCall* call); + GenTree* LowerDelegateInvoke(GenTreeCall* call, bool* shouldStopNongcBeforeCall); GenTree* LowerIndirectNonvirtCall(GenTreeCall* call); GenTree* LowerDirectCall(GenTreeCall* call); GenTree* LowerNonvirtPinvokeCall(GenTreeCall* call); diff --git a/src/coreclr/jit/lsraarm.cpp b/src/coreclr/jit/lsraarm.cpp index 815f0149aede11..5af90aab2f303c 100644 --- a/src/coreclr/jit/lsraarm.cpp +++ b/src/coreclr/jit/lsraarm.cpp @@ -413,6 +413,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: case GT_PROF_HOOK: srcCount = 0; assert(dstCount == 0); diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 5d6cf7f1945e4c..5aa5f2a461422c 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -659,6 +659,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: srcCount = 0; assert(dstCount == 0); break; diff --git a/src/coreclr/jit/lsraloongarch64.cpp b/src/coreclr/jit/lsraloongarch64.cpp index fb8c7759f2e56f..64240b13eb22c6 100644 --- a/src/coreclr/jit/lsraloongarch64.cpp +++ b/src/coreclr/jit/lsraloongarch64.cpp @@ -122,6 +122,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: srcCount = 0; assert(dstCount == 0); break; diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp index 8a95b5ebbdd17a..1df65c6909613a 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -123,6 +123,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: srcCount = 0; assert(dstCount == 0); break; diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index c8a0f7f9cbc0e2..9599af1c48c42e 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -130,6 +130,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_NO_OP: case GT_START_NONGC: + case GT_STOP_NONGC: srcCount = 0; assert(dstCount == 0); break; From bfc872f88c5e3a374f596ba7e32342fbb66e0668 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 19 Jul 2024 12:01:10 +0200 Subject: [PATCH 09/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c4321668bf75f5..fdde8bf470c668 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5568,7 +5568,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call, bool* shouldEndNogcBef // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - BlockRange().InsertBefore(newThisAddr, startNonGCNode); + BlockRange().InsertAfter(thisArgNode, startNonGCNode); // We should try to end the non-GC region just before the actual call. *shouldEndNogcBeforeCall = true; From 300ab49b9035ce32c1fc513bc33088ba7a6a34c5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 19 Jul 2024 12:20:14 +0200 Subject: [PATCH 10/26] Address feedback --- src/coreclr/jit/lower.cpp | 17 ++++------------- src/coreclr/jit/lower.h | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index fdde8bf470c668..250d4eb21fc898 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2700,7 +2700,7 @@ GenTree* Lowering::LowerCall(GenTree* node) // an indirect call. if (call->IsDelegateInvoke()) { - controlExpr = LowerDelegateInvoke(call, &endNogcBeforeCall); + controlExpr = LowerDelegateInvoke(call); } else { @@ -2809,12 +2809,6 @@ GenTree* Lowering::LowerCall(GenTree* node) ContainCheckCallOperands(call); - if (endNogcBeforeCall) - { - GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - BlockRange().InsertBefore(call, stopNonGCNode); - } - JITDUMP("lowering call (after):\n"); DISPTREERANGE(BlockRange(), call); JITDUMP("\n"); @@ -5499,10 +5493,8 @@ GenTree* Lowering::LowerDirectCall(GenTreeCall* call) return result; } -GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call, bool* shouldEndNogcBeforeCall) +GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) { - assert(shouldEndNogcBeforeCall != nullptr); - *shouldEndNogcBeforeCall = false; noway_assert(call->gtCallType == CT_USER_FUNC); assert((comp->info.compCompHnd->getMethodAttribs(call->gtCallMethHnd) & @@ -5568,10 +5560,9 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call, bool* shouldEndNogcBef // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); BlockRange().InsertAfter(thisArgNode, startNonGCNode); - - // We should try to end the non-GC region just before the actual call. - *shouldEndNogcBeforeCall = true; + BlockRange().InsertAfter(call, stopNonGCNode); } #endif diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index f143e0d6e65215..81e337abdaf668 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -165,7 +165,7 @@ class Lowering final : public Phase #if !defined(WINDOWS_AMD64_ABI) GenTreeLclVar* SpillStructCallResult(GenTreeCall* call) const; #endif // WINDOWS_AMD64_ABI - GenTree* LowerDelegateInvoke(GenTreeCall* call, bool* shouldStopNongcBeforeCall); + GenTree* LowerDelegateInvoke(GenTreeCall* call); GenTree* LowerIndirectNonvirtCall(GenTreeCall* call); GenTree* LowerDirectCall(GenTreeCall* call); GenTree* LowerNonvirtPinvokeCall(GenTreeCall* call); From 26da6ab89856e8ad622e490e73acb71ba044bb8f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 19 Jul 2024 12:21:10 +0200 Subject: [PATCH 11/26] Address feedback --- src/coreclr/jit/lower.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 250d4eb21fc898..0bcc4971c89fa1 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2691,7 +2691,6 @@ GenTree* Lowering::LowerCall(GenTree* node) // note that everything generated from this point might run AFTER the outgoing args are placed GenTree* controlExpr = nullptr; bool callWasExpandedEarly = false; - bool endNogcBeforeCall = false; // for x86, this is where we record ESP for checking later to make sure stack is balanced @@ -2808,7 +2807,6 @@ GenTree* Lowering::LowerCall(GenTree* node) } ContainCheckCallOperands(call); - JITDUMP("lowering call (after):\n"); DISPTREERANGE(BlockRange(), call); JITDUMP("\n"); From b35c8922b87d63d63034e75cd7bb61f2f71f3828 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 1 Aug 2024 04:00:27 +0200 Subject: [PATCH 12/26] handle tail calls --- src/coreclr/jit/error.cpp | 5 +++-- src/coreclr/jit/lower.cpp | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/error.cpp b/src/coreclr/jit/error.cpp index 5ae6cea056efeb..0fd5a5c59f9b90 100644 --- a/src/coreclr/jit/error.cpp +++ b/src/coreclr/jit/error.cpp @@ -268,9 +268,10 @@ extern "C" void __cdecl assertAbort(const char* why, const char* file, unsigned { phaseName = PhaseNames[env->compiler->mostRecentlyActivePhase]; _snprintf_s(buff, BUFF_SIZE, _TRUNCATE, - "Assertion failed '%s' in '%s' during '%s' (IL size %d; hash 0x%08x; %s)\n", why, + "Assertion failed '%s' in '%s' during '%s' (IL size %d; BBs: %d; hash 0x%08x; %s)\n", why, env->compiler->info.compFullName, phaseName, env->compiler->info.compILCodeSize, - env->compiler->info.compMethodHash(), env->compiler->compGetTieringName(/* short name */ true)); + env->compiler->fgBBcount, env->compiler->info.compMethodHash(), + env->compiler->compGetTieringName(/* short name */ true)); msg = buff; } printf(""); // null string means flush diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 56566dffe992bf..68c30bb978318f 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5559,7 +5559,12 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); BlockRange().InsertAfter(thisArgNode, startNonGCNode); - BlockRange().InsertAfter(call, stopNonGCNode); + if (!call->IsTailCall()) + { + // We don't have to insert the STOP_NONGC node for tail calls, as the call itself is + // a safe point and effectively the last node. + BlockRange().InsertAfter(call, stopNonGCNode); + } } #endif From ffdc8e5320f62fb87a07c6389077e9f9426bb5c8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 1 Aug 2024 21:32:38 +0200 Subject: [PATCH 13/26] remove bogus assert --- src/coreclr/jit/lower.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 68c30bb978318f..8066eea7750dbe 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -743,7 +743,6 @@ GenTree* Lowering::LowerArrLength(GenTreeArrCommon* node) // Create the expression `*(array_addr + lenOffset)` GenTree* addr; - noway_assert(arr->gtNext == node); if ((arr->gtOper == GT_CNS_INT) && (arr->AsIntCon()->gtIconVal == 0)) { From 1dac2156c8efb2d7561c05ecaa3677618df4faec Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 1 Aug 2024 23:00:04 +0200 Subject: [PATCH 14/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 1b05ba06fa9228..4415362bceab2e 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5555,8 +5555,8 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); -#if !defined(TARGET_XARCH) - if (comp->GetInterruptible()) + //#if !defined(TARGET_XARCH) + //if (comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. @@ -5571,7 +5571,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().InsertAfter(call, stopNonGCNode); } } -#endif + //#endif ContainCheckIndir(newThis->AsIndir()); From 7cb1d652db32cd61385a60c240e90e53fb7678eb Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 2 Aug 2024 01:31:34 +0200 Subject: [PATCH 15/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 4415362bceab2e..3893baa44545d0 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5564,12 +5564,12 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); BlockRange().InsertAfter(thisArgNode, startNonGCNode); - if (!call->IsTailCall()) + /*if (!call->IsTailCall()) { // We don't have to insert the STOP_NONGC node for tail calls, as the call itself is // a safe point and effectively the last node. BlockRange().InsertAfter(call, stopNonGCNode); - } + }*/ } //#endif From f0964fd86453f4a7daeee2c6279143e1c3f36651 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 2 Aug 2024 01:32:11 +0200 Subject: [PATCH 16/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 3893baa44545d0..b48ad991fc8430 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5555,8 +5555,8 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); - //#if !defined(TARGET_XARCH) - //if (comp->GetInterruptible()) +#if !defined(TARGET_XARCH) + if (comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. @@ -5571,7 +5571,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().InsertAfter(call, stopNonGCNode); }*/ } - //#endif +#endif ContainCheckIndir(newThis->AsIndir()); From 126fca994c39f00f28884239ef6c2972d3ddb890 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 2 Aug 2024 03:12:20 +0200 Subject: [PATCH 17/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index b48ad991fc8430..36c475f54d65a3 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5563,7 +5563,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) // to keep the delegate object alive while we're obtaining the function pointer. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - BlockRange().InsertAfter(thisArgNode, startNonGCNode); + BlockRange().InsertAfter(base, startNonGCNode); /*if (!call->IsTailCall()) { // We don't have to insert the STOP_NONGC node for tail calls, as the call itself is From 65d7909edd4055c70636ac99fcf2c27ba9416e55 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Oct 2024 02:41:21 +0200 Subject: [PATCH 18/26] test --- src/coreclr/jit/lower.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index e1838ac05dc351..2ba1252df631af 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5694,23 +5694,18 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); -#if !defined(TARGET_XARCH) +//#if !defined(TARGET_XARCH) if (comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. - GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - BlockRange().InsertAfter(base, startNonGCNode); - /*if (!call->IsTailCall()) - { - // We don't have to insert the STOP_NONGC node for tail calls, as the call itself is - // a safe point and effectively the last node. - BlockRange().InsertAfter(call, stopNonGCNode); - }*/ + //GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + //GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + //BlockRange().InsertAfter(base, startNonGCNode); + //BlockRange().InsertAfter(call, stopNonGCNode); } -#endif +//#endif ContainCheckIndir(newThis->AsIndir()); From dded5c94bbb2e65be24f858ad9123a3ff6f83b7b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Oct 2024 02:49:20 +0200 Subject: [PATCH 19/26] test2 --- src/coreclr/jit/lower.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 2ba1252df631af..4b4cb2fa25db6e 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5695,15 +5695,18 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); //#if !defined(TARGET_XARCH) - if (comp->GetInterruptible()) + //if (comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. - //GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - //GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - //BlockRange().InsertAfter(base, startNonGCNode); - //BlockRange().InsertAfter(call, stopNonGCNode); + GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + BlockRange().InsertAfter(thisArgNode, startNonGCNode); + if (!call->IsTailCall()) + { + BlockRange().InsertAfter(call, stopNonGCNode); + } } //#endif From dca303bd9b07feeb09a2230468b84b3c14d6b8c8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Oct 2024 03:25:33 +0200 Subject: [PATCH 20/26] test --- src/coreclr/jit/lower.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 4b4cb2fa25db6e..c429a2187cddce 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5694,8 +5694,9 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); -//#if !defined(TARGET_XARCH) - //if (comp->GetInterruptible()) +#ifndef JIT32_GCENCODER + // #if !defined(TARGET_XARCH) + // if (comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. @@ -5708,7 +5709,7 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().InsertAfter(call, stopNonGCNode); } } -//#endif +#endif ContainCheckIndir(newThis->AsIndir()); From 95b6301e99ad14d5d2aadca064560266ec554eb2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Oct 2024 16:07:56 +0200 Subject: [PATCH 21/26] Clean up --- src/coreclr/jit/lower.cpp | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c429a2187cddce..c08395437ad7e3 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2916,6 +2916,23 @@ GenTree* Lowering::LowerCall(GenTree* node) ContainCheckRange(controlExprRange); BlockRange().InsertBefore(call, std::move(controlExprRange)); + +#ifndef JIT32_GCENCODER + if (call->IsDelegateInvoke()) + { + // If the target's backend doesn't support indirect calls with immediate operands (contained) + // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. + // to keep the delegate object alive while we're obtaining the function pointer. + GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + BlockRange().InsertBefore(controlExpr, startNonGCNode); + if (!call->IsTailCall()) + { + BlockRange().InsertAfter(call, stopNonGCNode); + } + } +#endif + call->gtControlExpr = controlExpr; } @@ -5694,22 +5711,6 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); -#ifndef JIT32_GCENCODER - // #if !defined(TARGET_XARCH) - // if (comp->GetInterruptible()) - { - // If the target's backend doesn't support indirect calls with immediate operands (contained) - // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. - // to keep the delegate object alive while we're obtaining the function pointer. - GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - BlockRange().InsertAfter(thisArgNode, startNonGCNode); - if (!call->IsTailCall()) - { - BlockRange().InsertAfter(call, stopNonGCNode); - } - } -#endif ContainCheckIndir(newThis->AsIndir()); From 237732212927844d315a0ae3c7e4fc67948fa486 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 10 Oct 2024 19:27:09 +0200 Subject: [PATCH 22/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c08395437ad7e3..161d9f9f767d07 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2928,7 +2928,7 @@ GenTree* Lowering::LowerCall(GenTree* node) BlockRange().InsertBefore(controlExpr, startNonGCNode); if (!call->IsTailCall()) { - BlockRange().InsertAfter(call, stopNonGCNode); + BlockRange().InsertBefore(call, stopNonGCNode); } } #endif From bef38f38fc293a69862691ce73054d990cbda71b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Oct 2024 22:15:15 +0200 Subject: [PATCH 23/26] clean up --- src/coreclr/jit/lower.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 161d9f9f767d07..7a07d47e03d547 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2917,18 +2917,19 @@ GenTree* Lowering::LowerCall(GenTree* node) BlockRange().InsertBefore(call, std::move(controlExprRange)); -#ifndef JIT32_GCENCODER - if (call->IsDelegateInvoke()) +#ifndef TARGET_XARCH + if (call->IsDelegateInvoke() && comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); BlockRange().InsertBefore(controlExpr, startNonGCNode); if (!call->IsTailCall()) { - BlockRange().InsertBefore(call, stopNonGCNode); + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + BlockRange().InsertAfter(call, stopNonGCNode); + BlockRange().InsertAfter(stopNonGCNode, new (comp, GT_NO_OP) GenTree(GT_NO_OP, TYP_VOID)); } } #endif @@ -5711,7 +5712,6 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) BlockRange().Remove(thisArgNode); BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); - ContainCheckIndir(newThis->AsIndir()); // the control target is From 9fba5a3ce5b74b6591b7183c8ac4fe484d72689d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 10 Oct 2024 23:14:35 +0200 Subject: [PATCH 24/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 7a07d47e03d547..5cf4d48a6147b6 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2917,7 +2917,7 @@ GenTree* Lowering::LowerCall(GenTree* node) BlockRange().InsertBefore(call, std::move(controlExprRange)); -#ifndef TARGET_XARCH +#ifndef TARGET_X86 if (call->IsDelegateInvoke() && comp->GetInterruptible()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) From 9ccc937778ce495a3835906d3ef586845778d04f Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 11 Oct 2024 02:28:29 +0200 Subject: [PATCH 25/26] Update lower.cpp --- src/coreclr/jit/lower.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 5cf4d48a6147b6..daae7eae85da19 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2918,14 +2918,15 @@ GenTree* Lowering::LowerCall(GenTree* node) BlockRange().InsertBefore(call, std::move(controlExprRange)); #ifndef TARGET_X86 - if (call->IsDelegateInvoke() && comp->GetInterruptible()) + if (call->IsDelegateInvoke() && comp->GetInterruptible() && !call->IsTailCall()) { // If the target's backend doesn't support indirect calls with immediate operands (contained) // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. // to keep the delegate object alive while we're obtaining the function pointer. GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); BlockRange().InsertBefore(controlExpr, startNonGCNode); - if (!call->IsTailCall()) + BlockRange().InsertBefore(startNonGCNode, new (comp, GT_NO_OP) GenTree(GT_NO_OP, TYP_VOID)); + //if (!call->IsTailCall()) { GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); BlockRange().InsertAfter(call, stopNonGCNode); From ef49ee686ea5ef9c5263cc9d0ef25a842f343e4d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 11 Oct 2024 17:00:20 +0200 Subject: [PATCH 26/26] test --- src/coreclr/jit/lower.cpp | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index daae7eae85da19..2b6878298ff105 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2917,24 +2917,6 @@ GenTree* Lowering::LowerCall(GenTree* node) BlockRange().InsertBefore(call, std::move(controlExprRange)); -#ifndef TARGET_X86 - if (call->IsDelegateInvoke() && comp->GetInterruptible() && !call->IsTailCall()) - { - // If the target's backend doesn't support indirect calls with immediate operands (contained) - // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. - // to keep the delegate object alive while we're obtaining the function pointer. - GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); - BlockRange().InsertBefore(controlExpr, startNonGCNode); - BlockRange().InsertBefore(startNonGCNode, new (comp, GT_NO_OP) GenTree(GT_NO_OP, TYP_VOID)); - //if (!call->IsTailCall()) - { - GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); - BlockRange().InsertAfter(call, stopNonGCNode); - BlockRange().InsertAfter(stopNonGCNode, new (comp, GT_NO_OP) GenTree(GT_NO_OP, TYP_VOID)); - } - } -#endif - call->gtControlExpr = controlExpr; } @@ -2968,6 +2950,22 @@ GenTree* Lowering::LowerCall(GenTree* node) } ContainCheckCallOperands(call); + +#ifndef TARGET_X86 + if (call->IsDelegateInvoke() && !call->IsTailCall()) + { + // If the target's backend doesn't support indirect calls with immediate operands (contained) + // and the method is marked as interruptible, we need to insert a GT_START_NONGC before the call. + // to keep the delegate object alive while we're obtaining the function pointer. + GenTree* startNonGCNode = new (comp, GT_START_NONGC) GenTree(GT_START_NONGC, TYP_VOID); + BlockRange().InsertBefore(controlExpr, startNonGCNode); + + GenTree* stopNonGCNode = new (comp, GT_STOP_NONGC) GenTree(GT_STOP_NONGC, TYP_VOID); + BlockRange().InsertAfter(call, stopNonGCNode); + BlockRange().InsertAfter(stopNonGCNode, new (comp, GT_NO_OP) GenTree(GT_NO_OP, TYP_VOID)); + } +#endif + JITDUMP("lowering call (after):\n"); DISPTREERANGE(BlockRange(), call); JITDUMP("\n");