From 0b650c683386dbb01dadb20b1ccf33c1e0eafeff Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Thu, 8 Jan 2026 16:43:08 -0800 Subject: [PATCH 1/7] initial impl --- src/coreclr/jit/lower.cpp | 2 +- src/coreclr/jit/lowerarmarch.cpp | 137 +++++++++++++++++++++++-------- 2 files changed, 103 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index da8e8fb96f793d..426025f311c44f 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4846,7 +4846,7 @@ GenTree* Lowering::LowerSelect(GenTreeConditional* select) { TryLowerCselToCSOp(select, cond); } - else if (trueVal->IsCnsIntOrI() && falseVal->IsCnsIntOrI()) + else if (trueVal->IsCnsIntOrI() || falseVal->IsCnsIntOrI()) { TryLowerCnsIntCselToCinc(select, cond); } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index c8f677347601cc..e9eb82cd085d23 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3454,7 +3454,8 @@ void Lowering::TryLowerCselToCSOp(GenTreeOp* select, GenTree* cond) //---------------------------------------------------------------------------------------------- // 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. +// Conversion is possible if both the trueVal and falseVal are integer constants and abs(trueVal - falseVal) = 1, +// or if one of trueVal/falseVal is 1 greater than the value being compared against // // Arguments: // select - The select node that is now SELECT or SELECTCC @@ -3466,54 +3467,120 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) 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)) + if (trueVal->IsCnsIntOrI() && falseVal->IsCnsIntOrI()) { - const bool shouldReverseCondition = (op1Val + 1 == op2Val); + size_t op1Val = (size_t)trueVal->AsIntCon()->IconValue(); + size_t op2Val = (size_t)falseVal->AsIntCon()->IconValue(); - if (select->OperIs(GT_SELECT)) + if ((op1Val + 1 == op2Val) || (op2Val + 1 == op1Val)) { - if (shouldReverseCondition) + const bool shouldReverseCondition = (op1Val + 1 == op2Val); + + if (select->OperIs(GT_SELECT)) { - // Reverse the condition so that op2 will be selected - if (!cond->OperIsCompare()) + if (shouldReverseCondition) { - // Non-compare nodes add additional GT_NOT node after reversing. - // This would remove gains from this optimisation so don't proceed. - return; + // 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; + } + GenTree* revCond = comp->gtReverseCond(cond); + assert(cond == revCond); // Ensure `gtReverseCond` did not create a new node. } - GenTree* revCond = comp->gtReverseCond(cond); - assert(cond == revCond); // Ensure `gtReverseCond` did not create a new node. + BlockRange().Remove(select->gtOp2, true); + select->gtOp2 = nullptr; + select->SetOper(GT_SELECT_INC); + JITDUMP("Converted to: GT_SELECT_INC\n"); + DISPTREERANGE(BlockRange(), select); + JITDUMP("\n"); } - BlockRange().Remove(select->gtOp2, true); - select->gtOp2 = nullptr; - select->SetOper(GT_SELECT_INC); - JITDUMP("Converted to: GT_SELECT_INC\n"); - DISPTREERANGE(BlockRange(), select); - JITDUMP("\n"); + 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, true); + selectcc->gtOp2 = nullptr; + selectcc->SetOper(GT_SELECT_INCCC); + JITDUMP("Converted to: GT_SELECT_INCCC\n"); + DISPTREERANGE(BlockRange(), selectcc); + JITDUMP("\n"); + } + } + } + else + { + // Look for cases like this to convert to conditional increment + // if (myvar==0) myvar = 1; + // If we're comparing a local to a constant int, and reassigning that local to a value that is 1 larger + if (!cond->gtGetOp1()->IsIntegralConst() && !cond->gtGetOp2()->IsIntegralConst()) + { + return; + } + if (!cond->gtGetOp1()->OperIs(GT_LCL_VAR) && !cond->gtGetOp2()->OperIs(GT_LCL_VAR)) + { + return; + } + + // One of the CMP operands is a constant int + auto code = select->AsOpCC()->gtCondition.GetCode(); + int64_t constVal = 0; + unsigned lclNum = 0; + + if (cond->gtGetOp1()->IsIntegralConst()) + { + constVal = cond->gtGetOp1()->AsIntCon()->IntegralValue(); + lclNum = cond->gtGetOp2()->AsLclVar()->GetLclNum(); } else { - GenTreeOpCC* selectcc = select->AsOpCC(); - GenCondition selectCond = selectcc->gtCondition; + constVal = cond->gtGetOp2()->AsIntCon()->IntegralValue(); + lclNum = cond->gtGetOp1()->AsLclVar()->GetLclNum(); + } - if (shouldReverseCondition) - { - // Reverse the condition so that op2 will be selected - selectcc->gtCondition = GenCondition::Reverse(selectCond); - } - else - { - std::swap(selectcc->gtOp1, selectcc->gtOp2); - } + if (code != GenCondition::EQ && code != GenCondition::NE) + { + return; + } + + GenTree** replaceOperand = nullptr; + GenTree* removeVal = nullptr; + if (code == GenCondition::NE && falseVal->IsIntegralConst() && + falseVal->AsIntCon()->IntegralValue() - constVal == 1) + { + removeVal = falseVal; + replaceOperand = &(select->gtOp2); + } + else if (code == GenCondition::EQ && trueVal->IsIntegralConst() && + trueVal->AsIntCon()->IntegralValue() - constVal == 1) + { + removeVal = trueVal; + replaceOperand = &(select->gtOp1); + } + if (removeVal) + { + GenTree* newLocal = comp->gtNewLclvNode(lclNum, removeVal->TypeGet()); + BlockRange().InsertBefore(removeVal, newLocal); + BlockRange().Remove(removeVal); + *replaceOperand = newLocal; + select->SetOper(GT_SELECT_INCCC); - BlockRange().Remove(selectcc->gtOp2, true); - selectcc->gtOp2 = nullptr; - selectcc->SetOper(GT_SELECT_INCCC); JITDUMP("Converted to: GT_SELECT_INCCC\n"); - DISPTREERANGE(BlockRange(), selectcc); + DISPTREERANGE(BlockRange(), select); JITDUMP("\n"); } } From c8a948506730a7fda3dd8c15cb034e4bed13554c Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Fri, 9 Jan 2026 17:25:57 -0800 Subject: [PATCH 2/7] fix build break --- src/coreclr/jit/lowerarmarch.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index e9eb82cd085d23..82499f07fad020 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3524,6 +3524,10 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) } else { + if (!cond->OperIs(GT_CMP)) + { + return; + } // Look for cases like this to convert to conditional increment // if (myvar==0) myvar = 1; // If we're comparing a local to a constant int, and reassigning that local to a value that is 1 larger From 67a786b55b33186775d1f4e1b889d0164b593038 Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Fri, 16 Jan 2026 17:17:03 -0800 Subject: [PATCH 3/7] reverse condcode if needed --- src/coreclr/jit/lowerarmarch.cpp | 48 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 82499f07fad020..91fd2053468e80 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3560,33 +3560,39 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) { return; } - - GenTree** replaceOperand = nullptr; - GenTree* removeVal = nullptr; - if (code == GenCondition::NE && falseVal->IsIntegralConst() && - falseVal->AsIntCon()->IntegralValue() - constVal == 1) + if (code == GenCondition::EQ && + (!trueVal->IsIntegralConst() || trueVal->AsIntCon()->IntegralValue() - constVal != 1)) { - removeVal = falseVal; - replaceOperand = &(select->gtOp2); + return; } - else if (code == GenCondition::EQ && trueVal->IsIntegralConst() && - trueVal->AsIntCon()->IntegralValue() - constVal == 1) + if (code == GenCondition::NE && + (!falseVal->IsIntegralConst() || falseVal->AsIntCon()->IntegralValue() - constVal != 1)) { - removeVal = trueVal; - replaceOperand = &(select->gtOp1); + return; } - if (removeVal) - { - GenTree* newLocal = comp->gtNewLclvNode(lclNum, removeVal->TypeGet()); - BlockRange().InsertBefore(removeVal, newLocal); - BlockRange().Remove(removeVal); - *replaceOperand = newLocal; - select->SetOper(GT_SELECT_INCCC); - JITDUMP("Converted to: GT_SELECT_INCCC\n"); - DISPTREERANGE(BlockRange(), select); - JITDUMP("\n"); + GenTree** replaceOperand = nullptr; + GenTree* removeVal = nullptr; + + // The increment needs to occur along the false path, + // reverse condition if that's not the case + if (code == GenCondition::EQ) + { + GenTreeOpCC* selectcc = select->AsOpCC(); + selectcc->gtCondition = GenCondition::Reverse(selectcc->gtCondition); + std::swap(selectcc->gtOp1, selectcc->gtOp2); + falseVal = selectcc->gtOp2; } + + GenTree* newLocal = comp->gtNewLclvNode(lclNum, falseVal->TypeGet()); + BlockRange().InsertBefore(falseVal, newLocal); + BlockRange().Remove(falseVal); + select->gtOp2 = newLocal; + select->SetOper(GT_SELECT_INCCC); + + JITDUMP("Converted to: GT_SELECT_INCCC\n"); + DISPTREERANGE(BlockRange(), select); + JITDUMP("\n"); } } From d9212f464d9c118f8dade52beb6df2b5cd9edde1 Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Tue, 20 Jan 2026 11:32:31 -0800 Subject: [PATCH 4/7] some code cleanup --- src/coreclr/jit/lowerarmarch.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 91fd2053468e80..dffe64ba24fdc8 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3524,18 +3524,16 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) } else { - if (!cond->OperIs(GT_CMP)) - { - return; - } // Look for cases like this to convert to conditional increment // if (myvar==0) myvar = 1; - // If we're comparing a local to a constant int, and reassigning that local to a value that is 1 larger - if (!cond->gtGetOp1()->IsIntegralConst() && !cond->gtGetOp2()->IsIntegralConst()) + // If we're comparing a local to a constant int, and branch has a use of the value+1 + + if (!cond->OperIs(GT_CMP)) { return; } - if (!cond->gtGetOp1()->OperIs(GT_LCL_VAR) && !cond->gtGetOp2()->OperIs(GT_LCL_VAR)) + if ((!cond->gtGetOp1()->IsIntegralConst() && !cond->gtGetOp2()->IsIntegralConst()) || + (!cond->gtGetOp1()->OperIs(GT_LCL_VAR) && !cond->gtGetOp2()->OperIs(GT_LCL_VAR))) { return; } @@ -3560,6 +3558,7 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) { return; } + // Look for constVal+1 along the branch where the local is known to be == constVal if (code == GenCondition::EQ && (!trueVal->IsIntegralConst() || trueVal->AsIntCon()->IntegralValue() - constVal != 1)) { From b10f0e887539b1473acb66e504a0fc883235cfdb Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Thu, 22 Jan 2026 16:02:07 -0800 Subject: [PATCH 5/7] unneeded locals --- src/coreclr/jit/lowerarmarch.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index dffe64ba24fdc8..6dd804e9e0beec 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3570,9 +3570,6 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) return; } - GenTree** replaceOperand = nullptr; - GenTree* removeVal = nullptr; - // The increment needs to occur along the false path, // reverse condition if that's not the case if (code == GenCondition::EQ) From 5bfce98d065a6e5c069922cd85c20537844b16c9 Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Tue, 27 Jan 2026 14:34:13 -0800 Subject: [PATCH 6/7] pr feedback --- src/coreclr/jit/lowerarmarch.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 6dd804e9e0beec..f09827f543c667 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3528,7 +3528,8 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) // if (myvar==0) myvar = 1; // If we're comparing a local to a constant int, and branch has a use of the value+1 - if (!cond->OperIs(GT_CMP)) + if (!cond->OperIs(GT_CMP) || + !select->OperIs(GT_SELECTCC)) { return; } @@ -3539,9 +3540,9 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) } // One of the CMP operands is a constant int - auto code = select->AsOpCC()->gtCondition.GetCode(); - int64_t constVal = 0; - unsigned lclNum = 0; + GenCondition::Code code = select->AsOpCC()->gtCondition.GetCode(); + int64_t constVal = 0; + unsigned lclNum = 0; if (cond->gtGetOp1()->IsIntegralConst()) { From 1624d870914219119fde561501d382ba61b8b65b Mon Sep 17 00:00:00 2001 From: David Hartglass Date: Tue, 27 Jan 2026 16:32:26 -0800 Subject: [PATCH 7/7] fix formatting --- src/coreclr/jit/lowerarmarch.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index f09827f543c667..8f9b756e529884 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -3528,8 +3528,7 @@ void Lowering::TryLowerCnsIntCselToCinc(GenTreeOp* select, GenTree* cond) // if (myvar==0) myvar = 1; // If we're comparing a local to a constant int, and branch has a use of the value+1 - if (!cond->OperIs(GT_CMP) || - !select->OperIs(GT_SELECTCC)) + if (!cond->OperIs(GT_CMP) || !select->OperIs(GT_SELECTCC)) { return; }