Merge pull request dotnet/coreclr#8858 from sandreenko/CoreRT-PInvoke

Update PInvoke inlining restrictions for CoreRT

Commit migrated from dotnet/coreclr@01f8e7d

Author: sandreenko

src/coreclr/src/jit/compiler.cpp

@@ -3294,8 +3294,6 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
     }
 #endif
 
-    opts.compMustInlinePInvokeCalli = jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB);
-
     opts.compScopeInfo = opts.compDbgInfo;
 
 #ifdef LATE_DISASM

src/coreclr/src/jit/compiler.h

@@ -2785,7 +2785,7 @@ class Compiler
 
     void impImportNewObjArray(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_CALL_INFO* pCallInfo);
 
-    bool impCanPInvokeInline(BasicBlock* block);
+    bool impCanPInvokeInline();
     bool impCanPInvokeInlineCallSite(BasicBlock* block);
     void impCheckForPInvokeCall(
         GenTreePtr call, CORINFO_METHOD_HANDLE methHnd, CORINFO_SIG_INFO* sig, unsigned mflags, BasicBlock* block);
@@ -7625,8 +7625,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         static const bool compNoPInvokeInlineCB;
 #endif
 
-        bool compMustInlinePInvokeCalli; // Unmanaged CALLI in IL stubs must be inlined
-
 #ifdef DEBUG
         bool compGcChecks;         // Check arguments and return values to ensure they are sane
         bool compStackCheckOnRet;  // Check ESP on return to ensure it is correct

src/coreclr/src/jit/importer.cpp

@@ -5390,29 +5390,23 @@ GenTreePtr Compiler::impTransformThis(GenTreePtr              thisPtr,
 }
 
 //------------------------------------------------------------------------
-// impCanPInvokeInline: examine information from a call to see if the call
-// qualifies as an inline pinvoke.
-//
-// Arguments:
-//    block      - block contaning the call, or for inlinees, block
-//                 containing the call being inlined
+// impCanPInvokeInline: check whether PInvoke inlining should enabled in current method.
 //
 // Return Value:
-//    true if this call qualifies as an inline pinvoke, false otherwise
+//    true if PInvoke inlining should be enabled in current method, false otherwise
 //
 // Notes:
-//    Checks basic legality and then a number of ambient conditions
-//    where we could pinvoke but choose not to
+//    Checks a number of ambient conditions where we could pinvoke but choose not to
 
-bool Compiler::impCanPInvokeInline(BasicBlock* block)
+bool Compiler::impCanPInvokeInline()
 {
-    return impCanPInvokeInlineCallSite(block) && getInlinePInvokeEnabled() && (!opts.compDbgCode) &&
-           (compCodeOpt() != SMALL_CODE) && (!opts.compNoPInvokeInlineCB) // profiler is preventing inline pinvoke
+    return getInlinePInvokeEnabled() && (!opts.compDbgCode) && (compCodeOpt() != SMALL_CODE) &&
+           (!opts.compNoPInvokeInlineCB) // profiler is preventing inline pinvoke
         ;
 }
 
 //------------------------------------------------------------------------
-// impCanPInvokeInlineSallSite: basic legality checks using information
+// impCanPInvokeInlineCallSite: basic legality checks using information
 // from a call to see if the call qualifies as an inline pinvoke.
 //
 // Arguments:
@@ -5441,6 +5435,17 @@ bool Compiler::impCanPInvokeInline(BasicBlock* block)
 
 bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
 {
+    if (block->hasHndIndex())
+    {
+        return false;
+    }
+
+    // The remaining limitations do not apply to CoreRT
+    if (IsTargetAbi(CORINFO_CORERT_ABI))
+    {
+        return true;
+    }
+
 #ifdef _TARGET_AMD64_
     // On x64, we disable pinvoke inlining inside of try regions.
     // Here is the comment from JIT64 explaining why:
@@ -5462,12 +5467,13 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
     //
     //   A desktop test case where this seems to matter is
     //   jit\jit64\ebvts\mcpp\sources2\ijw\__clrcall\vector_ctor_dtor.02\deldtor_clr.exe
-    const bool inX64Try = block->hasTryIndex();
-#else
-    const bool inX64Try = false;
+    if (block->hasTryIndex())
+    {
+        return false;
+    }
 #endif // _TARGET_AMD64_
 
-    return !inX64Try && !block->hasHndIndex();
+    return true;
 }
 
 //------------------------------------------------------------------------
@@ -5533,27 +5539,38 @@ void Compiler::impCheckForPInvokeCall(
     }
     optNativeCallCount++;
 
-    if (opts.compMustInlinePInvokeCalli && methHnd == nullptr)
+    if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && methHnd == nullptr)
     {
-        // Always inline pinvoke.
+        // PInvoke CALLI in IL stubs must be inlined
     }
     else
     {
-        // Check legality and profitability.
-        if (!impCanPInvokeInline(block))
+        // Check legality
+        if (!impCanPInvokeInlineCallSite(block))
         {
             return;
         }
 
-        if (info.compCompHnd->pInvokeMarshalingRequired(methHnd, sig))
+        // PInvoke CALL in IL stubs must be inlined on CoreRT. Skip the ambient conditions checks and
+        // profitability checks
+        if (!(opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && IsTargetAbi(CORINFO_CORERT_ABI)))
         {
-            return;
+            if (impCanPInvokeInline())
+            {
+                return;
+            }
+
+            // Size-speed tradeoff: don't use inline pinvoke at rarely
+            // executed call sites.  The non-inline version is more
+            // compact.
+            if (block->isRunRarely())
+            {
+                return;
+            }
         }
 
-        // Size-speed tradeoff: don't use inline pinvoke at rarely
-        // executed call sites.  The non-inline version is more
-        // compact.
-        if (block->isRunRarely())
+        // The expensive check should be last
+        if (info.compCompHnd->pInvokeMarshalingRequired(methHnd, sig))
         {
             return;
         }
@@ -6220,7 +6237,7 @@ bool Compiler::impIsTailCallILPattern(bool        tailPrefixed,
              ((nextOpcode == CEE_NOP) || ((nextOpcode == CEE_POP) && (++cntPop == 1)))); // Next opcode = nop or exactly
                                                                                          // one pop seen so far.
 #else
-    nextOpcode          = (OPCODE)getU1LittleEndian(codeAddrOfNextOpcode);
+    nextOpcode = (OPCODE)getU1LittleEndian(codeAddrOfNextOpcode);
 #endif
 
     if (isCallPopAndRet)