diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index cec348f86ee44..840ad2e8c6091 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -2631,6 +2631,16 @@ void CodeGen::genCodeForJumpTrue(GenTreeOp* jtrue) condition = GenCondition(GenCondition::P); } + + if (relop->MarkedForSignJumpOpt()) + { + // If relop was previously marked for a signed jump check optimization because of SF flag + // reuse, replace jge/jl with jns/js. + + assert(relop->OperGet() == GT_LT || relop->OperGet() == GT_GE); + condition = (relop->OperGet() == GT_LT) ? GenCondition(GenCondition::S) : GenCondition(GenCondition::NS); + } + #endif inst_JCC(condition, compiler->compCurBB->bbJumpDest); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 5c8e0bdd5c2ae..d580cebc72fea 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -6227,11 +6227,18 @@ void CodeGen::genCompareInt(GenTree* treeNode) assert(genTypeSize(type) <= genTypeSize(TYP_I_IMPL)); // TYP_UINT and TYP_ULONG should not appear here, only small types can be unsigned assert(!varTypeIsUnsigned(type) || varTypeIsSmall(type)); + // Sign jump optimization should only be set the following check + assert((tree->gtFlags & GTF_RELOP_SJUMP_OPT) == 0); if (canReuseFlags && emit->AreFlagsSetToZeroCmp(op1->GetRegNum(), emitTypeSize(type), tree->OperGet())) { JITDUMP("Not emitting compare due to flags being already set\n"); } + else if (canReuseFlags && emit->AreFlagsSetForSignJumpOpt(op1->GetRegNum(), emitTypeSize(type), tree)) + { + JITDUMP("Not emitting compare due to sign being already set, follow up instr will transform jump\n"); + tree->gtFlags |= GTF_RELOP_SJUMP_OPT; + } else { emit->emitInsBinary(ins, emitTypeSize(type), op1, op2); diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 7f88c5049acba..fa6d44cda0f4b 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -164,6 +164,21 @@ bool emitter::DoesWriteZeroFlag(instruction ins) return (CodeGenInterface::instInfo[ins] & Writes_ZF) != 0; } +//------------------------------------------------------------------------ +// DoesWriteSignFlag: check if the instruction writes the +// SF flag. +// +// Arguments: +// ins - instruction to test +// +// Return Value: +// true if instruction writes the SF flag, false otherwise. +// +bool emitter::DoesWriteSignFlag(instruction ins) +{ + return (CodeGenInterface::instInfo[ins] & Writes_SF) != 0; +} + //------------------------------------------------------------------------ // DoesResetOverflowAndCarryFlags: check if the instruction resets the // OF and CF flag to 0. @@ -338,6 +353,11 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, genTreeOps tr { assert(reg != REG_NA); + if (!emitComp->opts.OptimizationEnabled()) + { + return false; + } + // Don't look back across IG boundaries (possible control flow) if (emitCurIGinsCnt == 0 && ((emitCurIG->igFlags & IGF_EXTEND) == 0)) { @@ -393,6 +413,79 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, genTreeOps tr return false; } +//------------------------------------------------------------------------ +// AreFlagsSetToForSignJumpOpt: checks if the previous instruction set the SF if the tree +// node qualifies for a jg/jle to jns/js optimization +// +// Arguments: +// reg - register of interest +// opSize - size of register +// relop - relational tree node +// +// Return Value: +// true if the tree node qualifies for the jg/jle to jns/js optimization +// false if not, or if we can't safely determine +// +// Notes: +// Currently only looks back one instruction. +bool emitter::AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenTree* relop) +{ + assert(reg != REG_NA); + + if (!emitComp->opts.OptimizationEnabled()) + { + return false; + } + + // Don't look back across IG boundaries (possible control flow) + if (emitCurIGinsCnt == 0 && ((emitCurIG->igFlags & IGF_EXTEND) == 0)) + { + return false; + } + + instrDesc* id = emitLastIns; + instruction lastIns = id->idIns(); + 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; + } + + if (id->idReg1() != reg) + { + return false; + } + + // If we have a GT_GE/GT_LT which generates an jge/jl, and the previous instruction + // sets the SF, we can omit a test instruction and check for jns/js. + if ((relop->OperGet() == GT_GE || relop->OperGet() == GT_LT) && !GenCondition::FromRelop(relop).IsUnsigned()) + { + if (DoesWriteSignFlag(lastIns) && IsFlagsAlwaysModified(id)) + { + return id->idOpSize() == opSize; + } + } + + 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 diff --git a/src/coreclr/jit/emitxarch.h b/src/coreclr/jit/emitxarch.h index 39458eade16cf..d57b986623a13 100644 --- a/src/coreclr/jit/emitxarch.h +++ b/src/coreclr/jit/emitxarch.h @@ -116,6 +116,7 @@ static bool IsJmpInstruction(instruction ins); bool AreUpper32BitsZero(regNumber reg); bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, genTreeOps treeOps); +bool AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenTree* tree); bool hasRexPrefix(code_t code) { @@ -190,6 +191,7 @@ void SetContains256bitAVX(bool value) bool IsDstDstSrcAVXInstruction(instruction ins); bool IsDstSrcSrcAVXInstruction(instruction ins); bool DoesWriteZeroFlag(instruction ins); +bool DoesWriteSignFlag(instruction ins); bool DoesResetOverflowAndCarryFlags(instruction ins); bool IsFlagsAlwaysModified(instrDesc* id); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ec3d178a95115..762b76268227a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -531,6 +531,8 @@ enum GenTreeFlags : unsigned int GTF_RELOP_QMARK = 0x20000000, // GT_ -- the node is the condition for ?: GTF_RELOP_ZTT = 0x08000000, // GT_ -- Loop test cloned for converting while-loops into do-while // with explicit "loop test" in the header block. + GTF_RELOP_SJUMP_OPT = 0x04000000, // GT_ -- Swap signed jl/jge with js/jns during emitter, reuses flags + // from previous instruction. GTF_JCMP_EQ = 0x80000000, // GTF_JCMP_EQ -- Branch on equal rather than not equal GTF_JCMP_TST = 0x40000000, // GTF_JCMP_TST -- Use bit test instruction rather than compare against zero instruction @@ -3028,6 +3030,13 @@ struct GenTreeOp : public GenTreeUnOp { } #endif + + // True if this relop is marked for a transform during the emitter + // phase, e.g., jge => jns + bool MarkedForSignJumpOpt() const + { + return (gtFlags & GTF_RELOP_SJUMP_OPT) != 0; + } }; struct GenTreeVal : public GenTree