Conditionally remove the GC transition from a P/Invoke

Documentation/botr/clr-abi.md

@@ -85,7 +85,9 @@ ARM64-only: When a method returns a structure that is larger than 16 bytes the c
 
 # PInvokes
 
-The convention is that any method with an InlinedCallFrame (either an IL stub or a normal method with an inlined pinvoke) saves/restores all non-volatile integer registers in its prolog/epilog respectively. This is done so that the InlinedCallFrame can just contain a return address, a stack pointer and a frame pointer. Then using just those three it can start a full stack walk using the normal RtlVirtualUnwind.
+The convention is that any method with an InlinedCallFrame (either an IL stub or a normal method with an inlined PInvoke) saves/restores all non-volatile integer registers in its prolog/epilog respectively. This is done so that the InlinedCallFrame can just contain a return address, a stack pointer and a frame pointer. Then using just those three it can start a full stack walk using the normal RtlVirtualUnwind.
+
+When encountering a PInvoke, the JIT will query the VM if the GC transition should be suppressed. Suppression of the GC transition is indicated by the addition of an attribute on the PInvoke definition. If the VM indicates the GC transition is to be suppressed, the PInvoke frame will be omitted in either the IL stub or inlined scenario and a GC Poll will be inserted near the unmanaged call site. If an enclosing function contains more than one inlined PInvoke but not all have requested a suppression of the GC transition a PInvoke frame will still be constructed for the other inlined PInvokes. 
 
 For AMD64, a method with an InlinedCallFrame must use RBP as the frame register.
 
@@ -113,6 +115,8 @@ For IL stubs only, the per-frame initialization includes setting `Thread->m_pFra
 
 ## Per-call-site PInvoke work
 
+The below is performed when the GC transition is not suppressed.
+
 1. For direct calls, the JITed code sets `InlinedCallFrame->m_pDatum` to the MethodDesc of the call target.
     * For JIT64, indirect calls within IL stubs sets it to the secret parameter (this seems redundant, but it might have changed since the per-frame initialization?).
     * For JIT32 (ARM) indirect calls, it sets this member to the size of the pushed arguments, according to the comments. The implementation however always passed 0.

Documentation/botr/readytorun-format.md

@@ -205,7 +205,8 @@ fixup kind, the rest of the signature varies based on the fixup kind.
 | READYTORUN_FIXUP_Check_FieldOffset       |  0x2B | Verification of field offset, followed by field signature and expected field layout descriptor
 | READYTORUN_FIXUP_DelegateCtor            |  0x2C | Delegate constructor, followed by method signature
 | READYTORUN_FIXUP_DeclaringTypeHandle     |  0x2D | Dictionary lookup for method declaring type. Followed by the type signature.
-| READYTORUN_FIXUP_IndirectPInvokeTarget   |  0x2E | Target of an inlined PInvoke. Followed by method signature.
+| READYTORUN_FIXUP_IndirectPInvokeTarget   |  0x2E | Target (indirect) of an inlined PInvoke. Followed by method signature.
+| READYTORUN_FIXUP_PInvokeTarget           |  0x2F | Target of an inlined PInvoke. Followed by method signature.
 | READYTORUN_FIXUP_ModuleOverride          |  0x80 | When or-ed to the fixup ID, the fixup byte in the signature is followed by an encoded uint with assemblyref index, either within the MSIL metadata of the master context module for the signature or within the manifest metadata R2R header table (used in cases inlining brings in references to assemblies not seen in the input MSIL).
 
 #### Method Signatures

src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems

@@ -697,6 +697,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeHandle.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SEHException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\StructLayoutAttribute.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SuppressGCTransitionAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\TypeIdentifierAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\UnknownWrapper.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\UnmanagedFunctionPointerAttribute.cs" />

src/System.Private.CoreLib/shared/System/Runtime/InteropServices/SuppressGCTransitionAttribute.cs

@@ -0,0 +1,20 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Runtime.InteropServices
+{
+    /// <summary>
+    /// An attribute used to indicate a GC transition should be skipped when making an unmanaged function call.
+    /// </summary>
+    /// <remarks>
+    /// The eventual public attribute should replace this one: https://github.com/dotnet/corefx/issues/40740
+    /// </remarks>
+    [AttributeUsage(AttributeTargets.Method, Inherited = false)]
+    public sealed class SuppressGCTransitionAttribute : Attribute
+    {
+        public SuppressGCTransitionAttribute()
+        {
+        }
+    }
+}

src/debug/daccess/nidump.cpp

@@ -2194,6 +2194,10 @@ void NativeImageDumper::FixupBlobToString(RVA rva, SString& buf)
         buf.Append( W("Indirect P/Invoke target for ") );
         break;
 
+    case ENCODE_PINVOKE_TARGET:
+        buf.Append( W("P/Invoke target for ") );
+        break;
+
     case ENCODE_PROFILING_HANDLE:
         buf.Append( W("Profiling handle for ") );
         goto EncodeMethod;

src/inc/corcompile.h

@@ -688,7 +688,8 @@ enum CORCOMPILE_FIXUP_BLOB_KIND
 
     ENCODE_DECLARINGTYPE_HANDLE,
 
-    ENCODE_INDIRECT_PINVOKE_TARGET,                 /* For calling a pinvoke method ptr  */
+    ENCODE_INDIRECT_PINVOKE_TARGET,                 /* For calling a pinvoke method ptr indirectly */
+    ENCODE_PINVOKE_TARGET,                          /* For calling a pinvoke method ptr */
 
     ENCODE_MODULE_HANDLE                = 0x50,     /* Module token */
     ENCODE_STATIC_FIELD_ADDRESS,                    /* For accessing a static field */
@@ -1180,6 +1181,8 @@ class ICorCompilePreloader
 
     virtual BOOL IsUncompiledMethod(CORINFO_METHOD_HANDLE handle) = 0;
 
+    virtual BOOL ShouldSuppressGCTransition(CORINFO_METHOD_HANDLE handle) = 0;
+
     // Return a method handle that was previously registered and
     // hasn't been compiled already, and remove it from the set
     // of uncompiled methods.

src/inc/corinfo.h

@@ -836,6 +836,7 @@ enum CorInfoFlag
     CORINFO_FLG_AGGRESSIVE_OPT        = 0x01000000, // The method may contain hot code and should be aggressively optimized if possible
     CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS = 0x02000000, // Indicates that tier 0 JIT should not be used for a method that contains a loop
     CORINFO_FLG_NOSECURITYWRAP        = 0x04000000, // The method requires no security checks
+//  CORINFO_FLG_UNUSED                = 0x08000000,
     CORINFO_FLG_DONT_INLINE           = 0x10000000, // The method should not be inlined
     CORINFO_FLG_DONT_INLINE_CALLER    = 0x20000000, // The method should not be inlined, nor should its callers. It cannot be tail called.
     CORINFO_FLG_JIT_INTRINSIC         = 0x40000000, // Method is a potential jit intrinsic; verify identity by name check
@@ -1188,8 +1189,9 @@ enum CorInfoContextFlags
 
 enum CorInfoSigInfoFlags
 {
-    CORINFO_SIGFLAG_IS_LOCAL_SIG = 0x01,
-    CORINFO_SIGFLAG_IL_STUB      = 0x02,
+    CORINFO_SIGFLAG_IS_LOCAL_SIG           = 0x01,
+    CORINFO_SIGFLAG_IL_STUB                = 0x02,
+    CORINFO_SIGFLAG_SUPPRESS_GC_TRANSITION = 0x04,
 };
 
 struct CORINFO_SIG_INST

src/inc/readytorun.h

@@ -185,7 +185,8 @@ enum ReadyToRunFixupKind
     READYTORUN_FIXUP_DelegateCtor               = 0x2C, /* optimized delegate ctor */
     READYTORUN_FIXUP_DeclaringTypeHandle        = 0x2D,
 
-    READYTORUN_FIXUP_IndirectPInvokeTarget      = 0x2E, /* Target of an inlined pinvoke */
+    READYTORUN_FIXUP_IndirectPInvokeTarget      = 0x2E, /* Target (indirect) of an inlined pinvoke */
+    READYTORUN_FIXUP_PInvokeTarget              = 0x2F, /* Target of an inlined pinvoke */
 };
 
 //
@@ -239,6 +240,7 @@ enum ReadyToRunHelper
     // PInvoke helpers
     READYTORUN_HELPER_PInvokeBegin              = 0x42,
     READYTORUN_HELPER_PInvokeEnd                = 0x43,
+    READYTORUN_HELPER_GCPoll                    = 0x44,
 
     // Get string handle lazily
     READYTORUN_HELPER_GetString                 = 0x50,

src/inc/readytorunhelpers.h

@@ -113,6 +113,7 @@ HELPER(READYTORUN_HELPER_EndCatch,                  CORINFO_HELP_ENDCATCH,
 
 HELPER(READYTORUN_HELPER_PInvokeBegin,              CORINFO_HELP_JIT_PINVOKE_BEGIN,                 )
 HELPER(READYTORUN_HELPER_PInvokeEnd,                CORINFO_HELP_JIT_PINVOKE_END,                   )
+HELPER(READYTORUN_HELPER_GCPoll,                    CORINFO_HELP_POLL_GC,                           )
 
 HELPER(READYTORUN_HELPER_MonitorEnter,              CORINFO_HELP_MON_ENTER,                         )
 HELPER(READYTORUN_HELPER_MonitorExit,               CORINFO_HELP_MON_EXIT,                          )

src/jit/codegencommon.cpp

@@ -6862,7 +6862,7 @@ void CodeGen::genFinalizeFrame()
     }
 
     /* If we have any pinvoke calls, we might potentially trash everything */
-    if (compiler->info.compCallUnmanaged)
+    if (compiler->compMethodRequiresPInvokeFrame())
     {
         noway_assert(isFramePointerUsed()); // Setup of Pinvoke frame currently requires an EBP style frame
         regSet.rsSetRegsModified(RBM_INT_CALLEE_SAVED & ~RBM_FPBASE);
@@ -7339,7 +7339,7 @@ void CodeGen::genFnProlog()
 
     // We should not use the special PINVOKE registers as the initReg
     // since they are trashed by the jithelper call to setup the PINVOKE frame
-    if (compiler->info.compCallUnmanaged)
+    if (compiler->compMethodRequiresPInvokeFrame())
     {
         excludeMask |= RBM_PINVOKE_FRAME;
 
@@ -7402,7 +7402,7 @@ void CodeGen::genFnProlog()
         }
     }
 
-    noway_assert(!compiler->info.compCallUnmanaged || (initReg != REG_PINVOKE_FRAME));
+    noway_assert(!compiler->compMethodRequiresPInvokeFrame() || (initReg != REG_PINVOKE_FRAME));
 
 #if defined(_TARGET_AMD64_)
     // If we are a varargs call, in order to set up the arguments correctly this

src/jit/compiler.cpp

@@ -5914,8 +5914,8 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE            classPtr,
     }
     info.compRetNativeType = info.compRetType = JITtype2varType(methodInfo->args.retType);
 
-    info.compCallUnmanaged   = 0;
-    info.compLvFrameListRoot = BAD_VAR_NUM;
+    info.compUnmanagedCallCountWithGCTransition = 0;
+    info.compLvFrameListRoot                    = BAD_VAR_NUM;
 
     info.compInitMem = ((methodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0);
 

src/jit/compiler.h

@@ -8790,12 +8790,13 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         UNATIVE_OFFSET compTotalHotCodeSize;  // Total number of bytes of Hot Code in the method
         UNATIVE_OFFSET compTotalColdCodeSize; // Total number of bytes of Cold Code in the method
 
-        unsigned compCallUnmanaged;   // count of unmanaged calls
+        unsigned compUnmanagedCallCountWithGCTransition; // count of unmanaged calls with GC transition.
+
         unsigned compLvFrameListRoot; // lclNum for the Frame root
         unsigned compXcptnsCount;     // Number of exception-handling clauses read in the method's IL.
                                       // You should generally use compHndBBtabCount instead: it is the
                                       // current number of EH clauses (after additions like synchronized
-                                      // methods and funclets, and removals like unreachable code deletion).
+        // methods and funclets, and removals like unreachable code deletion).
 
         bool compMatchedVM; // true if the VM is "matched": either the JIT is a cross-compiler
                             // and the VM expects that, or the JIT is a "self-host" compiler
@@ -8896,6 +8897,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
                compMethodReturnsMultiRegRetType();
     }
 
+    // Returns true if the method requires a PInvoke prolog and epilog
+    bool compMethodRequiresPInvokeFrame()
+    {
+        return (info.compUnmanagedCallCountWithGCTransition > 0);
+    }
+
 #if defined(DEBUG)
 
     void compDispLocalVars();

src/jit/compiler.hpp

@@ -3681,7 +3681,15 @@ inline bool Compiler::IsGcSafePoint(GenTree* tree)
         GenTreeCall* call = tree->AsCall();
         if (!call->IsFastTailCall())
         {
-            if (call->gtCallType == CT_INDIRECT)
+            if (call->IsUnmanaged() && call->IsSuppressGCTransition())
+            {
+                // Both an indirect and user calls can be unmanaged
+                // and have a request to suppress the GC transition so
+                // the check is done prior to the separate handling of
+                // indirect and user calls.
+                return false;
+            }
+            else if (call->gtCallType == CT_INDIRECT)
             {
                 return true;
             }

src/jit/flowgraph.cpp

@@ -3860,7 +3860,6 @@ void Compiler::fgCreateGCPolls()
 
 bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block)
 {
-    assert(!(block->bbFlags & BBF_GC_SAFE_POINT));
     bool createdPollBlocks;
 
     void* addrTrap;
@@ -3878,19 +3877,26 @@ bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block)
     }
 #endif // ENABLE_FAST_GCPOLL_HELPER
 
+    // If the trap and address of thread global are null, make the call.
+    if (addrTrap == nullptr && pAddrOfCaptureThreadGlobal == nullptr)
+    {
+        pollType = GCPOLL_CALL;
+    }
+
     if (GCPOLL_CALL == pollType)
     {
         createdPollBlocks = false;
         GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_POLL_GC, TYP_VOID);
+        GenTree*     temp = fgMorphCall(call);
 
         // for BBJ_ALWAYS I don't need to insert it before the condition.  Just append it.
         if (block->bbJumpKind == BBJ_ALWAYS)
         {
-            fgNewStmtAtEnd(block, call);
+            fgNewStmtAtEnd(block, temp);
         }
         else
         {
-            Statement* newStmt = fgNewStmtNearEnd(block, call);
+            Statement* newStmt = fgNewStmtNearEnd(block, temp);
             // For DDB156656, we need to associate the GC Poll with the IL offset (and therefore sequence
             // point) of the tree before which we inserted the poll.  One example of when this is a
             // problem:
@@ -4026,6 +4032,10 @@ bool Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block)
         //      jumps, 2 for conditional branches, N for switches).
         switch (oldJumpKind)
         {
+            case BBJ_NONE:
+                // nothing to update. This can happen when inserting a GC Poll
+                // when suppressing a GC transition during an unmanaged call.
+                break;
             case BBJ_RETURN:
                 // no successors
                 break;
@@ -8901,7 +8911,7 @@ void Compiler::fgAddInternal()
 
     // The backend requires a scratch BB into which it can safely insert a P/Invoke method prolog if one is
     // required. Create it here.
-    if (info.compCallUnmanaged != 0)
+    if (compMethodRequiresPInvokeFrame())
     {
         fgEnsureFirstBBisScratch();
         fgFirstBB->bbFlags |= BBF_DONT_REMOVE;
@@ -9000,7 +9010,7 @@ void Compiler::fgAddInternal()
     //   or for synchronized methods.
     //
     BasicBlock* lastBlockBeforeGenReturns = fgLastBB;
-    if (compIsProfilerHookNeeded() || (info.compCallUnmanaged != 0) || opts.IsReversePInvoke() ||
+    if (compIsProfilerHookNeeded() || compMethodRequiresPInvokeFrame() || opts.IsReversePInvoke() ||
         ((info.compFlags & CORINFO_FLG_SYNCH) != 0))
     {
         // We will generate only one return block
@@ -9053,7 +9063,7 @@ void Compiler::fgAddInternal()
 
     merger.PlaceReturns();
 
-    if (info.compCallUnmanaged != 0)
+    if (compMethodRequiresPInvokeFrame())
     {
         // The P/Invoke helpers only require a frame variable, so only allocate the
         // TCB variable if we're not using them.
@@ -23188,8 +23198,8 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     }
 #endif // FEATURE_SIMD
 
-    // Update unmanaged call count
-    info.compCallUnmanaged += InlineeCompiler->info.compCallUnmanaged;
+    // Update unmanaged call details
+    info.compUnmanagedCallCountWithGCTransition += InlineeCompiler->info.compUnmanagedCallCountWithGCTransition;
 
 // Update optMethodFlags