From 75923e710cf8b8a22a82925b79a5b344b7168f3d Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 2 Dec 2016 21:42:18 -0800 Subject: [PATCH] Relax PInvoke inlining restrictions - Remove !impLocAllocOnStack() and callRetTyp != TYP_STRUCT PInvoke inlining restrictions that were left-overs from old times (before 2001) when PInvoke inlining was very impoverished and different from regular calls. The argument handling for PInvoke inlining and regular calls is same today. Since regular calls have to deal with these conditions properly, PInvoke inlining gets it for free. Also, these conditions have been exercised by IL stubs where the PInvoke inlining restrictions are ignored. - Disable CoreCLR-specific PInvoke inlining restriction for CoreRT. --- src/jit/compiler.h | 6 +-- src/jit/importer.cpp | 119 ++++++++++++++++++++----------------------- 2 files changed, 58 insertions(+), 67 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index b8b6a1aea2f4..ab80df92d603 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2770,8 +2770,8 @@ class Compiler void impImportNewObjArray(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_CALL_INFO* pCallInfo); - bool impCanPInvokeInline(var_types callRetTyp); - bool impCanPInvokeInlineCallSite(var_types callRetTyp); + bool impCanPInvokeInline(); + bool impCanPInvokeInlineCallSite(); void impCheckForPInvokeCall(GenTreePtr call, CORINFO_METHOD_HANDLE methHnd, CORINFO_SIG_INFO* sig, unsigned mflags); GenTreePtr impImportIndirectCall(CORINFO_SIG_INFO* sig, IL_OFFSETX ilOffset = BAD_IL_OFFSET); void impPopArgsForUnmanagedCall(GenTreePtr call, CORINFO_SIG_INFO* sig); @@ -2820,7 +2820,7 @@ class Compiler void impImportLeave(BasicBlock* block); void impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr); - BOOL impLocAllocOnStack(); + GenTreePtr impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 094bfc4e687c..be1d7bfc79cc 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -2853,27 +2853,6 @@ GenTreePtr Compiler::impImplicitR4orR8Cast(GenTreePtr tree, var_types dstTyp) return tree; } -/*****************************************************************************/ -BOOL Compiler::impLocAllocOnStack() -{ - if (!compLocallocUsed) - { - return (FALSE); - } - - // Returns true if a GT_LCLHEAP node is encountered in any of the trees - // that have been pushed on the importer evaluatuion stack. - // - for (unsigned i = 0; i < verCurrentState.esStackDepth; i++) - { - if (fgWalkTreePre(&verCurrentState.esStack[i].val, Compiler::fgChkLocAllocCB) == WALK_ABORT) - { - return (TRUE); - } - } - return (FALSE); -} - //------------------------------------------------------------------------ // impInitializeArrayIntrinsic: Attempts to replace a call to InitializeArray // with a GT_COPYBLK node. @@ -5367,53 +5346,70 @@ GenTreePtr Compiler::impTransformThis(GenTreePtr thisPtr, } } -bool Compiler::impCanPInvokeInline(var_types callRetTyp) +bool Compiler::impCanPInvokeInline() { - return impCanPInvokeInlineCallSite(callRetTyp) && getInlinePInvokeEnabled() && (!opts.compDbgCode) && + return impCanPInvokeInlineCallSite() && getInlinePInvokeEnabled() && (!opts.compDbgCode) && (compCodeOpt() != SMALL_CODE) && (!opts.compNoPInvokeInlineCB) // profiler is preventing inline pinvoke ; } // Returns false only if the callsite really cannot be inlined. Ignores global variables // like debugger, profiler etc. -bool Compiler::impCanPInvokeInlineCallSite(var_types callRetTyp) +bool Compiler::impCanPInvokeInlineCallSite() { - return - // We have to disable pinvoke inlining inside of filters - // because in case the main execution (i.e. in the try block) is inside - // unmanaged code, we cannot reuse the inlined stub (we still need the - // original state until we are in the catch handler) - (!bbInFilterILRange(compCurBB)) && - // We disable pinvoke inlining inside handlers since the GSCookie is - // in the inlined Frame (see CORINFO_EE_INFO::InlinedCallFrameInfo::offsetOfGSCookie), - // but this would not protect framelets/return-address of handlers. - !compCurBB->hasHndIndex() && + // We have to disable pinvoke inlining inside of filters + // because in case the main execution (i.e. in the try block) is inside + // unmanaged code, we cannot reuse the inlined stub (we still need the + // original state until we are in the catch handler) + if (bbInFilterILRange(compCurBB)) + { + return false; + } + + // We disable pinvoke inlining inside handlers since the GSCookie is + // in the inlined Frame (see CORINFO_EE_INFO::InlinedCallFrameInfo::offsetOfGSCookie), + // but this would not protect framelets/return-address of handlers. + if (compCurBB->hasHndIndex()) + { + return false; + } + + // The remaining limitations do not apply to CoreRT + if (IsTargetAbi(CORINFO_CORERT_ABI)) + { + return true; + } + #ifdef _TARGET_AMD64_ - // Turns out JIT64 doesn't perform PInvoke inlining inside try regions, here's an excerpt of - // the comment from JIT64 explaining why: - // - //// [VSWhidbey: 611015] - because the jitted code links in the Frame (instead - //// of the stub) we rely on the Frame not being 'active' until inside the - //// stub. This normally happens by the stub setting the return address - //// pointer in the Frame object inside the stub. On a normal return, the - //// return address pointer is zeroed out so the Frame can be safely re-used, - //// but if an exception occurs, nobody zeros out the return address pointer. - //// Thus if we re-used the Frame object, it would go 'active' as soon as we - //// link it into the Frame chain. - //// - //// Technically we only need to disable PInvoke inlining if we're in a - //// handler or if we're - //// in a try body with a catch or filter/except where other non-handler code - //// in this method might run and try to re-use the dirty Frame object. - // - // Now, because of this, the VM actually assumes that in 64 bit we never PInvoke - // inline calls on any EH construct, you can verify that on VM\ExceptionHandling.cpp:203 - // The method responsible for resuming execution is UpdateObjectRefInResumeContextCallback - // you can see how it aligns with JIT64 policy of not inlining PInvoke calls almost right - // at the beginning of the body of the method. - !compCurBB->hasTryIndex() && + // Turns out JIT64 doesn't perform PInvoke inlining inside try regions, here's an excerpt of + // the comment from JIT64 explaining why: + // + //// [VSWhidbey: 611015] - because the jitted code links in the Frame (instead + //// of the stub) we rely on the Frame not being 'active' until inside the + //// stub. This normally happens by the stub setting the return address + //// pointer in the Frame object inside the stub. On a normal return, the + //// return address pointer is zeroed out so the Frame can be safely re-used, + //// but if an exception occurs, nobody zeros out the return address pointer. + //// Thus if we re-used the Frame object, it would go 'active' as soon as we + //// link it into the Frame chain. + //// + //// Technically we only need to disable PInvoke inlining if we're in a + //// handler or if we're + //// in a try body with a catch or filter/except where other non-handler code + //// in this method might run and try to re-use the dirty Frame object. + // + // Now, because of this, the VM actually assumes that in 64 bit we never PInvoke + // inline calls on any EH construct, you can verify that on VM\ExceptionHandling.cpp:203 + // The method responsible for resuming execution is UpdateObjectRefInResumeContextCallback + // you can see how it aligns with JIT64 policy of not inlining PInvoke calls almost right + // at the beginning of the body of the method. + if (compCurBB->hasTryIndex()) + { + return false; + } #endif - (!impLocAllocOnStack()) && (callRetTyp != TYP_STRUCT); + + return true; } void Compiler::impCheckForPInvokeCall(GenTreePtr call, @@ -5421,7 +5417,6 @@ void Compiler::impCheckForPInvokeCall(GenTreePtr call, CORINFO_SIG_INFO* sig, unsigned mflags) { - var_types callRetTyp = JITtype2varType(sig->retType); CorInfoUnmanagedCallConv unmanagedCallConv; // If VM flagged it as Pinvoke, flag the call node accordingly @@ -5464,15 +5459,11 @@ void Compiler::impCheckForPInvokeCall(GenTreePtr call, if (opts.compMustInlinePInvokeCalli && methHnd == nullptr) { -#ifdef _TARGET_X86_ // CALLI in IL stubs must be inlined - assert(impCanPInvokeInlineCallSite(callRetTyp)); - assert(!info.compCompHnd->pInvokeMarshalingRequired(methHnd, sig)); -#endif // _TARGET_X86_ } else { - if (!impCanPInvokeInline(callRetTyp)) + if (!impCanPInvokeInline()) { return; }