Skip to content

Arm64: Fix for 96441 #123022

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

Open
dhartglassMSFT wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Arm64: Fix for 96441 #123022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0b650c6
initial impl
dhartglassMSFT Jan 9, 2026
c8a9485
fix build break
dhartglassMSFT Jan 10, 2026
67a786b
reverse condcode if needed
dhartglassMSFT Jan 17, 2026
d9212f4
some code cleanup
dhartglassMSFT Jan 20, 2026
b10f0e8
unneeded locals
dhartglassMSFT Jan 23, 2026
5bfce98
pr feedback
dhartglassMSFT Jan 27, 2026
1624d87
fix formatting
dhartglassMSFT Jan 28, 2026
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: 1 addition & 1 deletion src/coreclr/jit/lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
147 changes: 110 additions & 37 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -3466,56 +3467,128 @@ 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 branch has a use of the value+1

if (!cond->OperIs(GT_CMP) || !select->OperIs(GT_SELECTCC))
{
return;
}
if ((!cond->gtGetOp1()->IsIntegralConst() && !cond->gtGetOp2()->IsIntegralConst()) ||
(!cond->gtGetOp1()->OperIs(GT_LCL_VAR) && !cond->gtGetOp2()->OperIs(GT_LCL_VAR)))
{
return;
}

// One of the CMP operands is a constant int
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

In the non-constant/constant path, this code calls select->AsOpCC() and later mutates the node to GT_SELECT_INCCC, but TryLowerCnsIntCselToCinc is declared to accept both GT_SELECT and GT_SELECTCC. Please add an explicit guard/assert that select is GT_SELECTCC before calling AsOpCC() (or restructure so this path can’t be reached for GT_SELECT) to avoid UB if the precondition ever changes.

Suggested change
// One of the CMP operands is a constant int
// One of the CMP operands is a constant int
if (!select->OperIs(GT_SELECTCC))
{
// This transformation currently applies only to GT_SELECTCC nodes.
return;
}

Copilot uses AI. Check for mistakes.
GenCondition::Code 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;
}
// 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))
{
return;
}
if (code == GenCondition::NE &&
(!falseVal->IsIntegralConst() || falseVal->AsIntCon()->IntegralValue() - constVal != 1))
{
return;
}

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

Expand Down
Loading