Skip to content

Commit b54529f

Browse files
authored
JIT: give inlinees their own EH table (#112968)
For a long time now inlinees have "inherited" the root method EH table. Looks like the only reason we were doing this was to make certain pinvoke inlining checks simpler. Rework those to handle the inline case directly. Quirk one bit of profitability logic for now to avoid diffs. Part of #108900.
1 parent 478cfe2 commit b54529f

File tree

2 files changed

+51
-64
lines changed

2 files changed

+51
-64
lines changed

src/coreclr/jit/fgbasic.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3513,13 +3513,6 @@ void Compiler::fgFindBasicBlocks()
35133513
return;
35143514
}
35153515

3516-
noway_assert(info.compXcptnsCount == 0);
3517-
compHndBBtab = impInlineInfo->InlinerCompiler->compHndBBtab;
3518-
compHndBBtabAllocCount =
3519-
impInlineInfo->InlinerCompiler->compHndBBtabAllocCount; // we probably only use the table, not add to it.
3520-
compHndBBtabCount = impInlineInfo->InlinerCompiler->compHndBBtabCount;
3521-
info.compXcptnsCount = impInlineInfo->InlinerCompiler->info.compXcptnsCount;
3522-
35233516
// Use a spill temp for the return value if there are multiple return blocks,
35243517
// or if the inlinee has GC ref locals.
35253518
if ((info.compRetNativeType != TYP_VOID) && ((fgReturnCount > 1) || impInlineInfo->HasGcRefLocals()))

src/coreclr/jit/importercalls.cpp

Lines changed: 51 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
110110
{
111111
if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
112112
{
113-
// See comment in impCheckForPInvokeCall
114-
BasicBlock* block = compIsForInlining() ? impInlineInfo->iciBlock : compCurBB;
115-
if (info.compCompHnd->convertPInvokeCalliToCall(pResolvedToken, !impCanPInvokeInlineCallSite(block)))
113+
if (info.compCompHnd->convertPInvokeCalliToCall(pResolvedToken, !impCanPInvokeInlineCallSite(compCurBB)))
116114
{
117115
eeGetCallInfo(pResolvedToken, nullptr, CORINFO_CALLINFO_ALLOWINSTPARAM, callInfo);
118116
return impImportCall(CEE_CALL, pResolvedToken, nullptr, nullptr, prefixFlags, callInfo, rawILOffset);
@@ -633,17 +631,9 @@ var_types Compiler::impImportCall(OPCODE opcode,
633631
}
634632

635633
//--------------------------- Inline NDirect ------------------------------
636-
637-
// For inline cases we technically should look at both the current
638-
// block and the call site block (or just the latter if we've
639-
// fused the EH trees). However the block-related checks pertain to
640-
// EH and we currently won't inline a method with EH. So for
641-
// inlinees, just checking the call site block is sufficient.
642-
{
643-
// New lexical block here to avoid compilation errors because of GOTOs.
644-
BasicBlock* block = compIsForInlining() ? impInlineInfo->iciBlock : compCurBB;
645-
impCheckForPInvokeCall(call->AsCall(), methHnd, sig, mflags, block);
646-
}
634+
// If this is a call to a PInvoke method, we may be able to inline the invocation frame.
635+
//
636+
impCheckForPInvokeCall(call->AsCall(), methHnd, sig, mflags, compCurBB);
647637

648638
#ifdef UNIX_X86_ABI
649639
// On Unix x86 we use caller-cleaned convention.
@@ -6366,8 +6356,7 @@ bool Compiler::impCanPInvokeInline()
63666356
// from a call to see if the call qualifies as an inline pinvoke.
63676357
//
63686358
// Arguments:
6369-
// block - block containing the call, or for inlinees, block
6370-
// containing the call being inlined
6359+
// block - block containing the call
63716360
//
63726361
// Return Value:
63736362
// true if this call can legally qualify as an inline pinvoke, false otherwise
@@ -6384,9 +6373,9 @@ bool Compiler::impCanPInvokeInline()
63846373
// TODO-CQ: The inlining frame no longer has a GSCookie, so the common on this
63856374
// restriction is out of date. However, given that there is a comment
63866375
// about protecting the framelet, I'm not confident about what this
6387-
// is actually protecteing, so I don't want to remove this
6376+
// is actually protecting, so I don't want to remove this
63886377
// restriction without further analysis analysis.
6389-
// * We disable pinvoke inlini1ng inside handlers since the GSCookie
6378+
// * We disable pinvoke inlining inside handlers since the GSCookie
63906379
// is in the inlined Frame (see
63916380
// CORINFO_EE_INFO::InlinedCallFrameInfo::offsetOfGSCookie), but
63926381
// this would not protect framelets/return-address of handlers.
@@ -6401,49 +6390,53 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
64016390
return false;
64026391
}
64036392

6404-
// The remaining limitations do not apply to NativeAOT
6405-
if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
6406-
{
6407-
return true;
6408-
}
6409-
6410-
// The VM assumes that the PInvoke frame in IL Stub is only going to be used
6411-
// for the PInvoke target call. The PInvoke frame cannot be reused by marshalling helper
6412-
// calls (see InlinedCallFrame::GetActualInteropMethodDesc and related stackwalking code).
6413-
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB))
6393+
// The following limitations do not apply to NativeAOT
6394+
//
6395+
if (!IsTargetAbi(CORINFO_NATIVEAOT_ABI))
64146396
{
6415-
return false;
6416-
}
6397+
// The VM assumes that the PInvoke frame in IL Stub is only going to be used
6398+
// for the PInvoke target call. The PInvoke frame cannot be reused by marshalling helper
6399+
// calls (see InlinedCallFrame::GetActualInteropMethodDesc and related stackwalking code).
6400+
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB))
6401+
{
6402+
return false;
6403+
}
64176404

64186405
#ifdef USE_PER_FRAME_PINVOKE_INIT
6419-
// For platforms that use per-P/Invoke InlinedCallFrame initialization,
6420-
// we can't inline P/Invokes inside of try blocks where we can resume execution in the same function.
6421-
// The runtime can correctly unwind out of an InlinedCallFrame and out of managed code. However,
6422-
// it cannot correctly unwind out of an InlinedCallFrame and stop at that frame without also unwinding
6423-
// at least one managed frame. In particular, the runtime struggles to restore non-volatile registers
6424-
// from the top-most unmanaged call before the InlinedCallFrame. As a result, the runtime does not support
6425-
// re-entering the same method frame as the InlinedCallFrame after an exception in unmanaged code.
6426-
if (block->hasTryIndex())
6427-
{
6428-
// Check if this block's try block or any containing try blocks have catch handlers.
6429-
// If any of the containing try blocks have catch handlers,
6430-
// we cannot inline a P/Invoke for reasons above. If the handler is a fault or finally handler,
6431-
// we can inline a P/Invoke into this block in the try since the code will not resume execution
6432-
// in the same method after throwing an exception if only fault or finally handlers are executed.
6433-
for (unsigned int ehIndex = block->getTryIndex(); ehIndex != EHblkDsc::NO_ENCLOSING_INDEX;
6434-
ehIndex = ehGetEnclosingTryIndex(ehIndex))
6435-
{
6436-
if (ehGetDsc(ehIndex)->HasCatchHandler())
6406+
// For platforms that use per-P/Invoke InlinedCallFrame initialization,
6407+
// we can't inline P/Invokes inside of try blocks where we can resume execution in the same function.
6408+
// The runtime can correctly unwind out of an InlinedCallFrame and out of managed code. However,
6409+
// it cannot correctly unwind out of an InlinedCallFrame and stop at that frame without also unwinding
6410+
// at least one managed frame. In particular, the runtime struggles to restore non-volatile registers
6411+
// from the top-most unmanaged call before the InlinedCallFrame. As a result, the runtime does not support
6412+
// re-entering the same method frame as the InlinedCallFrame after an exception in unmanaged code.
6413+
if (block->hasTryIndex())
6414+
{
6415+
// Check if this block's try block or any containing try blocks have catch handlers.
6416+
// If any of the containing try blocks have catch handlers,
6417+
// we cannot inline a P/Invoke for reasons above. If the handler is a fault or finally handler,
6418+
// we can inline a P/Invoke into this block in the try since the code will not resume execution
6419+
// in the same method after throwing an exception if only fault or finally handlers are executed.
6420+
for (unsigned int ehIndex = block->getTryIndex(); ehIndex != EHblkDsc::NO_ENCLOSING_INDEX;
6421+
ehIndex = ehGetEnclosingTryIndex(ehIndex))
64376422
{
6438-
return false;
6423+
if (ehGetDsc(ehIndex)->HasCatchHandler())
6424+
{
6425+
return false;
6426+
}
64396427
}
64406428
}
6429+
#endif // USE_PER_FRAME_PINVOKE_INIT
6430+
}
64416431

6432+
if (!compIsForInlining())
6433+
{
64426434
return true;
64436435
}
6444-
#endif // USE_PER_FRAME_PINVOKE_INIT
64456436

6446-
return true;
6437+
// If inlining, verify conditions for the call site block too.
6438+
//
6439+
return impInlineRoot()->impCanPInvokeInlineCallSite(impInlineInfo->iciBlock);
64476440
}
64486441

64496442
//------------------------------------------------------------------------
@@ -6455,8 +6448,7 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
64556448
// methHnd - handle for the method being called (may be null)
64566449
// sig - signature of the method being called
64576450
// mflags - method flags for the method being called
6458-
// block - block containing the call, or for inlinees, block
6459-
// containing the call being inlined
6451+
// block - block containing the call
64606452
//
64616453
// Notes:
64626454
// Sets GTF_CALL_M_PINVOKE on the call for pinvokes.
@@ -6554,7 +6546,11 @@ void Compiler::impCheckForPInvokeCall(
65546546
// Size-speed tradeoff: don't use inline pinvoke at rarely
65556547
// executed call sites. The non-inline version is more
65566548
// compact.
6557-
if (block->isRunRarely())
6549+
//
6550+
// Zero-diff quirk: the first clause below should simply be block->isRunRarely()
6551+
//
6552+
if ((!compIsForInlining() && block->isRunRarely()) ||
6553+
(compIsForInlining() && impInlineInfo->iciBlock->isRunRarely()))
65586554
{
65596555
return;
65606556
}
@@ -7742,9 +7738,7 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call,
77427738

77437739
if (methAttr & CORINFO_FLG_PINVOKE)
77447740
{
7745-
// See comment in impCheckForPInvokeCall
7746-
BasicBlock* block = compIsForInlining() ? impInlineInfo->iciBlock : compCurBB;
7747-
if (!impCanPInvokeInlineCallSite(block))
7741+
if (!impCanPInvokeInlineCallSite(compCurBB))
77487742
{
77497743
inlineResult->NoteFatal(InlineObservation::CALLSITE_PINVOKE_EH);
77507744
return;

0 commit comments

Comments
 (0)