Skip to content

Commit f8f5125

Browse files
authored
JIT: stop using ehTrueEnclosingTryIndexIL outside of importation (#113102)
Once we can inline methods with EH, IL ranges are no longer a reliable indicator of a mutual-protect try regions. Instead, after importation, we can rely on mutual-protect trys having the same start and end blocks. Also update other case where we were using `info.compXcptnsCount` in morph to decide if we needed a frame pointer. This lets us simplify the logic around frame pointers and EH (though I still think we're making up our minds too early). Contributes to #108900.
1 parent bb146ad commit f8f5125

File tree

7 files changed

+81
-46
lines changed

7 files changed

+81
-46
lines changed

src/coreclr/jit/codegencommon.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -2363,8 +2363,8 @@ void CodeGen::genReportEH()
23632363
{
23642364
for (XTnum = 0; XTnum < compiler->compHndBBtabCount; XTnum++)
23652365
{
2366-
for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndexIL(XTnum); // find the true enclosing try index,
2367-
// ignoring 'mutual protect' trys
2366+
for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndex(XTnum); // find the true enclosing try index,
2367+
// ignoring 'mutual protect' trys
23682368
enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX;
23692369
enclosingTryIndex = compiler->ehGetEnclosingTryIndex(enclosingTryIndex))
23702370
{
@@ -2670,8 +2670,8 @@ void CodeGen::genReportEH()
26702670

26712671
EHblkDsc* fletTab = compiler->ehGetDsc(XTnum2);
26722672

2673-
for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndexIL(XTnum2); // find the true enclosing try index,
2674-
// ignoring 'mutual protect' trys
2673+
for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndex(XTnum2); // find the true enclosing try index,
2674+
// ignoring 'mutual protect' trys
26752675
enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX;
26762676
enclosingTryIndex = compiler->ehGetEnclosingTryIndex(enclosingTryIndex))
26772677
{

src/coreclr/jit/compiler.h

+5
Original file line numberDiff line numberDiff line change
@@ -2779,6 +2779,9 @@ class Compiler
27792779
// Find the true enclosing try index, ignoring 'mutual protect' try. Uses IL ranges to check.
27802780
unsigned ehTrueEnclosingTryIndexIL(unsigned regionIndex);
27812781

2782+
// Find the true enclosing try index, ignoring 'mutual protect' try. Uses blocks to check.
2783+
unsigned ehTrueEnclosingTryIndex(unsigned regionIndex);
2784+
27822785
// Return the index of the most nested enclosing region for a particular EH region. Returns NO_ENCLOSING_INDEX
27832786
// if there is no enclosing region. If the returned index is not NO_ENCLOSING_INDEX, then '*inTryRegion'
27842787
// is set to 'true' if the enclosing region is a 'try', or 'false' if the enclosing region is a handler.
@@ -5285,6 +5288,8 @@ class Compiler
52855288
// This is derived from the profile data
52865289
// or is BB_UNITY_WEIGHT when we don't have profile data
52875290

5291+
bool fgImportDone = false; // true once importation has finished
5292+
52885293
bool fgFuncletsCreated = false; // true if the funclet creation phase has been run
52895294

52905295
bool fgGlobalMorph = false; // indicates if we are during the global morphing phase

src/coreclr/jit/fgehopt.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2736,7 +2736,7 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
27362736
break;
27372737
}
27382738
outermostEbd = ehGetDsc(enclosingTryIndex);
2739-
if (!EHblkDsc::ebdIsSameILTry(outermostEbd, tryEbd))
2739+
if (!EHblkDsc::ebdIsSameTry(outermostEbd, tryEbd))
27402740
{
27412741
break;
27422742
}

src/coreclr/jit/flowgraph.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,8 @@ PhaseStatus Compiler::fgImport()
590590
INDEBUG(fgPgoDeferredInconsistency = false);
591591
}
592592

593+
fgImportDone = true;
594+
593595
return PhaseStatus::MODIFIED_EVERYTHING;
594596
}
595597

@@ -6140,7 +6142,7 @@ bool FlowGraphNaturalLoop::CanDuplicateWithEH(INDEBUG(const char** reason))
61406142
//
61416143
const bool headerInTry = header->hasTryIndex();
61426144
unsigned blockIndex = block->getTryIndex();
6143-
unsigned outermostBlockIndex = comp->ehTrueEnclosingTryIndexIL(blockIndex);
6145+
unsigned outermostBlockIndex = comp->ehTrueEnclosingTryIndex(blockIndex);
61446146

61456147
if ((headerInTry && (outermostBlockIndex == header->getTryIndex())) ||
61466148
(!headerInTry && (outermostBlockIndex == EHblkDsc::NO_ENCLOSING_INDEX)))

src/coreclr/jit/jiteh.cpp

+60-7
Original file line numberDiff line numberDiff line change
@@ -799,14 +799,24 @@ unsigned Compiler::ehGetMostNestedRegionIndex(BasicBlock* block, bool* inTryRegi
799799
return mostNestedRegion;
800800
}
801801

802-
/*****************************************************************************
803-
* Returns the try index of the enclosing try, skipping all EH regions with the
804-
* same try region (that is, all 'mutual protect' regions). If there is no such
805-
* enclosing try, returns EHblkDsc::NO_ENCLOSING_INDEX.
806-
*/
802+
//-------------------------------------------------------------
803+
// ehTrueEnclosingTryIndexIL: find the outermost enclosing try
804+
// region that is not a mutual-protect try
805+
//
806+
// Arguments:
807+
// regionIndex - index of interest
808+
//
809+
// Returns:
810+
// Index of enclosng non-mutual protect try region, or EHblkDsc::NO_ENCLOSING_INDEX.
811+
//
812+
// Notes:
813+
// Only safe to use during importation, before we have normalize the
814+
// EH in the flow graph. Post importation use, the non-IL version.
815+
//
807816
unsigned Compiler::ehTrueEnclosingTryIndexIL(unsigned regionIndex)
808817
{
809818
assert(regionIndex != EHblkDsc::NO_ENCLOSING_INDEX);
819+
assert(!fgImportDone);
810820

811821
EHblkDsc* ehDscRoot = ehGetDsc(regionIndex);
812822
EHblkDsc* HBtab = ehDscRoot;
@@ -832,6 +842,49 @@ unsigned Compiler::ehTrueEnclosingTryIndexIL(unsigned regionIndex)
832842
return regionIndex;
833843
}
834844

845+
//-------------------------------------------------------------
846+
// ehTrueEnclosingTryIndex: find the closest enclosing try
847+
// region that is not a mutual-protect try
848+
//
849+
// Arguments:
850+
// regionIndex - index of interest
851+
//
852+
// Returns:
853+
// Index of enclosng non-mutual protect try region, or EHblkDsc::NO_ENCLOSING_INDEX.
854+
//
855+
// Notes:
856+
// Only safe to use after importation, once we have normalized the
857+
// EH in the flow graph. For importation, use the IL version.
858+
//
859+
unsigned Compiler::ehTrueEnclosingTryIndex(unsigned regionIndex)
860+
{
861+
assert(regionIndex != EHblkDsc::NO_ENCLOSING_INDEX);
862+
assert(fgImportDone);
863+
864+
EHblkDsc* ehDscRoot = ehGetDsc(regionIndex);
865+
EHblkDsc* HBtab = ehDscRoot;
866+
867+
for (;;)
868+
{
869+
regionIndex = HBtab->ebdEnclosingTryIndex;
870+
if (regionIndex == EHblkDsc::NO_ENCLOSING_INDEX)
871+
{
872+
// No enclosing 'try'; we're done
873+
break;
874+
}
875+
876+
HBtab = ehGetDsc(regionIndex);
877+
if (!EHblkDsc::ebdIsSameTry(ehDscRoot, HBtab))
878+
{
879+
// Found an enclosing 'try' that has a different 'try' region (is not mutually-protect with the
880+
// original region). Return it.
881+
break;
882+
}
883+
}
884+
885+
return regionIndex;
886+
}
887+
835888
unsigned Compiler::ehGetEnclosingRegionIndex(unsigned regionIndex, bool* inTryRegion)
836889
{
837890
assert(regionIndex != EHblkDsc::NO_ENCLOSING_INDEX);
@@ -3614,8 +3667,8 @@ void Compiler::fgVerifyHandlerTab()
36143667
// on the block.
36153668
for (XTnum = 0, HBtab = compHndBBtab; XTnum < compHndBBtabCount; XTnum++, HBtab++)
36163669
{
3617-
unsigned enclosingTryIndex = ehTrueEnclosingTryIndexIL(XTnum); // find the true enclosing try index,
3618-
// ignoring 'mutual protect' trys
3670+
unsigned enclosingTryIndex = ehTrueEnclosingTryIndex(XTnum); // find the true enclosing try index,
3671+
// ignoring 'mutual protect' trys
36193672
if (enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX)
36203673
{
36213674
// The handler funclet for 'XTnum' has a try index of 'enclosingTryIndex' (at least, the parts of the

src/coreclr/jit/morph.cpp

+6-31
Original file line numberDiff line numberDiff line change
@@ -13388,47 +13388,22 @@ void Compiler::fgSetOptions()
1338813388
codeGen->setFramePointerRequired(true);
1338913389
}
1339013390

13391-
// Assert that the EH table has been initialized by now. Note that
13392-
// compHndBBtabAllocCount never decreases; it is a high-water mark
13393-
// of table allocation. In contrast, compHndBBtabCount does shrink
13394-
// if we delete a dead EH region, and if it shrinks to zero, the
13395-
// table pointer compHndBBtab is unreliable.
13396-
assert(compHndBBtabAllocCount >= info.compXcptnsCount);
13397-
13398-
#ifdef TARGET_X86
13399-
13400-
// Note: this case, and the !X86 case below, should both use the
13401-
// !X86 path. This would require a few more changes for X86 to use
13402-
// compHndBBtabCount (the current number of EH clauses) instead of
13403-
// info.compXcptnsCount (the number of EH clauses in IL), such as
13404-
// in ehNeedsShadowSPslots(). This is because sometimes the IL has
13405-
// an EH clause that we delete as statically dead code before we
13406-
// get here, leaving no EH clauses left, and thus no requirement
13407-
// to use a frame pointer because of EH. But until all the code uses
13408-
// the same test, leave info.compXcptnsCount here. Also test for
13409-
// CORINFO_FLG_SYNCH methods which are converted into try-finally
13410-
// with Monitor helper calls in funclet ABI and need to be treated
13411-
// as methods with EH.
13412-
if (info.compXcptnsCount > 0 || (UsesFunclets() && (info.compFlags & CORINFO_FLG_SYNCH)))
13391+
// If there is EH, we need a frame pointer.
13392+
// Note this may premature... we can eliminate all EH after morph, sometimes.
13393+
//
13394+
if (compHndBBtabCount > 0)
1341313395
{
1341413396
codeGen->setFramePointerRequiredEH(true);
1341513397

13398+
#ifdef TARGET_X86
1341613399
if (UsesFunclets())
1341713400
{
1341813401
assert(!codeGen->isGCTypeFixed());
1341913402
// Enforce fully interruptible codegen for funclet unwinding
1342013403
SetInterruptible(true);
1342113404
}
13422-
}
13423-
13424-
#else // !TARGET_X86
13425-
13426-
if (compHndBBtabCount > 0)
13427-
{
13428-
codeGen->setFramePointerRequiredEH(true);
13429-
}
13430-
1343113405
#endif // TARGET_X86
13406+
}
1343213407

1343313408
if (compMethodRequiresPInvokeFrame())
1343413409
{

src/coreclr/jit/optimizer.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -2866,7 +2866,7 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop)
28662866
{
28672867
// Preheader should be in the true enclosing region of the header.
28682868
//
2869-
preheaderEHRegion = ehTrueEnclosingTryIndexIL(preheaderEHRegion);
2869+
preheaderEHRegion = ehTrueEnclosingTryIndex(preheaderEHRegion);
28702870
inSameRegionAsHeader = false;
28712871
break;
28722872
}
@@ -5208,7 +5208,7 @@ void Compiler::fgSetEHRegionForNewPreheaderOrExit(BasicBlock* block)
52085208
{
52095209
// `next` is the beginning of a try block. Figure out the EH region to use.
52105210
assert(next->hasTryIndex());
5211-
unsigned newTryIndex = ehTrueEnclosingTryIndexIL(next->getTryIndex());
5211+
unsigned newTryIndex = ehTrueEnclosingTryIndex(next->getTryIndex());
52125212
if (newTryIndex == EHblkDsc::NO_ENCLOSING_INDEX)
52135213
{
52145214
// No EH try index.

0 commit comments

Comments
 (0)