Skip to content

Commit

Permalink
JIT: do early block merging in more cases (#86157)
Browse files Browse the repository at this point in the history
OSR and PGO both rely on the fact that the early flowgraph the JIT builds is
the same flowgraph as the one seen in a previous JIT complation of that method,
since there is metadata (patchpoint offset, pgo schema) that refers to the
flowgraph. Previous work here (#85860) didn't ensure this for enough cases.

In particular if Tier0 does not do early block merging, but OSR does, it's possible
for the OSR entry point to fall within a merged block range, which the JIT is not
prepared to handle. This lead to the asserts seen in #86125.

The fix is to enable early block merging unless we're truly in a minopts or
debug codegen mode (previous to this Tier0 would not merge unless it also
was instrumenting; now it will always merge).

An alternative fix would be to find the block containing the OSR entry IL
offset, scan its statements, and split the block at the right spot, but that
seemed more involved.

Fixes #86125.
  • Loading branch information
AndyAyersMS authored May 13, 2023
1 parent ff65075 commit aa38055
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 7 deletions.
16 changes: 12 additions & 4 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -9393,11 +9393,19 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return IsInstrumented() && jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT);
}

bool CanBeInstrumentedOrIsOptimized() const
bool DoEarlyBlockMerging() const
{
return IsInstrumented() || (jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0) &&
jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR_IF_LOOPS)) ||
jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT);
if (jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_EnC) || jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_CODE))
{
return false;
}

if (jitFlags->IsSet(JitFlags::JIT_FLAG_MIN_OPT) && !jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0))
{
return false;
}

return true;
}

// true if we should use the PINVOKE_{BEGIN,END} helpers instead of generating
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1840,7 +1840,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed

if ((jmpDist == 0) &&
(opcode == CEE_LEAVE || opcode == CEE_LEAVE_S || opcode == CEE_BR || opcode == CEE_BR_S) &&
opts.CanBeInstrumentedOrIsOptimized())
opts.DoEarlyBlockMerging())
{
break; /* NOP */
}
Expand Down Expand Up @@ -2989,7 +2989,7 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F

jmpDist = (sz == 1) ? getI1LittleEndian(codeAddr) : getI4LittleEndian(codeAddr);

if ((jmpDist == 0) && (opcode == CEE_BR || opcode == CEE_BR_S) && opts.CanBeInstrumentedOrIsOptimized())
if ((jmpDist == 0) && (opcode == CEE_BR || opcode == CEE_BR_S) && opts.DoEarlyBlockMerging())
{
continue; /* NOP */
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7420,7 +7420,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
case CEE_BR_S:
jmpDist = (sz == 1) ? getI1LittleEndian(codeAddr) : getI4LittleEndian(codeAddr);

if ((jmpDist == 0) && opts.CanBeInstrumentedOrIsOptimized())
if ((jmpDist == 0) && opts.DoEarlyBlockMerging())
{
break; /* NOP */
}
Expand Down

0 comments on commit aa38055

Please sign in to comment.