Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Relax PInvoke inlining restrictions
Browse files Browse the repository at this point in the history 
- 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.
  • Loading branch information
@jkotas
jkotas committed Dec 3, 2016
1 parent a0a055b commit 75923e7
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 67 deletions.
6 changes: 3 additions & 3 deletions src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
119 changes: 55 additions & 64 deletions src/jit/importer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -5367,61 +5346,77 @@ 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,
CORINFO_METHOD_HANDLE methHnd,
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
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 75923e7

Please sign in to comment.