Skip to content

Use cinv and cneg instead of csel when possible #84926

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
May 25, 2023
Merged

Use cinv and cneg instead of csel when possible #84926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
cddd702
Use cinv instead of csel when possible
SwapnilGaikwad Apr 13, 2023
0af5517
Fix when the operands of conditional select should be reversed
SwapnilGaikwad Apr 19, 2023
3045a55
Merge main
SwapnilGaikwad Apr 20, 2023
a61733b
Scope down the cinv optimisation to local vars
SwapnilGaikwad Apr 20, 2023
b480a50
Merge main
SwapnilGaikwad May 11, 2023
a3739a3
Use more generic CSINV instead of CINV instruction
SwapnilGaikwad May 11, 2023
b76546a
Add support for CSNEG and refactor conditional selects
SwapnilGaikwad May 18, 2023
2833d1c
Fix initialisation of srcReg2
SwapnilGaikwad May 18, 2023
31c0da4
Add RequiresProcessIsolation property to fix test failures
SwapnilGaikwad May 19, 2023
d872fdd
Address review comments - Part 1
SwapnilGaikwad May 22, 2023
0dc7361
Remove output type from conditional negate tests
SwapnilGaikwad May 22, 2023
c32126a
Ensure operands of cneg/cinv are invariants while optimising
SwapnilGaikwad May 22, 2023
34b779a
Merge main
SwapnilGaikwad May 22, 2023
7c3a5fc
Update tests to the new format
SwapnilGaikwad May 22, 2023
bb415c9
Use annotations to run tests through the framework
SwapnilGaikwad May 23, 2023
c46cf43
Fix conditional negate tests
SwapnilGaikwad May 23, 2023
9534d10
Handle conditional negate/invert of shifted operands
SwapnilGaikwad May 24, 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
2 changes: 0 additions & 2 deletions src/coreclr/jit/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,7 +901,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void genCodeForCompare(GenTreeOp* tree);
#ifdef TARGET_ARM64
void genCodeForCCMP(GenTreeCCMP* ccmp);
void genCodeForCinc(GenTreeOp* cinc);
#endif
void genCodeForSelect(GenTreeOp* select);
void genIntrinsic(GenTreeIntrinsic* treeNode);
Expand Down Expand Up @@ -1250,7 +1249,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#if defined(TARGET_ARM64)
void genCodeForJumpCompare(GenTreeOpCC* tree);
void genCodeForBfiz(GenTreeOp* tree);
void genCodeForCond(GenTreeOp* tree);
#endif // TARGET_ARM64

#if defined(FEATURE_EH_FUNCLETS)
Expand Down
188 changes: 64 additions & 124 deletions src/coreclr/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4683,32 +4683,49 @@ void CodeGen::genCodeForCCMP(GenTreeCCMP* ccmp)
}

//------------------------------------------------------------------------
// genCodeForSelect: Produce code for a GT_SELECT node.
// genCodeForSelect: Produce code for a GT_SELECT/GT_SELECT_INV/GT_SELECT_NEG node.
//
// Arguments:
// tree - the node
//
void CodeGen::genCodeForSelect(GenTreeOp* tree)
{
assert(tree->OperIs(GT_SELECT, GT_SELECTCC));
GenTree* opcond = nullptr;
if (tree->OperIs(GT_SELECT))
assert(tree->OperIs(GT_SELECT, GT_SELECTCC, GT_SELECT_INC, GT_SELECT_INCCC, GT_SELECT_INV, GT_SELECT_INVCC,
GT_SELECT_NEG, GT_SELECT_NEGCC));
GenTree* opcond = nullptr;
instruction ins = INS_csel;
GenTree* op1 = tree->gtOp1;
GenTree* op2 = tree->gtOp2;

if (tree->OperIs(GT_SELECT_INV, GT_SELECT_INVCC))
{
ins = (op2 == nullptr) ? INS_cinv : INS_csinv;
}
else if (tree->OperIs(GT_SELECT_NEG, GT_SELECT_NEGCC))
{
ins = (op2 == nullptr) ? INS_cneg : INS_csneg;
}
else if (tree->OperIs(GT_SELECT_INC, GT_SELECT_INCCC))
{
ins = (op2 == nullptr) ? INS_cinc : INS_csinc;
}

if (tree->OperIs(GT_SELECT, GT_SELECT_INV, GT_SELECT_NEG))
{
opcond = tree->AsConditional()->gtCond;
genConsumeRegs(opcond);
}

emitter* emit = GetEmitter();

GenTree* op1 = tree->gtOp1;
GenTree* op2 = tree->gtOp2;
var_types op1Type = genActualType(op1);
var_types op2Type = genActualType(op2);
emitAttr attr = emitActualTypeSize(tree);
if (op2 != nullptr)
{
var_types op1Type = genActualType(op1);
var_types op2Type = genActualType(op2);
assert(genTypeSize(op1Type) == genTypeSize(op2Type));
}

assert(!op1->isUsedFromMemory());
assert(genTypeSize(op1Type) == genTypeSize(op2Type));

emitter* emit = GetEmitter();
GenCondition cond;

if (opcond != nullptr)
Expand All @@ -4719,92 +4736,62 @@ void CodeGen::genCodeForSelect(GenTreeOp* tree)
}
else
{
assert(tree->OperIs(GT_SELECTCC));
assert(tree->OperIs(GT_SELECTCC, GT_SELECT_INCCC, GT_SELECT_INVCC, GT_SELECT_NEGCC));
cond = tree->AsOpCC()->gtCondition;
}

assert(!op1->isContained() || op1->IsIntegralConst(0));
assert(!op2->isContained() || op2->IsIntegralConst(0));
assert(op2 == nullptr || !op2->isContained() || op2->IsIntegralConst(0));

regNumber targetReg = tree->GetRegNum();
regNumber srcReg1 = op1->IsIntegralConst(0) ? REG_ZR : genConsumeReg(op1);
regNumber srcReg2 = op2->IsIntegralConst(0) ? REG_ZR : genConsumeReg(op2);
const GenConditionDesc& prevDesc = GenConditionDesc::Get(cond);
emitAttr attr = emitActualTypeSize(tree);
regNumber srcReg2;

emit->emitIns_R_R_R_COND(INS_csel, attr, targetReg, srcReg1, srcReg2, JumpKindToInsCond(prevDesc.jumpKind1));

// Some conditions require an additional condition check.
if (prevDesc.oper == GT_OR)
{
emit->emitIns_R_R_R_COND(INS_csel, attr, targetReg, srcReg1, targetReg, JumpKindToInsCond(prevDesc.jumpKind2));
}
else if (prevDesc.oper == GT_AND)
{
emit->emitIns_R_R_R_COND(INS_csel, attr, targetReg, targetReg, srcReg2, JumpKindToInsCond(prevDesc.jumpKind2));
}

regSet.verifyRegUsed(targetReg);
genProduceReg(tree);
}

//------------------------------------------------------------------------
// genCodeForCinc: Produce code for a GT_CINC/GT_CINCCC node.
//
// Arguments:
// tree - the node
//
void CodeGen::genCodeForCinc(GenTreeOp* cinc)
{
assert(cinc->OperIs(GT_CINC, GT_CINCCC));

GenTree* opcond = nullptr;
GenTree* op = cinc->gtOp1;
if (cinc->OperIs(GT_CINC))
if (op2 == nullptr)
{
opcond = cinc->gtOp1;
op = cinc->gtOp2;
genConsumeRegs(opcond);
}

emitter* emit = GetEmitter();
var_types opType = genActualType(op->TypeGet());
emitAttr attr = emitActualTypeSize(cinc->TypeGet());

assert(!op->isUsedFromMemory());
genConsumeRegs(op);

GenCondition cond;

if (cinc->OperIs(GT_CINC))
{
assert(!opcond->isContained());
// Condition has been generated into a register - move it into flags.
emit->emitIns_R_I(INS_cmp, emitActualTypeSize(opcond), opcond->GetRegNum(), 0);
cond = GenCondition::NE;
srcReg2 = srcReg1;
emit->emitIns_R_R_COND(ins, attr, targetReg, srcReg1, JumpKindToInsCond(prevDesc.jumpKind1));
}
else
{
assert(cinc->OperIs(GT_CINCCC));
cond = cinc->AsOpCC()->gtCondition;
srcReg2 = (op2->IsIntegralConst(0) ? REG_ZR : genConsumeReg(op2));
emit->emitIns_R_R_R_COND(ins, attr, targetReg, srcReg1, srcReg2, JumpKindToInsCond(prevDesc.jumpKind1));
}
const GenConditionDesc& prevDesc = GenConditionDesc::Get(cond);
regNumber targetReg = cinc->GetRegNum();
regNumber srcReg;

if (op->isContained())
// Some floating point comparision conditions require an additional condition check.
// These checks are emitted as a subsequent check using GT_AND or GT_OR nodes.
// e.g., using GT_OR => `dest = (cond1 || cond2) ? src1 : src2`
// GT_AND => `dest = (cond1 && cond2) ? src1 : src2`
// The GT_OR case results in emitting the following sequence of two csel instructions.
// csel dest, src1, src2, cond1 # emitted previously
// csel dest, src1, dest, cond2
//
if (prevDesc.oper == GT_AND)
{
assert(op->IsIntegralConst(0));
srcReg = REG_ZR;
// To ensure correctness with invert and negate variants of conditional select, the second instruction needs to
// be csinv or csneg respectively.
// dest = (cond1 && cond2) ? src1 : ~src2
// csinv dest, src1, src2, cond1
// csinv dest, dest, src2, cond2
//
// However, the other variants - increment and select, the second instruction needs to be csel.
// dest = (cond1 && cond2) ? src1 : src2++
// csinc dest, src1, src2, cond1
// csel dest, dest, src1 cond2
ins = ((ins == INS_csinv) || (ins == INS_csneg)) ? ins : INS_csel;
emit->emitIns_R_R_R_COND(ins, attr, targetReg, targetReg, srcReg2, JumpKindToInsCond(prevDesc.jumpKind2));
}
else
else if (prevDesc.oper == GT_OR)
{
srcReg = op->GetRegNum();
// Similarly, the second instruction needs to be csinc while emitting conditional increment.
ins = (ins == INS_csinc) ? ins : INS_csel;
emit->emitIns_R_R_R_COND(ins, attr, targetReg, srcReg1, targetReg, JumpKindToInsCond(prevDesc.jumpKind2));
}

assert(prevDesc.oper != GT_OR && prevDesc.oper != GT_AND);
emit->emitIns_R_R_COND(INS_cinc, attr, targetReg, srcReg, JumpKindToInsCond(prevDesc.jumpKind1));
regSet.verifyRegUsed(targetReg);
genProduceReg(cinc);
genProduceReg(tree);
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -10365,53 +10352,6 @@ void CodeGen::genCodeForBfiz(GenTreeOp* tree)
genProduceReg(tree);
}

//------------------------------------------------------------------------
// genCodeForCond: Generates the code sequence for a GenTree node that
// represents a conditional instruction.
//
// Arguments:
// tree - conditional op
//
void CodeGen::genCodeForCond(GenTreeOp* tree)
{
assert(tree->OperIs(GT_CSNEG_MI, GT_CNEG_LT));
assert(!(tree->gtFlags & GTF_SET_FLAGS));
genConsumeOperands(tree);

switch (tree->OperGet())
{
case GT_CSNEG_MI:
{
instruction ins = INS_csneg;
insCond cond = INS_COND_MI;

regNumber dstReg = tree->GetRegNum();
regNumber op1Reg = tree->gtGetOp1()->GetRegNum();
regNumber op2Reg = tree->gtGetOp2()->GetRegNum();

GetEmitter()->emitIns_R_R_R_COND(ins, emitActualTypeSize(tree), dstReg, op1Reg, op2Reg, cond);
break;
}

case GT_CNEG_LT:
{
instruction ins = INS_cneg;
insCond cond = INS_COND_LT;

regNumber dstReg = tree->GetRegNum();
regNumber op1Reg = tree->gtGetOp1()->GetRegNum();

GetEmitter()->emitIns_R_R_COND(ins, emitActualTypeSize(tree), dstReg, op1Reg, cond);
break;
}

default:
unreached();
}

genProduceReg(tree);
}

//------------------------------------------------------------------------
// JumpKindToInsCond: Convert a Jump Kind to a condition.
//
Expand Down
16 changes: 6 additions & 10 deletions src/coreclr/jit/codegenarmarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
case GT_BFIZ:
genCodeForBfiz(treeNode->AsOp());
break;

case GT_CSNEG_MI:
case GT_CNEG_LT:
genCodeForCond(treeNode->AsOp());
break;
#endif // TARGET_ARM64

case GT_JMP:
Expand Down Expand Up @@ -355,15 +350,16 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
break;

#ifdef TARGET_ARM64
case GT_SELECT_NEG:
case GT_SELECT_INV:
case GT_SELECT_INC:
case GT_SELECT:
genCodeForSelect(treeNode->AsConditional());
break;

case GT_CINC:
case GT_CINCCC:
genCodeForCinc(treeNode->AsOp());
break;

case GT_SELECT_NEGCC:
case GT_SELECT_INVCC:
case GT_SELECT_INCCC:
case GT_SELECTCC:
genCodeForSelect(treeNode->AsOp());
break;
Expand Down
6 changes: 1 addition & 5 deletions src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3984,11 +3984,7 @@ void GenTree::VisitOperands(TVisitor visitor)
}
FALLTHROUGH;

// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
// Standard unary operators
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
case GT_NOT:
Expand Down
37 changes: 22 additions & 15 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6442,11 +6442,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
case GT_IL_OFFSET:
return false;

// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
// Standard unary operators
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
case GT_NOT:
Expand Down Expand Up @@ -6635,7 +6631,11 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
}
return false;
}

#ifdef TARGET_ARM64
case GT_SELECT_NEG:
case GT_SELECT_INV:
case GT_SELECT_INC:
#endif
case GT_SELECT:
{
GenTreeConditional* const conditional = this->AsConditional();
Expand Down Expand Up @@ -9836,11 +9836,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
m_state = -1;
return;

// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
// Standard unary operators
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
case GT_NOT:
Expand Down Expand Up @@ -9952,7 +9948,11 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
m_advance = &GenTreeUseEdgeIterator::AdvanceCall<CALL_ARGS>;
AdvanceCall<CALL_ARGS>();
return;

#ifdef TARGET_ARM64
case GT_SELECT_NEG:
case GT_SELECT_INV:
case GT_SELECT_INC:
#endif
case GT_SELECT:
m_edge = &m_node->AsConditional()->gtCond;
assert(*m_edge != nullptr);
Expand Down Expand Up @@ -10078,8 +10078,15 @@ void GenTreeUseEdgeIterator::AdvanceConditional()
switch (m_state)
{
case 0:
m_edge = &conditional->gtOp1;
m_state = 1;
m_edge = &conditional->gtOp1;
if (conditional->gtOp2 == nullptr)
{
m_advance = &GenTreeUseEdgeIterator::Terminate;
}
else
{
m_state = 1;
}
break;
case 1:
m_edge = &conditional->gtOp2;
Expand Down Expand Up @@ -12361,7 +12368,7 @@ void Compiler::gtDispTree(GenTree* tree,
printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
}
#ifdef TARGET_ARM64
else if (tree->OperIs(GT_CINCCC))
else if (tree->OperIs(GT_SELECT_INCCC, GT_SELECT_INVCC, GT_SELECT_NEGCC))
{
printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
}
Expand Down
Loading