Skip to content

Commit

Permalink
Fix regression in runtime-jit-experimental (#70797)
Browse files Browse the repository at this point in the history
The newly-introduced `emitRemoveJumpToNextInst` optimization caused
a regression when hot/cold-splitting, where jumps from the last hot
instruction to the first cold instruction were erroneously removed.
This is fixed by disabling the `isRemovableJmpCandidate` flag for
branches between hot/cold sections.

On an unrelated note, a JIT dump message has been added to indicate
stress-splitting is occurring.
  • Loading branch information
Aman Khalid committed Jun 16, 2022
1 parent 3b04dc1 commit 8b3be89
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
33 changes: 18 additions & 15 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,24 +753,27 @@ void CodeGen::genCodeForBBlist()
break;

case BBJ_ALWAYS:
inst_JMP(EJ_jmp, block->bbJumpDest
#ifdef TARGET_XARCH
{
// If a block was selected to place an alignment instruction because it ended
// with a jump, do not remove jumps from such blocks.
// Do not remove a jump between hot and cold regions.
bool isRemovableJmpCandidate =
!block->hasAlign() && !compiler->fgInDifferentRegions(block, block->bbJumpDest);

#ifdef TARGET_AMD64
// AMD64 requires an instruction after a call instruction for unwinding
// inside an EH region so if the last instruction generated was a call instruction
// do not allow this jump to be marked for possible later removal.
//
// If a block was selected to place an alignment instruction because it ended
// with a jump, do not remove jumps from such blocks.
,
/* isRemovableJmpCandidate */ !GetEmitter()->emitIsLastInsCall() && !block->hasAlign()
// AMD64 requires an instruction after a call instruction for unwinding
// inside an EH region so if the last instruction generated was a call instruction
// do not allow this jump to be marked for possible later removal.
isRemovableJmpCandidate = isRemovableJmpCandidate && !GetEmitter()->emitIsLastInsCall();
#endif // TARGET_AMD64

inst_JMP(EJ_jmp, block->bbJumpDest, isRemovableJmpCandidate);
}
#else
#ifdef TARGET_XARCH
,
/* isRemovableJmpCandidate */ !block->hasAlign()
#endif
inst_JMP(EJ_jmp, block->bbJumpDest);
#endif // TARGET_XARCH

#endif
);
FALLTHROUGH;

case BBJ_COND:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3421,6 +3421,7 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock()
{
firstColdBlock = fgFirstBB->bbNext;
prevToFirstColdBlock = fgFirstBB;
JITDUMP("JitStressProcedureSplitting is enabled: Splitting after the first basic block\n");
}
else
{
Expand Down

0 comments on commit 8b3be89

Please sign in to comment.