From ef89aebea05686ac538468e3a29b36c39cfc5447 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Sat, 14 Mar 2026 05:27:36 +0100 Subject: [PATCH 1/5] * handle TYP_INT only in Lower (for now?) --- src/coreclr/jit/lower.cpp | 112 ++++++++++++++++++++++++++++++++++++++ src/coreclr/jit/lower.h | 1 + 2 files changed, 113 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index cbd4e5e814d4d3..d1fcde5c3a917c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4814,6 +4814,113 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) } #endif // !TARGET_LOONGARCH64 && !TARGET_RISCV64 && !defined(TARGET_WASM) +//---------------------------------------------------------------------------------------------- +// TryLowerSelectToSarAdd: Try to optimize: +// SELECT(x < 0, C - 1, C) -> SAR(x, 31) + C +// SAR(x, 31) gives us either -1 or 0 dependig on if x was negative or not. +// The +C then just shifts to the wanted range. +// +// Arguments: +// select - The SELECT node +// cond - The condition +// trueVal - Value when condition is true +// falseVal - Value when condition is false +// +// Return Value: +// Next node if the optimization was applied, nullptr otherwise. +// +GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, + GenTree* cond, + GenTree* trueVal, + GenTree* falseVal) +{ + if (!select->TypeIs(TYP_INT)) + { + return nullptr; + } + + if (!trueVal->IsIntegralConst() || !falseVal->IsIntegralConst()) + { + return nullptr; + } + + if (!cond->OperIsCompare()) + { + return nullptr; + } + + GenTree* relopOp1 = cond->gtGetOp1(); + GenTree* relopOp2 = cond->gtGetOp2(); + + if (!relopOp2->IsIntegralConst(0)) + { + return nullptr; + } + + int32_t trueValConst = static_cast(trueVal->AsIntConCommon()->IntegralValue()); + int32_t falseValConst = static_cast(falseVal->AsIntConCommon()->IntegralValue()); + + if (trueValConst - falseValConst == -1) + { + // x < 0 ? C-1 : C + if (!cond->OperIs(GT_LT)) + { + return nullptr; + } + } + else if (trueValConst - falseValConst == 1) + { + // x >= 0 ? C : C-1 + if (!cond->OperIs(GT_GE)) + { + return nullptr; + } + + std::swap(trueValConst, falseValConst); + } + else + { + return nullptr; + } + + LIR::Use use; + if (!BlockRange().TryGetUse(select, &use)) + { + return nullptr; + } + + BlockRange().Remove(relopOp2); + BlockRange().Remove(cond); + BlockRange().Remove(trueVal); + BlockRange().Remove(falseVal); + BlockRange().Remove(select); + + relopOp1->ClearContained(); + + GenTree* shiftConst = m_compiler->gtNewIconNode(31, TYP_INT); + GenTree* sar = m_compiler->gtNewOperNode(GT_RSH, select->TypeGet(), relopOp1, shiftConst); + shiftConst->SetContained(); + + BlockRange().InsertAfter(relopOp1, shiftConst); + BlockRange().InsertAfter(shiftConst, sar); + + GenTree* replacement = sar; + bool addRequired = falseValConst != 0; + if (addRequired) + { + GenTree* addConst = m_compiler->gtNewIconNode(falseValConst, select->TypeGet()); + GenTree* add = m_compiler->gtNewOperNode(GT_ADD, select->TypeGet(), sar, addConst); + addConst->SetContained(); + + BlockRange().InsertAfter(sar, addConst); + BlockRange().InsertAfter(addConst, add); + replacement = add; + } + + use.ReplaceWith(replacement); + return replacement->gtNext; +} + //---------------------------------------------------------------------------------------------- // LowerSelect: Lower a GT_SELECT node. // @@ -4829,6 +4936,11 @@ GenTree* Lowering::LowerSelect(GenTreeConditional* select) GenTree* trueVal = select->gtOp1; GenTree* falseVal = select->gtOp2; + if (GenTree* next = TryLowerSelectToSarAdd(select, cond, trueVal, falseVal); next != nullptr) + { + return next; + } + // Replace SELECT cond 1/0 0/1 with (perhaps reversed) cond if (cond->OperIsCompare() && ((trueVal->IsIntegralConst(0) && falseVal->IsIntegralConst(1)) || (trueVal->IsIntegralConst(1) && falseVal->IsIntegralConst(0)))) diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index b135a9a3109e1c..9b1d444c6bd537 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -164,6 +164,7 @@ class Lowering final : public Phase GenTree* LowerSavedIntegerCompare(GenTree* cmp); void SignExtendIfNecessary(GenTree** arg); #endif + GenTree* TryLowerSelectToSarAdd(GenTreeConditional* select, GenTree* cond, GenTree* trueVal, GenTree* falseVal); GenTree* LowerSelect(GenTreeConditional* cond); bool TryLowerConditionToFlagsNode(GenTree* parent, GenTree* condition, From ea86107c67b451bfd4c68dbdae0d37692bc812e5 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Sat, 14 Mar 2026 18:08:25 +0100 Subject: [PATCH 2/5] check if immed can be contained --- src/coreclr/jit/lower.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index d1fcde5c3a917c..575945a58b3cfd 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4899,7 +4899,7 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, GenTree* shiftConst = m_compiler->gtNewIconNode(31, TYP_INT); GenTree* sar = m_compiler->gtNewOperNode(GT_RSH, select->TypeGet(), relopOp1, shiftConst); - shiftConst->SetContained(); + CheckImmedAndMakeContained(sar, shiftConst); BlockRange().InsertAfter(relopOp1, shiftConst); BlockRange().InsertAfter(shiftConst, sar); @@ -4910,7 +4910,7 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, { GenTree* addConst = m_compiler->gtNewIconNode(falseValConst, select->TypeGet()); GenTree* add = m_compiler->gtNewOperNode(GT_ADD, select->TypeGet(), sar, addConst); - addConst->SetContained(); + CheckImmedAndMakeContained(add, addConst); BlockRange().InsertAfter(sar, addConst); BlockRange().InsertAfter(addConst, add); From f6e57c016cee1580be6e7a6b9e4d71ff4cf28ee2 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Sat, 14 Mar 2026 23:27:14 +0100 Subject: [PATCH 3/5] * switch from CheckImmedAndMakeContained to ContainCheckNode as it didn't contain the 31 in SAR(x, 31) on arm64 for some reason --- src/coreclr/jit/lower.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 575945a58b3cfd..140a064c684e16 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4899,7 +4899,7 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, GenTree* shiftConst = m_compiler->gtNewIconNode(31, TYP_INT); GenTree* sar = m_compiler->gtNewOperNode(GT_RSH, select->TypeGet(), relopOp1, shiftConst); - CheckImmedAndMakeContained(sar, shiftConst); + ContainCheckNode(sar); BlockRange().InsertAfter(relopOp1, shiftConst); BlockRange().InsertAfter(shiftConst, sar); @@ -4910,7 +4910,7 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, { GenTree* addConst = m_compiler->gtNewIconNode(falseValConst, select->TypeGet()); GenTree* add = m_compiler->gtNewOperNode(GT_ADD, select->TypeGet(), sar, addConst); - CheckImmedAndMakeContained(add, addConst); + ContainCheckNode(add); BlockRange().InsertAfter(sar, addConst); BlockRange().InsertAfter(addConst, add); From 30424d956c30e0767fc405f913f4eb82e6e65c37 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Sun, 15 Mar 2026 10:06:04 +0100 Subject: [PATCH 4/5] fix CI '(cur != nullptr) && 'Expected first node to precede end node'' * remove nodes at once * remove duplicate MakeSrcContained --- src/coreclr/jit/lower.cpp | 14 +++++--------- src/coreclr/jit/lowerarmarch.cpp | 1 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 8a582ba27ff7d3..87e8433c9a547f 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4895,20 +4895,16 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, return nullptr; } - BlockRange().Remove(relopOp2); - BlockRange().Remove(cond); - BlockRange().Remove(trueVal); - BlockRange().Remove(falseVal); - BlockRange().Remove(select); + // Remove select and its nodes except relopOp1 + BlockRange().Remove(relopOp2, select); relopOp1->ClearContained(); GenTree* shiftConst = m_compiler->gtNewIconNode(31, TYP_INT); GenTree* sar = m_compiler->gtNewOperNode(GT_RSH, select->TypeGet(), relopOp1, shiftConst); - ContainCheckNode(sar); - BlockRange().InsertAfter(relopOp1, shiftConst); BlockRange().InsertAfter(shiftConst, sar); + ContainCheckNode(sar); GenTree* replacement = sar; bool addRequired = falseValConst != 0; @@ -4916,10 +4912,10 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, { GenTree* addConst = m_compiler->gtNewIconNode(falseValConst, select->TypeGet()); GenTree* add = m_compiler->gtNewOperNode(GT_ADD, select->TypeGet(), sar, addConst); - ContainCheckNode(add); - BlockRange().InsertAfter(sar, addConst); BlockRange().InsertAfter(addConst, add); + ContainCheckNode(add); + replacement = add; } diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 2f7ec732acb297..cd89c8bd3de2de 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2900,7 +2900,6 @@ void Lowering::ContainCheckBinary(GenTreeOp* node) if (node->OperIsCommutative() && CheckImmedAndMakeContained(node, op1)) { - MakeSrcContained(node, op1); std::swap(node->gtOp1, node->gtOp2); return; } From 9ed977674bb4e4af3cea440e5d0e8a04aeba2bd5 Mon Sep 17 00:00:00 2001 From: BoyBaykiller Date: Mon, 16 Mar 2026 00:45:07 +0100 Subject: [PATCH 5/5] * cleanup --- src/coreclr/jit/lower.cpp | 105 ++++++++++++++++++++++---------------- src/coreclr/jit/lower.h | 3 +- 2 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 87e8433c9a547f..df00fd6ab3e2e8 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4820,6 +4820,52 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) } #endif // !TARGET_LOONGARCH64 && !TARGET_RISCV64 && !defined(TARGET_WASM) +//---------------------------------------------------------------------------------------------- +// TryLowerSelectToCond: Try to optimize: +// SELECT(relop, 1/0, 0/1) -> (reversed) relop +// +// Arguments: +// select - The SELECT node +// +// Return Value: +// Next node if the optimization was applied, nullptr otherwise. +// +GenTree* Lowering::TryLowerSelectToCond(GenTreeConditional* select) +{ + GenTree* cond = select->gtCond; + GenTree* trueVal = select->gtOp1; + GenTree* falseVal = select->gtOp2; + + if (cond->OperIsCompare() && ((trueVal->IsIntegralConst(0) && falseVal->IsIntegralConst(1)) || + (trueVal->IsIntegralConst(1) && falseVal->IsIntegralConst(0)))) + { + assert(select->TypeIs(TYP_INT, TYP_LONG)); + + LIR::Use use; + if (BlockRange().TryGetUse(select, &use)) + { + if (trueVal->IsIntegralConst(0)) + { + GenTree* reversed = m_compiler->gtReverseCond(cond); + assert(reversed == cond); + } + + // Codegen supports also TYP_LONG typed compares so we can just + // retype the compare instead of inserting a cast. + cond->gtType = select->TypeGet(); + + BlockRange().Remove(trueVal); + BlockRange().Remove(falseVal); + BlockRange().Remove(select); + use.ReplaceWith(cond); + + return cond->gtNext; + } + } + + return nullptr; +} + //---------------------------------------------------------------------------------------------- // TryLowerSelectToSarAdd: Try to optimize: // SELECT(x < 0, C - 1, C) -> SAR(x, 31) + C @@ -4828,29 +4874,23 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) // // Arguments: // select - The SELECT node -// cond - The condition -// trueVal - Value when condition is true -// falseVal - Value when condition is false // // Return Value: // Next node if the optimization was applied, nullptr otherwise. // -GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, - GenTree* cond, - GenTree* trueVal, - GenTree* falseVal) +GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select) { - if (!select->TypeIs(TYP_INT)) - { - return nullptr; - } + GenTree* cond = select->gtCond; + GenTree* trueVal = select->gtOp1; + GenTree* falseVal = select->gtOp2; - if (!trueVal->IsIntegralConst() || !falseVal->IsIntegralConst()) + if (!cond->OperIsCompare() || !trueVal->IsIntegralConst() || !falseVal->IsIntegralConst()) { return nullptr; } - if (!cond->OperIsCompare()) + // TODO-CQ: Handle TYP_LONG and then remove this check + if (!select->TypeIs(TYP_INT)) { return nullptr; } @@ -4934,43 +4974,20 @@ GenTree* Lowering::TryLowerSelectToSarAdd(GenTreeConditional* select, // GenTree* Lowering::LowerSelect(GenTreeConditional* select) { - GenTree* cond = select->gtCond; - GenTree* trueVal = select->gtOp1; - GenTree* falseVal = select->gtOp2; - - if (GenTree* next = TryLowerSelectToSarAdd(select, cond, trueVal, falseVal); next != nullptr) + if (GenTree* next = TryLowerSelectToCond(select); next != nullptr) { return next; } - - // Replace SELECT cond 1/0 0/1 with (perhaps reversed) cond - if (cond->OperIsCompare() && ((trueVal->IsIntegralConst(0) && falseVal->IsIntegralConst(1)) || - (trueVal->IsIntegralConst(1) && falseVal->IsIntegralConst(0)))) + + if (GenTree* next = TryLowerSelectToSarAdd(select); next != nullptr) { - assert(select->TypeIs(TYP_INT, TYP_LONG)); - - LIR::Use use; - if (BlockRange().TryGetUse(select, &use)) - { - if (trueVal->IsIntegralConst(0)) - { - GenTree* reversed = m_compiler->gtReverseCond(cond); - assert(reversed == cond); - } - - // Codegen supports also TYP_LONG typed compares so we can just - // retype the compare instead of inserting a cast. - cond->gtType = select->TypeGet(); - - BlockRange().Remove(trueVal); - BlockRange().Remove(falseVal); - BlockRange().Remove(select); - use.ReplaceWith(cond); - - return cond->gtNext; - } + return next; } + GenTree* cond = select->gtCond; + GenTree* trueVal = select->gtOp1; + GenTree* falseVal = select->gtOp2; + JITDUMP("Lowering select:\n"); DISPTREERANGE(BlockRange(), select); JITDUMP("\n"); diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 9b1d444c6bd537..13261af9f11f7f 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -164,7 +164,8 @@ class Lowering final : public Phase GenTree* LowerSavedIntegerCompare(GenTree* cmp); void SignExtendIfNecessary(GenTree** arg); #endif - GenTree* TryLowerSelectToSarAdd(GenTreeConditional* select, GenTree* cond, GenTree* trueVal, GenTree* falseVal); + GenTree* TryLowerSelectToCond(GenTreeConditional* select); + GenTree* TryLowerSelectToSarAdd(GenTreeConditional* select); GenTree* LowerSelect(GenTreeConditional* cond); bool TryLowerConditionToFlagsNode(GenTree* parent, GenTree* condition,