diff --git a/src/coreclr/src/jit/liveness.cpp b/src/coreclr/src/jit/liveness.cpp index dc24bdffc2bab..158df93e68c6d 100644 --- a/src/coreclr/src/jit/liveness.cpp +++ b/src/coreclr/src/jit/liveness.cpp @@ -342,21 +342,21 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) fgCurMemoryDef |= memoryKindSet(GcHeap, ByrefExposed); fgCurMemoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); } - } - // If this is a p/invoke unmanaged call or if this is a tail-call + // If this is a p/invoke unmanaged call or if this is a tail-call via helper, // and we have an unmanaged p/invoke call in the method, // then we're going to run the p/invoke epilog. // So we mark the FrameRoot as used by this instruction. // This ensures that the block->bbVarUse will contain // the FrameRoot local var if is it a tracked variable. - if ((tree->AsCall()->IsUnmanaged() || tree->AsCall()->IsTailCall()) && compMethodRequiresPInvokeFrame()) + if ((tree->AsCall()->IsUnmanaged() || tree->AsCall()->IsTailCallViaJitHelper()) && + compMethodRequiresPInvokeFrame()) { assert((!opts.ShouldUsePInvokeHelpers()) || (info.compLvFrameListRoot == BAD_VAR_NUM)); if (!opts.ShouldUsePInvokeHelpers()) { - /* Get the TCB local and mark it as used */ + // Get the FrameRoot local and mark it as used. noway_assert(info.compLvFrameListRoot < lvaCount); @@ -373,6 +373,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) } break; + } default: @@ -504,7 +505,7 @@ void Compiler::fgPerBlockLocalVarLiveness() } } - /* Get the TCB local and mark it as used */ + // Mark the FrameListRoot as used, if applicable. if (block->bbJumpKind == BBJ_RETURN && compMethodRequiresPInvokeFrame()) { @@ -513,13 +514,22 @@ void Compiler::fgPerBlockLocalVarLiveness() { noway_assert(info.compLvFrameListRoot < lvaCount); - LclVarDsc* varDsc = &lvaTable[info.compLvFrameListRoot]; - - if (varDsc->lvTracked) + // 32-bit targets always pop the frame in the epilog. + // For 64-bit targets, we only do this in the epilog for IL stubs; + // for non-IL stubs the frame is popped after every PInvoke call. + CLANG_FORMAT_COMMENT_ANCHOR; +#ifdef TARGET_64BIT + if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB)) +#endif { - if (!VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex)) + LclVarDsc* varDsc = &lvaTable[info.compLvFrameListRoot]; + + if (varDsc->lvTracked) { - VarSetOps::AddElemD(this, fgCurUseSet, varDsc->lvVarIndex); + if (!VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex)) + { + VarSetOps::AddElemD(this, fgCurUseSet, varDsc->lvVarIndex); + } } } } @@ -1437,16 +1447,16 @@ void Compiler::fgComputeLifeCall(VARSET_TP& life, GenTreeCall* call) { assert(call != nullptr); - // If this is a tail-call and we have any unmanaged p/invoke calls in - // the method then we're going to run the p/invoke epilog + // If this is a tail-call via helper, and we have any unmanaged p/invoke calls in + // the method, then we're going to run the p/invoke epilog // So we mark the FrameRoot as used by this instruction. // This ensure that this variable is kept alive at the tail-call - if (call->IsTailCall() && compMethodRequiresPInvokeFrame()) + if (call->IsTailCallViaJitHelper() && compMethodRequiresPInvokeFrame()) { assert((!opts.ShouldUsePInvokeHelpers()) || (info.compLvFrameListRoot == BAD_VAR_NUM)); if (!opts.ShouldUsePInvokeHelpers()) { - /* Get the TCB local and make it live */ + // Get the FrameListRoot local and make it live. noway_assert(info.compLvFrameListRoot < lvaCount); @@ -1465,7 +1475,7 @@ void Compiler::fgComputeLifeCall(VARSET_TP& life, GenTreeCall* call) /* Is this call to unmanaged code? */ if (call->IsUnmanaged() && compMethodRequiresPInvokeFrame()) { - /* Get the TCB local and make it live */ + // Get the FrameListRoot local and make it live. assert((!opts.ShouldUsePInvokeHelpers()) || (info.compLvFrameListRoot == BAD_VAR_NUM)); if (!opts.ShouldUsePInvokeHelpers()) { diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index b0765b203b9f5..30d8ee4ed2a4b 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -4055,20 +4055,6 @@ void Lowering::InsertPInvokeMethodEpilog(BasicBlock* returnBB DEBUGARG(GenTree* // Example3: GT_JMP. After inserting PME execution order would be: PME, GT_JMP // That is after PME, args for GT_JMP call will be setup. - // TODO-Cleanup: setting GCState to 1 seems to be redundant as InsertPInvokeCallProlog will set it to zero before a - // PInvoke call and InsertPInvokeCallEpilog() will set it back to 1 after the PInvoke. Though this is redundant, - // it is harmeless. - // Note that liveness is artificially extending the life of compLvFrameListRoot var if the method being compiled has - // PInvokes. Deleting the below stmnt would cause an an assert in lsra.cpp::SetLastUses() since compLvFrameListRoot - // will be live-in to a BBJ_RETURN block without any uses. Long term we need to fix liveness for x64 case to - // properly extend the life of compLvFrameListRoot var. - // - // Thread.offsetOfGcState = 0/1 - // That is [tcb + offsetOfGcState] = 1 - GenTree* storeGCState = SetGCState(1); - returnBlockRange.InsertBefore(insertionPoint, LIR::SeqTree(comp, storeGCState)); - ContainCheckStoreIndir(storeGCState->AsIndir()); - // Pop the frame if necessary. This always happens in the epilog on 32-bit targets. For 64-bit targets, we only do // this in the epilog for IL stubs; for non-IL stubs the frame is popped after every PInvoke call. CLANG_FORMAT_COMMENT_ANCHOR; diff --git a/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs b/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs index 6eddc68345be7..24759d4be71ac 100644 --- a/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs +++ b/src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs @@ -42,8 +42,7 @@ public static int Main(string[] args) TestUnmanagedCallersOnlyValid(); TestUnmanagedCallersOnlyValid_OnNewNativeThread(); TestUnmanagedCallersOnlyValid_PrepareMethod(); - // Fails due to https://github.com/dotnet/runtime/issues/38192 - //TestUnmanagedCallersOnlyMultipleTimesValid(); + TestUnmanagedCallersOnlyMultipleTimesValid(); NegativeTest_NonStaticMethod(); NegativeTest_ViaDelegate(); NegativeTest_NonBlittable();