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

Use cinc instead of csel when possible #82031

Merged
merged 12 commits into from
Apr 12, 2023
1 change: 1 addition & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,7 @@ 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
60 changes: 60 additions & 0 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4680,6 +4680,66 @@ void CodeGen::genCodeForSelect(GenTreeOp* tree)
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))
{
opcond = cinc->gtOp1;
op = cinc->gtOp2;
genConsumeRegs(opcond);
}

SwapnilGaikwad marked this conversation as resolved.
Show resolved Hide resolved
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;
}
else
{
assert(cinc->OperIs(GT_CINCCC));
cond = cinc->AsOpCC()->gtCondition;
}
const GenConditionDesc& prevDesc = GenConditionDesc::Get(cond);
regNumber targetReg = cinc->GetRegNum();
regNumber srcReg;

if (op->isContained())
{
assert(op->IsIntegralConst(0));
srcReg = REG_ZR;
}
else
{
srcReg = op->GetRegNum();
}

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);
}

//------------------------------------------------------------------------
// genCodeForJumpCompare: Generates code for jmpCompare statement.
//
Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,15 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
genCodeForSelect(treeNode->AsConditional());
break;

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

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

#ifdef TARGET_ARM64
case GT_JCMP:
genCodeForJumpCompare(treeNode->AsOp());
break;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4154,6 +4154,7 @@ void GenTree::VisitOperands(TVisitor visitor)
// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ void emitter::emitInsSanityCheck(instrDesc* id)
case IF_DR_2D: // DR_2D X..........nnnnn cccc..nnnnnmmmmm Rd Rn cond
assert(isValidGeneralDatasize(id->idOpSize()));
assert(isGeneralRegister(id->idReg1()));
assert(isGeneralRegister(id->idReg2()));
assert(isGeneralRegisterOrZR(id->idReg2()));
assert(isValidImmCond(emitGetInsSC(id)));
break;

Expand Down Expand Up @@ -7315,7 +7315,7 @@ void emitter::emitIns_R_R_COND(instruction ins, emitAttr attr, regNumber reg1, r
case INS_cinv:
case INS_cneg:
assert(isGeneralRegister(reg1));
assert(isGeneralRegister(reg2));
assert(isGeneralRegisterOrZR(reg2));
cfi.cond = cond;
fmt = IF_DR_2D;
break;
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6257,6 +6257,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
Expand Down Expand Up @@ -9507,6 +9508,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
// Standard unary operators
#ifdef TARGET_ARM64
case GT_CNEG_LT:
case GT_CINCCC:
#endif // TARGET_ARM64
case GT_STORE_LCL_VAR:
case GT_STORE_LCL_FLD:
Expand Down Expand Up @@ -12098,6 +12100,10 @@ void Compiler::gtDispTree(GenTree* tree,
printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
}
#ifdef TARGET_ARM64
else if (tree->OperIs(GT_CINCCC))
{
printf(" cond=%s", tree->AsOpCC()->gtCondition.Name());
}
else if (tree->OperIs(GT_CCMP))
{
printf(" cond=%s flags=%s", tree->AsCCMP()->gtCondition.Name(),
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1691,7 +1691,7 @@ struct GenTree
}
#endif
#if defined(TARGET_ARM64)
if (OperIs(GT_CCMP))
if (OperIs(GT_CCMP, GT_CINCCC))
{
return true;
}
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,13 @@ GTNODE(SELECTCC , GenTreeOpCC ,0,GTK_BINOP|DBK_NOTHIR)
// operands and sets the condition flags according to the result. Otherwise
// sets the condition flags to the specified immediate value.
GTNODE(CCMP , GenTreeCCMP ,0,GTK_BINOP|GTK_NOVALUE|DBK_NOTHIR)
// Maps to arm64 cinc instruction. It returns the operand incremented by one when the condition is true.
// Otherwise returns the unchanged operand. Optimises for patterns such as, result = condition ? op1 + 1 : op1
GTNODE(CINC , GenTreeOp ,0,GTK_BINOP|DBK_NOTHIR)
// Variant of CINC that reuses flags computed by a previous node with the specified condition.
GTNODE(CINCCC , GenTreeOpCC ,0,GTK_UNOP|DBK_NOTHIR)
#endif


//-----------------------------------------------------------------------------
// Other nodes that look like unary/binary operators:
//-----------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/jit/gtstructs.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,11 @@ 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)
#ifdef TARGET_ARM64
GTSTRUCT_1(CCMP , GT_CCMP)
GTSTRUCT_2(OpCC , GT_SELECTCC, GT_CINCCC)
#else
GTSTRUCT_1(OpCC , GT_SELECTCC)
#endif
#if defined(TARGET_X86)
GTSTRUCT_1(MultiRegOp , GT_MUL_LONG)
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/liveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2219,9 +2219,8 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange)
return GenTree::VisitResult::Continue;
});

if (node->OperIs(GT_SELECTCC, GT_SETCC))
if (node->OperConsumesFlags() && node->gtPrev->gtSetFlags())
{
assert((node->gtPrev->gtFlags & GTF_SET_FLAGS) != 0);
node->gtPrev->gtFlags &= ~GTF_SET_FLAGS;
}

Expand Down
17 changes: 13 additions & 4 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3830,20 +3830,29 @@ GenTree* Lowering::LowerSelect(GenTreeConditional* select)
// we could do this on x86. We currently disable if-conversion for TYP_LONG
// on 32-bit architectures because of this.
GenCondition selectCond;
GenTreeOpCC* newSelect = nullptr;
if (((select->gtFlags & GTF_SET_FLAGS) == 0) && TryLowerConditionToFlagsNode(select, cond, &selectCond))
{
select->SetOper(GT_SELECTCC);
GenTreeOpCC* newSelect = select->AsOpCC();
newSelect = select->AsOpCC();
newSelect->gtCondition = selectCond;
ContainCheckSelect(newSelect);
JITDUMP("Converted to SELECTCC:\n");
DISPTREERANGE(BlockRange(), newSelect);
JITDUMP("\n");
return newSelect->gtNext;
}
else
{
ContainCheckSelect(select);
}

ContainCheckSelect(select);
return select->gtNext;
#ifdef TARGET_ARM64
if (trueVal->IsCnsIntOrI() && falseVal->IsCnsIntOrI())
{
TryLowerCselToCinc(select, cond);
}
#endif
return newSelect != nullptr ? newSelect->gtNext : select->gtNext;
}

//----------------------------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class Lowering final : public Phase
insCflags TruthifyingFlags(GenCondition cond);
void ContainCheckConditionalCompare(GenTreeCCMP* ccmp);
void ContainCheckNeg(GenTreeOp* neg);
void TryLowerCselToCinc(GenTreeOp* select, GenTree* cond);
#endif
void ContainCheckSelect(GenTreeOp* select);
void ContainCheckBitCast(GenTree* node);
Expand Down
61 changes: 61 additions & 0 deletions src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2495,6 +2495,67 @@ void Lowering::ContainCheckNeg(GenTreeOp* neg)
}
}
}

//----------------------------------------------------------------------------------------------
// Try converting SELECT/SELECTCC to CINC/CINCCC. Conversion is possible only if
// both the trueVal and falseVal are integral constants and abs(trueVal - falseVal) = 1.
//
// Arguments:
// select - The select node that is now SELECT or SELECTCC
// cond - The condition node that SELECT or SELECTCC uses
//
void Lowering::TryLowerCselToCinc(GenTreeOp* select, GenTree* cond)
{
assert(select->OperIs(GT_SELECT, GT_SELECTCC));

GenTree* trueVal = select->gtOp1;
GenTree* falseVal = select->gtOp2;
size_t op1Val = (size_t)trueVal->AsIntCon()->IconValue();
size_t op2Val = (size_t)falseVal->AsIntCon()->IconValue();

if (op1Val + 1 == op2Val || op2Val + 1 == op1Val)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit for future PRs: we parenthesize all subexpressions: https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/clr-jit-coding-conventions.md#111-logical-and-arithmetic-expressions

Let's not rerun CI to fix this, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks. I will piggyback these changes along with the next related patch (probably cinv).

{
const bool shouldReverseCondition = op1Val + 1 == op2Val;

// Create a cinc node, insert it and update the use.
if (select->OperIs(GT_SELECT))
{
if (shouldReverseCondition)
{
// Reverse the condition so that op2 will be selected
if (!cond->OperIsCompare())
{
// Non-compare nodes add additional GT_NOT node after reversing.
// This would remove gains from this optimisation so don't proceed.
return;
}
select->gtOp2 = select->gtOp1;
GenTree* revCond = comp->gtReverseCond(cond);
assert(cond == revCond); // Ensure `gtReverseCond` did not create a new node.
}
select->gtOp1 = cond->AsOp();
select->SetOper(GT_CINC);
DISPTREERANGE(BlockRange(), select);
}
else
{
GenTreeOpCC* selectcc = select->AsOpCC();
GenCondition selectCond = selectcc->gtCondition;
if (shouldReverseCondition)
{
// Reverse the condition so that op2 will be selected
selectcc->gtCondition = GenCondition::Reverse(selectCond);
}
else
{
std::swap(selectcc->gtOp1, selectcc->gtOp2);
}
BlockRange().Remove(selectcc->gtOp2);
selectcc->SetOper(GT_CINCCC);
DISPTREERANGE(BlockRange(), selectcc);
}
}
}
#endif // TARGET_ARM64

//------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/tests/JIT/opt/Compares/compareAnd3Chains.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public static void Eq_else_long_3_consume(long a1, long a2, long a3) {
//ARM64-FULL-LINE-NEXT: ccmp {{x[0-9]+}}, #21, 0, {{eq|ne}}
//ARM64-FULL-LINE-NEXT: ccmp {{x[0-9]+}}, #19, z, {{eq|ne}}
//ARM64-FULL-LINE-NEXT: csel {{x[0-9]+}}, {{x[0-9]+}}, {{x[0-9]+}}, {{eq|ne}}
if (a1 == 19 || a2 == 20 && a3 == 21) { a1 = 10; } else { a1 = 11; }
if (a1 == 19 || a2 == 20 && a3 == 21) { a1 = 10; } else { a1 = 12; }
consume<long>(a1, a2, a3);
}

Expand Down
Loading