Skip to content

Commit

Permalink
JIT: Lift remaining cmov restrictions by introducing GT_SELECTCC (#82235
Browse files Browse the repository at this point in the history
)

This introduces GT_SELECTCC and unifies its handling with GT_JCC. We no longer
use containment for GT_SELECT conditions in the xarch backend.

Additionally teaches liveness DCE about GT_SETCC and GT_SELECTCC by allowing it
to remove GTF_SET_FLAGS from the previous node when they are unused.

Minor diffs expected; the additional cases are really not that common. The main
benefit is that GT_SELECT is now fully on par with GT_JTRUE, and does not have
any odd limitations. The code to handle conditions is completely shared.
  • Loading branch information
jakobbotsch authored Feb 23, 2023
1 parent 71753c4 commit 3bc4f0e
Show file tree
Hide file tree
Showing 14 changed files with 309 additions and 284 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void genCompareInt(GenTree* treeNode);
#ifdef TARGET_XARCH
bool genCanAvoidEmittingCompareAgainstZero(GenTree* tree, var_types opType);
GenTreeCC* genTryFindFlagsConsumer(GenTree* flagsProducer);
GenTree* genTryFindFlagsConsumer(GenTree* flagsProducer, GenCondition** condition);
#endif

#ifdef FEATURE_SIMD
Expand Down
68 changes: 30 additions & 38 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1356,18 +1356,14 @@ instruction CodeGen::JumpKindToCmov(emitJumpKind condition)
}

//------------------------------------------------------------------------
// genCodeForCompare: Produce code for a GT_SELECT/GT_SELECT_HI node.
// genCodeForCompare: Produce code for a GT_SELECT/GT_SELECTCC node.
//
// Arguments:
// select - the node
//
void CodeGen::genCodeForSelect(GenTreeOp* select)
{
#ifdef TARGET_X86
assert(select->OperIs(GT_SELECT, GT_SELECT_HI));
#else
assert(select->OperIs(GT_SELECT));
#endif
assert(select->OperIs(GT_SELECT, GT_SELECTCC));

if (select->OperIs(GT_SELECT))
{
Expand All @@ -1385,25 +1381,13 @@ void CodeGen::genCodeForSelect(GenTreeOp* select)

if (select->OperIs(GT_SELECT))
{
GenTree* cond = select->AsConditional()->gtCond;
if (cond->isContained())
{
assert(cond->OperIsCompare());
genCodeForCompare(cond->AsOp());
cc = GenCondition::FromRelop(cond);

if (cc.PreferSwap())
{
// genCodeForCompare generated the compare with swapped
// operands because this swap requires fewer branches/cmovs.
cc = GenCondition::Swap(cc);
}
}
else
{
regNumber condReg = cond->GetRegNum();
GetEmitter()->emitIns_R_R(INS_test, EA_4BYTE, condReg, condReg);
}
GenTree* cond = select->AsConditional()->gtCond;
regNumber condReg = cond->GetRegNum();
GetEmitter()->emitIns_R_R(INS_test, emitActualTypeSize(cond), condReg, condReg);
}
else
{
cc = select->AsOpCC()->gtCondition;
}

// The usual codegen will be
Expand Down Expand Up @@ -1875,11 +1859,9 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
genCodeForSelect(treeNode->AsConditional());
break;

#ifdef TARGET_X86
case GT_SELECT_HI:
case GT_SELECTCC:
genCodeForSelect(treeNode->AsOp());
break;
#endif

case GT_RETURNTRAP:
genCodeForReturnTrap(treeNode->AsOp());
Expand Down Expand Up @@ -6809,22 +6791,23 @@ bool CodeGen::genCanAvoidEmittingCompareAgainstZero(GenTree* tree, var_types opT
return false;
}

GenTreeCC* cc = nullptr;
GenCondition cond;
GenTree* consumer = nullptr;
GenCondition* mutableCond = nullptr;
GenCondition cond;

if (tree->OperIsCompare())
{
cond = GenCondition::FromIntegralRelop(tree);
}
else
{
cc = genTryFindFlagsConsumer(tree);
if (cc == nullptr)
consumer = genTryFindFlagsConsumer(tree, &mutableCond);
if (consumer == nullptr)
{
return false;
}

cond = cc->gtCondition;
cond = *mutableCond;
}

if (GetEmitter()->AreFlagsSetToZeroCmp(op1->GetRegNum(), emitTypeSize(opType), cond))
Expand All @@ -6833,11 +6816,12 @@ bool CodeGen::genCanAvoidEmittingCompareAgainstZero(GenTree* tree, var_types opT
return true;
}

if ((cc != nullptr) && GetEmitter()->AreFlagsSetForSignJumpOpt(op1->GetRegNum(), emitTypeSize(opType), cond))
if ((mutableCond != nullptr) &&
GetEmitter()->AreFlagsSetForSignJumpOpt(op1->GetRegNum(), emitTypeSize(opType), cond))
{
JITDUMP("Not emitting compare due to sign being already set; modifying [%06u] to check sign flag\n",
Compiler::dspTreeID(cc));
cc->gtCondition =
Compiler::dspTreeID(consumer));
*mutableCond =
(cond.GetCode() == GenCondition::SLT) ? GenCondition(GenCondition::S) : GenCondition(GenCondition::NS);
return true;
}
Expand All @@ -6851,11 +6835,12 @@ bool CodeGen::genCanAvoidEmittingCompareAgainstZero(GenTree* tree, var_types opT
//
// Parameters:
// producer - the node that produces CPU flags
// cond - [out] the pointer to the condition inside that consumer.
//
// Returns:
// A node that consumes the flags, or nullptr if no such node was found.
//
GenTreeCC* CodeGen::genTryFindFlagsConsumer(GenTree* producer)
GenTree* CodeGen::genTryFindFlagsConsumer(GenTree* producer, GenCondition** cond)
{
assert((producer->gtFlags & GTF_SET_FLAGS) != 0);
// We allow skipping some nodes where we know for sure that the flags are
Expand All @@ -6866,7 +6851,14 @@ GenTreeCC* CodeGen::genTryFindFlagsConsumer(GenTree* producer)
{
if (candidate->OperIs(GT_JCC, GT_SETCC))
{
return candidate->AsCC();
*cond = &candidate->AsCC()->gtCondition;
return candidate;
}

if (candidate->OperIs(GT_SELECTCC))
{
*cond = &candidate->AsOpCC()->gtCondition;
return candidate;
}

// The following nodes can be inserted between the compare and the user
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2414,6 +2414,9 @@ class Compiler
// For binary opers.
GenTree* gtNewOperNode(genTreeOps oper, var_types type, GenTree* op1, GenTree* op2);

GenTreeCC* gtNewCC(genTreeOps oper, var_types type, GenCondition cond);
GenTreeOpCC* gtNewOperCC(genTreeOps oper, var_types type, GenCondition cond, GenTree* op1, GenTree* op2);

GenTreeColon* gtNewColonNode(var_types type, GenTree* elseNode, GenTree* thenNode);
GenTreeQmark* gtNewQmarkNode(var_types type, GenTree* cond, GenTreeColon* colon);

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/decomposelongs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1545,10 +1545,10 @@ GenTree* DecomposeLongs::DecomposeSelect(LIR::Use& use)

// Normally GT_SELECT is responsible for evaluating the condition into
// flags, but for the "upper half" we treat the lower GT_SELECT similar to
// other flag producing nodes and reuse them. GT_SELECT_HI is the variant
// other flag producing nodes and reuse them. GT_SELECTCC is the variant
// that uses existing flags and has no condition as part of it.
select->gtFlags |= GTF_SET_FLAGS;
GenTree* hiSelect = m_compiler->gtNewOperNode(GT_SELECT_HI, TYP_INT, hiOp1, hiOp2);
GenTree* hiSelect = m_compiler->gtNewOperCC(GT_SELECTCC, TYP_INT, GenCondition::NE, hiOp1, hiOp2);

Range().InsertAfter(select, hiSelect);

Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ void GenTree::InitNodeSize()
static_assert_no_msg(sizeof(GenTreeLclVar) <= TREE_NODE_SZ_SMALL);
static_assert_no_msg(sizeof(GenTreeLclFld) <= TREE_NODE_SZ_SMALL);
static_assert_no_msg(sizeof(GenTreeCC) <= TREE_NODE_SZ_SMALL);
static_assert_no_msg(sizeof(GenTreeOpCC) <= TREE_NODE_SZ_SMALL);
static_assert_no_msg(sizeof(GenTreeCast) <= TREE_NODE_SZ_LARGE); // *** large node
static_assert_no_msg(sizeof(GenTreeBox) <= TREE_NODE_SZ_LARGE); // *** large node
static_assert_no_msg(sizeof(GenTreeField) <= TREE_NODE_SZ_LARGE); // *** large node
Expand Down Expand Up @@ -6944,6 +6945,18 @@ GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree* op1,
return node;
}

GenTreeCC* Compiler::gtNewCC(genTreeOps oper, var_types type, GenCondition cond)
{
GenTreeCC* node = new (this, oper) GenTreeCC(oper, type, cond);
return node;
}

GenTreeOpCC* Compiler::gtNewOperCC(genTreeOps oper, var_types type, GenCondition cond, GenTree* op1, GenTree* op2)
{
GenTreeOpCC* node = new (this, oper) GenTreeOpCC(oper, type, cond, op1, op2);
return node;
}

GenTreeColon* Compiler::gtNewColonNode(var_types type, GenTree* elseNode, GenTree* thenNode)
{
return new (this, GT_COLON) GenTreeColon(TYP_INT, elseNode, thenNode);
Expand Down Expand Up @@ -12135,6 +12148,10 @@ void Compiler::gtDispTree(GenTree* tree,
break;
}
}
else if (tree->OperIs(GT_SELECTCC))
{
printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
}

gtDispCommonEndLine(tree);

Expand Down
32 changes: 29 additions & 3 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1686,7 +1686,16 @@ struct GenTree

bool OperIsConditionalJump() const
{
return (gtOper == GT_JTRUE) || (gtOper == GT_JCMP) || (gtOper == GT_JCC);
return OperIs(GT_JTRUE, GT_JCMP, GT_JCC);
}

bool OperConsumesFlags() const
{
#if !defined(TARGET_64BIT)
if (OperIs(GT_ADD_HI, GT_SUB_HI))
return true;
#endif
return OperIs(GT_JCC, GT_SETCC, GT_SELECTCC);
}

#ifdef DEBUG
Expand Down Expand Up @@ -8521,12 +8530,11 @@ struct GenCondition
};

// Represents a GT_JCC or GT_SETCC node.

struct GenTreeCC final : public GenTree
{
GenCondition gtCondition;

GenTreeCC(genTreeOps oper, GenCondition condition, var_types type = TYP_VOID)
GenTreeCC(genTreeOps oper, var_types type, GenCondition condition)
: GenTree(oper, type DEBUGARG(/*largeNode*/ FALSE)), gtCondition(condition)
{
assert(OperIs(GT_JCC, GT_SETCC));
Expand All @@ -8539,6 +8547,24 @@ struct GenTreeCC final : public GenTree
#endif // DEBUGGABLE_GENTREE
};

// Represents a node with two operands and a condition.
struct GenTreeOpCC final : public GenTreeOp
{
GenCondition gtCondition;

GenTreeOpCC(genTreeOps oper, var_types type, GenCondition condition, GenTree* op1 = nullptr, GenTree* op2 = nullptr)
: GenTreeOp(oper, type, op1, op2 DEBUGARG(/*largeNode*/ FALSE)), gtCondition(condition)
{
assert(OperIs(GT_SELECTCC));
}

#if DEBUGGABLE_GENTREE
GenTreeOpCC() : GenTreeOp()
{
}
#endif // DEBUGGABLE_GENTREE
};

//------------------------------------------------------------------------
// Deferred inline functions of GenTree -- these need the subtypes above to
// be defined already.
Expand Down
13 changes: 7 additions & 6 deletions src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,6 @@ GTNODE(SUB_HI , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR)
GTNODE(LSH_HI , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR)
GTNODE(RSH_LO , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR)

// Variant of SELECT that reuses flags computed by a previous SELECT.
GTNODE(SELECT_HI , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR)
#endif // !defined(TARGET_64BIT)

#ifdef FEATURE_HW_INTRINSICS
Expand Down Expand Up @@ -234,16 +232,19 @@ GTNODE(CNEG_LT , GenTreeOp ,0,GTK_UNOP|DBK_NOTHIR) // Conditi
GTNODE(CMP , GenTreeOp ,0,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR)
// Generate a test instruction; sets the CPU flags according to (a & b) and does not produce a value.
GTNODE(TEST , GenTreeOp ,0,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR)
#ifdef TARGET_XARCH
// The XARCH BT instruction. Like CMP, this sets the condition flags (CF to be precise) and does not produce a value.
GTNODE(BT , GenTreeOp ,0,(GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR))
#endif
// Makes a comparison and jump if the condition specified. Does not set flags.
GTNODE(JCMP , GenTreeOp ,0,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR)
// Checks the condition flags and branch if the condition specified by GenTreeCC::gtCondition is true.
GTNODE(JCC , GenTreeCC ,0,GTK_LEAF|GTK_NOVALUE|DBK_NOTHIR)
// Checks the condition flags and produces 1 if the condition specified by GenTreeCC::gtCondition is true and 0 otherwise.
GTNODE(SETCC , GenTreeCC ,0,GTK_LEAF|DBK_NOTHIR)
#ifdef TARGET_XARCH
// The XARCH BT instruction. Like CMP, this sets the condition flags (CF to be precise) and does not produce a value.
GTNODE(BT , GenTreeOp ,0,(GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR))
#endif
// Variant of SELECT that reuses flags computed by a previous node with the specified condition.
GTNODE(SELECTCC , GenTreeCC ,0,GTK_BINOP|DBK_NOTHIR)


//-----------------------------------------------------------------------------
// Other nodes that look like unary/binary operators:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gtstructs.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ GTSTRUCT_1(AllocObj , GT_ALLOCOBJ)
GTSTRUCT_1(RuntimeLookup, GT_RUNTIMELOOKUP)
GTSTRUCT_1(ArrAddr , GT_ARR_ADDR)
GTSTRUCT_2(CC , GT_JCC, GT_SETCC)
GTSTRUCT_1(OpCC , GT_SELECTCC)
#if defined(TARGET_X86)
GTSTRUCT_1(MultiRegOp , GT_MUL_LONG)
#elif defined (TARGET_ARM)
Expand Down
Loading

0 comments on commit 3bc4f0e

Please sign in to comment.