From 8b3be893f0259d7b5c9a7ea66f9bdeecc9cafc13 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Thu, 16 Jun 2022 08:09:08 -0700 Subject: [PATCH] Fix regression in runtime-jit-experimental (#70797) 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. --- src/coreclr/jit/codegenlinear.cpp | 33 +++++++++++++++++-------------- src/coreclr/jit/flowgraph.cpp | 1 + 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index f2528f6f3c62..b9026bfedd6f 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -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: diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 52399aa6c169..ac240be9a2d2 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3421,6 +3421,7 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock() { firstColdBlock = fgFirstBB->bbNext; prevToFirstColdBlock = fgFirstBB; + JITDUMP("JitStressProcedureSplitting is enabled: Splitting after the first basic block\n"); } else {