Skip to content

Commit

Permalink
JIT: Remove unneeded unconditional jumps (#69041)
Browse files Browse the repository at this point in the history
* resolve merge conflict

* add checks for post-call jmp removal

* use emitIsLastInsCall() to preclude call,jmp pairs and simplfy emitRemoveJumpToNextInst
check for aligned instruction groups and skip them to preserve alignment
linearize emitRemoveJumpToNextInst, add safety asserts for ordering and comment the requirement, add debug output explaining decisions

* address feedback

* change instruction omit to use code size zero

* address feedback

* add assertion info dump

* address feedback

* address feedback

* remove end loop cleanup. cleanup jitdump messages and add early out flag.

* address feedback, formatting

* update variable naming and clarify comments

* more feedback

* add last instruction check and group flag set in emitSavIG

* more feedback

* fix jitdump output and make removal candidate printing optional

* remove emitDispJumpList parameter

* change overall size calculation and allow 0 size align to be printed in dumps

* fix invorrect jmp check, remove inline for emitInstHasNoCode

* allow alignment printing even when zero

* remove debug printf and fix x86

* address feedback
  • Loading branch information
Wraith2 authored Jun 7, 2022
1 parent a0ffea8 commit 11f24dd
Show file tree
Hide file tree
Showing 13 changed files with 429 additions and 69 deletions.
5 changes: 4 additions & 1 deletion src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -1472,8 +1472,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

public:
void instGen(instruction ins);

#if defined(TARGET_XARCH)
void inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock, bool isRemovableJmpCandidate = false);
#else
void inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock);
#endif

void inst_SET(emitJumpKind condition, regNumber reg);

Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4288,6 +4288,18 @@ void CodeGen::inst_SETCC(GenCondition condition, var_types type, regNumber dstRe
#endif
}

//------------------------------------------------------------------------
// inst_JMP: Generate a jump instruction.
//
void CodeGen::inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock)
{
#if !FEATURE_FIXED_OUT_ARGS
assert((tgtBlock->bbTgtStkDepth * sizeof(int) == genStackLevel) || isFramePointerUsed());
#endif // !FEATURE_FIXED_OUT_ARGS

GetEmitter()->emitIns_J(emitter::emitJumpKindToIns(jmp), tgtBlock);
}

//------------------------------------------------------------------------
// genCodeForStoreBlk: Produce code for a GT_STORE_OBJ/GT_STORE_DYN_BLK/GT_STORE_BLK node.
//
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1944,11 +1944,12 @@ void CodeGen::genGenerateMachineCode()
#endif // DEBUG

/* We can now generate the function prolog and epilog */

genGeneratePrologsAndEpilogs();

/* Bind jump distances */
// check to see if any jumps can be removed
GetEmitter()->emitRemoveJumpToNextInst();

/* Bind jump distances */
GetEmitter()->emitJumpDistBind();

#if FEATURE_LOOP_ALIGN
Expand Down
19 changes: 18 additions & 1 deletion src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,24 @@ void CodeGen::genCodeForBBlist()
break;

case BBJ_ALWAYS:
inst_JMP(EJ_jmp, block->bbJumpDest);
inst_JMP(EJ_jmp, 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()
#else
#ifdef TARGET_XARCH
,
/* isRemovableJmpCandidate */ !block->hasAlign()
#endif

#endif
);
FALLTHROUGH;

case BBJ_COND:
Expand Down
24 changes: 24 additions & 0 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,30 @@ void CodeGen::inst_SETCC(GenCondition condition, var_types type, regNumber dstRe
}
}

//------------------------------------------------------------------------
// inst_JMP: Generate a jump instruction.
//
void CodeGen::inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock, bool isRemovableJmpCandidate)
{
#if !FEATURE_FIXED_OUT_ARGS
// On the x86 we are pushing (and changing the stack level), but on x64 and other archs we have
// a fixed outgoing args area that we store into and we never change the stack level when calling methods.
//
// Thus only on x86 do we need to assert that the stack level at the target block matches the current stack level.
//
CLANG_FORMAT_COMMENT_ANCHOR;

#ifdef UNIX_X86_ABI
// bbTgtStkDepth is a (pure) argument count (stack alignment padding should be excluded).
assert((tgtBlock->bbTgtStkDepth * sizeof(int) == (genStackLevel - curNestedAlignment)) || isFramePointerUsed());
#else
assert((tgtBlock->bbTgtStkDepth * sizeof(int) == genStackLevel) || isFramePointerUsed());
#endif
#endif // !FEATURE_FIXED_OUT_ARGS

GetEmitter()->emitIns_J(emitter::emitJumpKindToIns(jmp), tgtBlock, 0, isRemovableJmpCandidate);
}

//------------------------------------------------------------------------
// genCodeForReturnTrap: Produce code for a GT_RETURNTRAP node.
//
Expand Down
Loading

0 comments on commit 11f24dd

Please sign in to comment.