Skip to content

Commit

Permalink
Don't set GCState to 1 in PInvoke Epilog
Browse files Browse the repository at this point in the history
The comment indicated this was necessary to keep the FrameListRoot live, but the code generated by `CreateFrameLinkUpdate` will do that if needed.

Fix dotnet#38192
  • Loading branch information
CarolEidt committed Jun 23, 2020
1 parent 54625f3 commit 8c33791
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 16 deletions.
14 changes: 0 additions & 14 deletions src/coreclr/src/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 8c33791

Please sign in to comment.