From 1e63ca0e018b278e013d2c7bf159b78d14aff126 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 2 Dec 2016 21:42:18 -0800 Subject: [PATCH] Update PInvoke inlining restrictions for CoreRT --- src/jit/compiler.cpp | 2 -- src/jit/compiler.h | 4 +-- src/jit/importer.cpp | 73 +++++++++++++++++++++++++++----------------- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 9a0215d09a2c..9a96be8cd8c7 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -3296,8 +3296,6 @@ void Compiler::compInitOptions(JitFlags* jitFlags) } #endif - opts.compMustInlinePInvokeCalli = jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB); - opts.compScopeInfo = opts.compDbgInfo; #ifdef LATE_DISASM diff --git a/src/jit/compiler.h b/src/jit/compiler.h index da30fa4e1840..ee1692d52d6f 100644 --- a/src/jit/compiler.h +++ b/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); @@ -7626,8 +7626,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 diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 9351094a7f5a..dd9f75f57d61 100644 --- a/src/jit/importer.cpp +++ b/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)