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: Don't emit some unnecessary tests #38586

Merged
merged 8 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 22 additions & 7 deletions src/coreclr/src/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5908,12 +5908,14 @@ void CodeGen::genCompareInt(GenTree* treeNode)
{
assert(treeNode->OperIsCompare() || treeNode->OperIs(GT_CMP));

GenTreeOp* tree = treeNode->AsOp();
GenTree* op1 = tree->gtOp1;
GenTree* op2 = tree->gtOp2;
var_types op1Type = op1->TypeGet();
var_types op2Type = op2->TypeGet();
regNumber targetReg = tree->GetRegNum();
GenTreeOp* tree = treeNode->AsOp();
GenTree* op1 = tree->gtOp1;
GenTree* op2 = tree->gtOp2;
var_types op1Type = op1->TypeGet();
var_types op2Type = op2->TypeGet();
regNumber targetReg = tree->GetRegNum();
emitter* emit = GetEmitter();
bool canReuseFlags = false;

genConsumeOperands(tree);

Expand Down Expand Up @@ -5945,6 +5947,11 @@ void CodeGen::genCompareInt(GenTree* treeNode)
}
else if (op1->isUsedFromReg() && op2->IsIntegralConst(0))
{
if (compiler->opts.OptimizationEnabled())
{
canReuseFlags = true;
}

// We're comparing a register to 0 so we can generate "test reg1, reg1"
// instead of the longer "cmp reg1, 0"
ins = INS_test;
Expand Down Expand Up @@ -5995,7 +6002,15 @@ void CodeGen::genCompareInt(GenTree* treeNode)
// TYP_UINT and TYP_ULONG should not appear here, only small types can be unsigned
assert(!varTypeIsUnsigned(type) || varTypeIsSmall(type));

GetEmitter()->emitInsBinary(ins, emitTypeSize(type), op1, op2);
bool needsOCFlags = !tree->OperIs(GT_EQ, GT_NE);
if (canReuseFlags && emit->AreFlagsSetToZeroCmp(op1->GetRegNum(), emitTypeSize(type), needsOCFlags))
{
JITDUMP("Not emitting compare due to flags being already set\n");
}
else
{
emit->emitInsBinary(ins, emitTypeSize(type), op1, op2);
}

// Are we evaluating this into a register?
if (targetReg != REG_NA)
Expand Down
87 changes: 87 additions & 0 deletions src/coreclr/src/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,93 @@ bool emitter::AreUpper32BitsZero(regNumber reg)
return false;
}

//------------------------------------------------------------------------
// 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
//
// Arguments:
// reg - register of interest
// opSize - size of register
// needsOCFlags - additionally check the overflow and carry flags
//
// Return Value:
// true if the previous instruction set the flags for reg
// false if not, or if we can't safely determine
//
// Notes:
// Currently only looks back one instruction.
bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, bool needsOCFlags)
{
assert(reg != REG_NA);
// Don't look back across IG boundaries (possible control flow)
if (emitCurIGinsCnt == 0 && ((emitCurIG->igFlags & IGF_EXTEND) == 0))
{
return false;
}

instrDesc* id = emitLastIns;
insFormat fmt = id->idInsFmt();

// make sure op1 is a reg
switch (fmt)
{
case IF_RWR_CNS:
case IF_RRW_CNS:
case IF_RRW_SHF:
case IF_RWR_RRD:
case IF_RRW_RRD:
case IF_RWR_MRD:
case IF_RWR_SRD:
case IF_RRW_SRD:
case IF_RWR_ARD:
case IF_RRW_ARD:
case IF_RWR:
case IF_RRD:
case IF_RRW:
break;

default:
return false;
}
nathan-moore marked this conversation as resolved.
Show resolved Hide resolved

if (id->idReg1() != reg)
{
return false;
}

switch (id->idIns())
{
case INS_adc:
case INS_add:
case INS_dec:
case INS_dec_l:
case INS_inc:
case INS_inc_l:
case INS_neg:
case INS_shr_1:
case INS_shl_1:
case INS_sar_1:
case INS_sbb:
case INS_sub:
case INS_xadd:
if (needsOCFlags)
{
return false;
}
nathan-moore marked this conversation as resolved.
Show resolved Hide resolved
__fallthrough;
// these always set OC to 0
case INS_and:
case INS_or:
case INS_xor:
return id->idOpSize() == opSize;

default:
break;
}

return false;
}

//------------------------------------------------------------------------
// IsDstSrcImmAvxInstruction: Checks if the instruction has a "reg, reg/mem, imm" or
// "reg/mem, reg, imm" form for the legacy, VEX, and EVEX
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/emitxarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ bool Is4ByteSSEInstruction(instruction ins);

bool AreUpper32BitsZero(regNumber reg);

bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, bool needsOCFlags);

bool hasRexPrefix(code_t code)
{
#ifdef TARGET_AMD64
Expand Down