Skip to content

Commit 17d413f

Browse files
authored
A couple of small OSR fixes (#38165)
* fix helper multithreading issue -- don't re-read flags that can change * fix helper logging and abort on error * change strategy for protecting original method entry block when doing an OSR compile. Instead of marking it don't remove, use artificial ref count boost.
1 parent dbcb819 commit 17d413f

File tree

4 files changed

+31
-10
lines changed

4 files changed

+31
-10
lines changed

src/coreclr/src/jit/flowgraph.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -3070,6 +3070,14 @@ void Compiler::fgComputePreds()
30703070
// Treat the initial block as a jump target
30713071
fgFirstBB->bbFlags |= BBF_JMP_TARGET | BBF_HAS_LABEL;
30723072

3073+
// Under OSR, we may need to specially protect the original method entry.
3074+
//
3075+
if (opts.IsOSR() && (fgEntryBB != nullptr) && (fgEntryBB->bbFlags & BBF_IMPORTED))
3076+
{
3077+
JITDUMP("OSR: protecting original method entry " FMT_BB "\n", fgEntryBB->bbNum);
3078+
fgEntryBB->bbRefs = 1;
3079+
}
3080+
30733081
for (block = fgFirstBB; block; block = block->bbNext)
30743082
{
30753083
switch (block->bbJumpKind)
@@ -21272,6 +21280,13 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
2127221280
blockRefs += 1;
2127321281
}
2127421282

21283+
// Under OSR, if we also are keeping the original method entry around,
21284+
// mark that as implicitly referenced as well.
21285+
if (opts.IsOSR() && (block == fgEntryBB))
21286+
{
21287+
blockRefs += 1;
21288+
}
21289+
2127521290
/* Check the bbRefs */
2127621291
if (checkBBRefs)
2127721292
{

src/coreclr/src/jit/importer.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -8670,12 +8670,6 @@ var_types Compiler::impImportCall(OPCODE opcode,
86708670
JITDUMP("\nOSR: found tail recursive call in the method, scheduling " FMT_BB " for importation\n",
86718671
fgEntryBB->bbNum);
86728672
impImportBlockPending(fgEntryBB);
8673-
8674-
// Note there is no explicit flow to this block yet,
8675-
// make sure it stays around until we actually try
8676-
// the optimization.
8677-
fgEntryBB->bbFlags |= BBF_DONT_REMOVE;
8678-
86798673
loopHead = fgEntryBB;
86808674
}
86818675
else

src/coreclr/src/jit/morph.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -16311,6 +16311,17 @@ void Compiler::fgMorphBlocks()
1631116311
fgGlobalMorph = false;
1631216312
compCurBB = nullptr;
1631316313

16314+
// Under OSR, we no longer need to specially protect the original method entry
16315+
//
16316+
if (opts.IsOSR() && (fgEntryBB != nullptr) && (fgEntryBB->bbFlags & BBF_IMPORTED))
16317+
{
16318+
JITDUMP("OSR: un-protecting original method entry " FMT_BB "\n", fgEntryBB->bbNum);
16319+
assert(fgEntryBB->bbRefs > 0);
16320+
fgEntryBB->bbRefs--;
16321+
// We don't need to remember this block anymore.
16322+
fgEntryBB = nullptr;
16323+
}
16324+
1631416325
#ifdef DEBUG
1631516326
if (verboseTrees)
1631616327
{

src/coreclr/src/vm/jithelpers.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -5206,7 +5206,7 @@ void JIT_Patchpoint(int* counter, int ilOffset)
52065206
return;
52075207
}
52085208

5209-
LONG newFlags = ppInfo->m_flags | PerPatchpointInfo::patchpoint_triggered;
5209+
LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered;
52105210
BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags;
52115211

52125212
if (!triggerTransition)
@@ -5238,8 +5238,8 @@ void JIT_Patchpoint(int* counter, int ilOffset)
52385238
if (osrMethodCode == NULL)
52395239
{
52405240
// Unexpected, but not fatal
5241-
STRESS_LOG4(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_Patchpoint: patchpoint (0x%p) OSR method creation failed,"
5242-
" marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, hitCount, pMD, ilOffset);
5241+
STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_Patchpoint: patchpoint (0x%p) OSR method creation failed,"
5242+
" marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset);
52435243

52445244
InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid);
52455245
return;
@@ -5279,8 +5279,9 @@ void JIT_Patchpoint(int* counter, int ilOffset)
52795279
if ((UINT_PTR)ip != GetIP(&frameContext))
52805280
{
52815281
// Should be fatal
5282-
STRESS_LOG2(LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: patchpoint (0x%p) TRANSITION"
5282+
STRESS_LOG2(LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_Patchpoint: patchpoint (0x%p) TRANSITION"
52835283
" unexpected context IP 0x%p\n", ip, GetIP(&frameContext));
5284+
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
52845285
}
52855286

52865287
// Now unwind back to the original method caller frame.

0 commit comments

Comments
 (0)