Skip to content

Commit

Permalink
Add optimisation of incrementing SELECT at lowering on arm64.
Browse files Browse the repository at this point in the history 
This patch adds support for optimising SELECT or SELECT_CC nodes
which contain a child of type GT_ADD with a second operand constant
value of 1 (increment) to be optimised into a SELECT_INC or
SELECT_INCCC node.

Change-Id: Ia26da6f4c0e3a75143e133964cbb76243d0822de
  • Loading branch information
@c272
c272 committed Aug 31, 2023
1 parent c4f8c3b commit 83adbd6
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 41 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/jit/lower.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3851,13 +3851,13 @@ GenTree* Lowering::LowerSelect(GenTreeConditional* select)
}

#ifdef TARGET_ARM64
if (trueVal->OperIs(GT_NOT, GT_NEG) || falseVal->OperIs(GT_NOT, GT_NEG))
if (trueVal->OperIs(GT_NOT, GT_NEG, GT_ADD) || falseVal->OperIs(GT_NOT, GT_NEG, GT_ADD))
{
TryLowerCselToCinvOrCneg(select, cond);
TryLowerCselToCSOp(select, cond);
}
else if (trueVal->IsCnsIntOrI() && falseVal->IsCnsIntOrI())
{
TryLowerCselToCinc(select, cond);
TryLowerCnsIntCselToCinc(select, cond);
}
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/lower.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ class Lowering final : public Phase
insCflags TruthifyingFlags(GenCondition cond);
void ContainCheckConditionalCompare(GenTreeCCMP* ccmp);
void ContainCheckNeg(GenTreeOp* neg);
void TryLowerCselToCinc(GenTreeOp* select, GenTree* cond);
void TryLowerCselToCinvOrCneg(GenTreeOp* select, GenTree* cond);
void TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond);
void TryLowerCselToCSOp(GenTreeOp* select, GenTree* cond);
#endif
void ContainCheckSelect(GenTreeOp* select);
void ContainCheckBitCast(GenTree* node);
Expand Down
121 changes: 85 additions & 36 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2545,41 +2545,55 @@ void Lowering::ContainCheckNeg(GenTreeOp* neg)
}

//----------------------------------------------------------------------------------------------
// TryLowerCselToCinvOrCneg: Try converting SELECT/SELECTCC to SELECT_INV/SELECT_INVCC. Conversion is possible only if
// one of the operands of the select node is inverted.
// TryLowerCselToCSOp: Try converting SELECT/SELECTCC to SELECT_?/SELECT_?CC. Conversion is possible only if
// one of the operands of the select node is one of GT_NEG, GT_NOT or GT_ADD.
//
// Arguments:
// select - The select node that is now SELECT or SELECTCC
// cond - The condition node that SELECT or SELECTCC uses
//
void Lowering::TryLowerCselToCinvOrCneg(GenTreeOp* select, GenTree* cond)
void Lowering::TryLowerCselToCSOp(GenTreeOp* select, GenTree* cond)
{
assert(select->OperIs(GT_SELECT, GT_SELECTCC));

bool shouldReverseCondition;
GenTree* invertedOrNegatedVal;
GenTree* nonInvertedOrNegatedVal;
GenTree* operatedVal;
GenTree* nonOperatedVal;
GenTree* nodeToRemove;
GenTree* trueVal = select->gtOp1;
GenTree* falseVal = select->gtOp2;

GenTree* trueVal = select->gtOp1;
GenTree* falseVal = select->gtOp2;
const bool isCneg = trueVal->OperIs(GT_NEG) || falseVal->OperIs(GT_NEG);

assert(trueVal->OperIs(GT_NOT, GT_NEG) || falseVal->OperIs(GT_NOT, GT_NEG));
// Determine the resulting operation type.
genTreeOps resultingOp;
if (trueVal->OperIs(GT_NEG) || falseVal->OperIs(GT_NEG))
{
resultingOp = GT_SELECT_NEG;
shouldReverseCondition = trueVal->OperIs(GT_NEG);
}
else if (trueVal->OperIs(GT_NOT) || falseVal->OperIs(GT_NOT))
{
resultingOp = GT_SELECT_INV;
shouldReverseCondition = trueVal->OperIs(GT_NOT);
}
else
{
assert(trueVal->OperIs(GT_ADD) || falseVal->OperIs(GT_ADD));
resultingOp = GT_SELECT_INC;
shouldReverseCondition = trueVal->OperIs(GT_ADD);
}

if ((isCneg && trueVal->OperIs(GT_NEG)) || (!isCneg && trueVal->OperIs(GT_NOT)))
// Values to which the operation are applied must come last.
if (shouldReverseCondition)
{
shouldReverseCondition = true;
invertedOrNegatedVal = trueVal->gtGetOp1();
nonInvertedOrNegatedVal = falseVal;
nodeToRemove = trueVal;
operatedVal = trueVal->gtGetOp1();
nonOperatedVal = falseVal;
nodeToRemove = trueVal;
}
else
{
shouldReverseCondition = false;
invertedOrNegatedVal = falseVal->gtGetOp1();
nonInvertedOrNegatedVal = trueVal;
nodeToRemove = falseVal;
operatedVal = falseVal->gtGetOp1();
nonOperatedVal = trueVal;
nodeToRemove = falseVal;
}

if (shouldReverseCondition && !cond->OperIsCompare() && select->OperIs(GT_SELECT))
Expand All @@ -2589,17 +2603,33 @@ void Lowering::TryLowerCselToCinvOrCneg(GenTreeOp* select, GenTree* cond)
return;
}

if (!(IsInvariantInRange(invertedOrNegatedVal, select) && IsInvariantInRange(nonInvertedOrNegatedVal, select)))
// For Csinc candidates, the second argument of the GT_ADD must be +1 (increment).
if (resultingOp == GT_SELECT_INC &&
!(nodeToRemove->gtGetOp2()->IsCnsIntOrI() && nodeToRemove->gtGetOp2()->AsIntCon()->IconValue() == 1))
{
return;
}

// As the select node would handle the negation/inversion, the op is not required.
// If a value is contained in the negate/invert op, it cannot be contained anymore.
// Check that we are safe to move both values.
if (!(IsInvariantInRange(operatedVal, select) && IsInvariantInRange(nonOperatedVal, select)))
{
return;
}

// Passed all checks, move on to block modification.
// If this is a Cinc candidate, we must remove the dangling second argument node.
if (resultingOp == GT_SELECT_INC)
{
BlockRange().Remove(nodeToRemove->gtGetOp2());
nodeToRemove->AsOp()->gtOp2 = nullptr;
}

// As the select node would handle the operation, the op is not required.
// If a value is contained in the negate/invert/increment op, it cannot be contained anymore.
BlockRange().Remove(nodeToRemove);
invertedOrNegatedVal->ClearContained();
select->gtOp1 = nonInvertedOrNegatedVal;
select->gtOp2 = invertedOrNegatedVal;
operatedVal->ClearContained();
select->gtOp1 = nonOperatedVal;
select->gtOp2 = operatedVal;

if (select->OperIs(GT_SELECT))
{
Expand All @@ -2608,10 +2638,7 @@ void Lowering::TryLowerCselToCinvOrCneg(GenTreeOp* select, GenTree* cond)
GenTree* revCond = comp->gtReverseCond(cond);
assert(cond == revCond); // Ensure `gtReverseCond` did not create a new node.
}
select->SetOper(isCneg ? GT_SELECT_NEG : GT_SELECT_INV);
JITDUMP("Converted to: %s\n", isCneg ? "SELECT_NEG" : "SELECT_INV");
DISPTREERANGE(BlockRange(), select);
JITDUMP("\n");
select->SetOper(resultingOp);
}
else
{
Expand All @@ -2622,22 +2649,44 @@ void Lowering::TryLowerCselToCinvOrCneg(GenTreeOp* select, GenTree* cond)
// Reverse the condition so that op2 will be selected
selectcc->gtCondition = GenCondition::Reverse(selectCond);
}
selectcc->SetOper(isCneg ? GT_SELECT_NEGCC : GT_SELECT_INVCC);
JITDUMP("Converted to: %s\n", isCneg ? "SELECT_NEGCC" : "SELECT_INVCC");
DISPTREERANGE(BlockRange(), selectcc);
JITDUMP("\n");

// Convert the resulting operation into the equivalent CC form.
switch (resultingOp)
{
case GT_SELECT_NEG:
resultingOp = GT_SELECT_NEGCC;
break;
case GT_SELECT_INV:
resultingOp = GT_SELECT_INVCC;
break;
case GT_SELECT_INC:
resultingOp = GT_SELECT_INCCC;
break;
default:
assert(false);
}
selectcc->SetOper(resultingOp);
}

#ifdef DEBUG
JITDUMP("Converted to ");
if (comp->verbose)
comp->gtDispNodeName(select);
JITDUMP(":\n");
DISPTREERANGE(BlockRange(), select);
JITDUMP("\n");
#endif
}

//----------------------------------------------------------------------------------------------
// TryLowerCselToCinc: Try converting SELECT/SELECTCC to SELECT_INC/SELECT_INCCC. Conversion is possible only if both
// the trueVal and falseVal are integral constants and abs(trueVal - falseVal) = 1.
// TryLowerCnsIntCselToCinc: Try converting SELECT/SELECTCC to SELECT_INC/SELECT_INCCC.
// Conversion is possible only if both the trueVal and falseVal are integer 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)
void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond)
{
assert(select->OperIs(GT_SELECT, GT_SELECTCC));

Expand Down
132 changes: 132 additions & 0 deletions src/tests/JIT/opt/Compares/conditionalIncrements.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,30 @@ public static void cinc_byte(byte op1, int expected)
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_byte(byte op1, int expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
int result = op1 <= 44 ? op1 + 1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_byte_reverse(byte op1, int expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
int result = op1 <= 44 ? 1 + op1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(72, byte.MinValue)]
[InlineData(32, byte.MaxValue)]
Expand Down Expand Up @@ -58,6 +82,30 @@ public static void cinc_short(short op1, short expected)
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_short(short op1, short expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
short result = (short) (op1 <= 44 ? op1 + 1 : 5);
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_short_reverse(short op1, short expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
short result = (short) (op1 <= 44 ? 1 + op1 : 5);
Assert.Equal(expected, result);
}

[Theory]
[InlineData(76, short.MinValue)]
[InlineData(-35, short.MaxValue)]
Expand All @@ -82,6 +130,66 @@ public static void cinc_int(int op1, int expected)
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_int(int op1, int expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
int result = op1 <= 44 ? op1 + 1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_int_reverse(int op1, int expected)
{
//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{gt|le}}
int result = op1 <= 44 ? 1 + op1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 69)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_int_postinc(int op1, int expected)
{
int result = 2 * op1;

//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{ge|lt}}
if (op1 < 44) {
result++;
} else {
result = 5;
}
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 149)]
[InlineData(34, 5)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_int_postinc_false(int op1, int expected)
{
int result = 2 * op1;

//ARM64-FULL-LINE: cmp {{w[0-9]+}}, #44
//ARM64-FULL-LINE-NEXT: csinc {{w[0-9]+}}, {{w[0-9]+}}, {{w[0-9]+}}, {{ge|lt}}
if (op1 < 44) {
result = 5;
} else {
result++;
}
Assert.Equal(expected, result);
}

[Theory]
[InlineData(77, int.MinValue)]
[InlineData(37, int.MaxValue)]
Expand All @@ -107,6 +215,30 @@ public static void cinc_long(long op1, long expected)
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_long(long op1, long expected)
{
//ARM64-FULL-LINE: cmp {{x[0-9]+}}, #48
//ARM64-FULL-LINE-NEXT: csinc {{x[0-9]+}}, {{x[0-9]+}}, {{x[0-9]+}}, {{gt|le}}
long result = op1 <= 48 ? op1 + 1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(74, 5)]
[InlineData(34, 35)]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void csinc_long_reverse(long op1, long expected)
{
//ARM64-FULL-LINE: cmp {{x[0-9]+}}, #48
//ARM64-FULL-LINE-NEXT: csinc {{x[0-9]+}}, {{x[0-9]+}}, {{x[0-9]+}}, {{gt|le}}
long result = op1 <= 48 ? 1 + op1 : 5;
Assert.Equal(expected, result);
}

[Theory]
[InlineData(79, long.MaxValue)]
[InlineData(39, long.MinValue)]
Expand Down

0 comments on commit 83adbd6

Please sign in to comment.