Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JIT] X86/X64 - Eliminate redundant 'cmp' instructions #82750

Merged
merged 33 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
5f12506
Fixed improper peephole zero-extension removal when cdq/cdqe/cwde ins…
TIHan Feb 27, 2023
cf0bdf2
Update regression test
TIHan Feb 27, 2023
697dfdc
Formatting
TIHan Feb 27, 2023
f78c28d
Handle cdq differently
TIHan Feb 27, 2023
b6467cb
Handle cdq differently
TIHan Feb 27, 2023
334fc46
Handle cdq differently
TIHan Feb 27, 2023
47eb762
Initial commit to eliminate redundant 'cmp' instructions
TIHan Feb 28, 2023
ef8688b
Take into account cmpxchg
TIHan Feb 28, 2023
2184607
Take into account cmpxchg
TIHan Feb 28, 2023
25ac969
Feedback
TIHan Feb 28, 2023
2cc3541
Temporarily disable cmp opt if we encounter a mov
TIHan Mar 1, 2023
1013666
Merge remote-tracking branch 'upstream/main' into redundant-cmp-opt
TIHan Mar 1, 2023
15e6462
Merge branch 'redundant-cmp-opt' of https://github.com/TIHan/runtime …
TIHan Mar 1, 2023
3218581
Allow checking for mov
TIHan Mar 1, 2023
e5e2599
Allow regardless of targetReg
TIHan Mar 1, 2023
baee67f
Allow regardless of targetReg
TIHan Mar 1, 2023
0827dc3
Merge branch 'zero-extend-fix' into redundant-cmp-opt
TIHan Mar 1, 2023
03e4f8b
Checking if an instruction resets a flag.
TIHan Mar 1, 2023
5058e59
Remove useless comment
TIHan Mar 1, 2023
0ba01ef
Minor fix
TIHan Mar 1, 2023
8d8485e
Abort are checking cmp
TIHan Mar 2, 2023
49b6f27
Some refactoring. Taking into account any instruction that modifies f…
TIHan Mar 3, 2023
49a8043
Minor cleanup
TIHan Mar 3, 2023
ee0cd47
Remove function from header
TIHan Mar 3, 2023
91c1054
Quick fix
TIHan Mar 3, 2023
1c5f518
Sync
TIHan Mar 4, 2023
f74a895
Merge
TIHan Mar 6, 2023
61ee9da
Merge remote-tracking branch 'upstream/main' into redundant-cmp-opt
TIHan Apr 3, 2023
4d4e059
Formatting
TIHan Apr 3, 2023
03d9ab3
Only look for 'cmp reg, reg'
TIHan Apr 5, 2023
51c70a7
Added comment
TIHan Apr 5, 2023
3dd723f
Update src/coreclr/jit/emitxarch.cpp
TIHan Apr 5, 2023
0cb00b4
Update src/coreclr/jit/emitxarch.cpp
TIHan Apr 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7023,7 +7023,15 @@ void CodeGen::genCompareInt(GenTree* treeNode)
targetType = TYP_BOOL; // just a tip for inst_SETCC that movzx is not needed
}
}
emit->emitInsBinary(ins, emitTypeSize(type), op1, op2);

emitAttr size = emitTypeSize(type);
bool canSkip = compiler->opts.OptimizationEnabled() && (ins == INS_cmp) && !op1->isUsedFromMemory() &&
!op2->isUsedFromMemory() && emit->IsRedundantCmp(size, op1->GetRegNum(), op2->GetRegNum());

if (!canSkip)
{
emit->emitInsBinary(ins, size, op1, op2);
}
}

// Are we evaluating this into a register?
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,7 @@ class emitter

#ifdef TARGET_XARCH
bool emitIsInstrWritingToReg(instrDesc* id, regNumber reg);
bool emitDoesInsModifyFlags(instruction ins);
#endif // TARGET_XARCH

/************************************************************************/
Expand Down
87 changes: 87 additions & 0 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,24 @@ bool emitter::AreUpper32BitsSignExtended(regNumber reg)
}
#endif // TARGET_64BIT

//------------------------------------------------------------------------
// emitDoesInsModifyFlags: checks if the given instruction modifies flags
//
// Arguments:
// ins - instruction of interest
//
// Return Value:
// true if the instruction modifies flags.
// false if it does not.
//
bool emitter::emitDoesInsModifyFlags(instruction ins)
{
return (CodeGenInterface::instInfo[ins] &
(Resets_OF | Resets_SF | Resets_AF | Resets_PF | Resets_CF | Undefined_OF | Undefined_SF | Undefined_AF |
Undefined_PF | Undefined_CF | Undefined_ZF | Writes_OF | Writes_SF | Writes_AF | Writes_PF | Writes_CF |
Writes_ZF | Restore_SF_ZF_AF_PF_CF));
}

//------------------------------------------------------------------------
// emitIsInstrWritingToReg: checks if the given register is being written to
//
Expand Down Expand Up @@ -794,6 +812,75 @@ bool emitter::emitIsInstrWritingToReg(instrDesc* id, regNumber reg)
return false;
}

//------------------------------------------------------------------------
// IsRedundantCmp: determines if there is a 'cmp' instruction that is redundant with the given inputs
//
// Arguments:
// size - size of 'cmp'
// reg1 - op1 register of 'cmp'
// reg2 - op2 register of 'cmp'
//
// Return Value:
// true if there is a redundant 'cmp'
//
bool emitter::IsRedundantCmp(emitAttr size, regNumber reg1, regNumber reg2)
{
// Only allow GPRs.
// If not a valid register, then return false.
if (!genIsValidIntReg(reg1))
return false;

if (!genIsValidIntReg(reg2))
return false;

// Only consider if safe
//
if (!emitCanPeepholeLastIns())
{
return false;
}

bool result = false;

emitPeepholeIterateLastInstrs([&](instrDesc* id) {
instruction ins = id->idIns();

switch (ins)
{
case INS_cmp:
{
// We only care about 'cmp reg, reg'.
if (id->idInsFmt() != IF_RRD_RRD)
return PEEPHOLE_ABORT;

if ((id->idReg1() == reg1) && (id->idReg2() == reg2))
{
result = (size == id->idOpSize());
}

return PEEPHOLE_ABORT;
}

default:
break;
}

if (emitDoesInsModifyFlags(ins))
{
return PEEPHOLE_ABORT;
}

if (emitIsInstrWritingToReg(id, reg1) || emitIsInstrWritingToReg(id, reg2))
{
return PEEPHOLE_ABORT;
}

return PEEPHOLE_CONTINUE;
});

return result;
}

//------------------------------------------------------------------------
// AreFlagsSetToZeroCmp: Checks if the previous instruction set the SZ, and optionally OC, flags to
// the same values as if there were a compare to 0
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/emitxarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ bool AreUpper32BitsZero(regNumber reg);
bool AreUpper32BitsSignExtended(regNumber reg);
#endif // TARGET_64BIT

bool IsRedundantCmp(emitAttr size, regNumber reg1, regNumber reg2);

bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition cond);
bool AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenCondition cond);

Expand Down