Skip to content

JIT: Add canonicalization to GT_EQ and GT_NE #127548

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
BoyBaykiller wants to merge 4 commits into
base: main
Choose a base branch
from
+22 −0
Open

JIT: Add canonicalization to GT_EQ and GT_NE #127548

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdef8cf
* add missing 'CNS relop op2' to 'op2 relop* CNS' canonicalization
BoyBaykiller Apr 29, 2026
d0091bb
* check if pow2, this misses 1 << 31...
BoyBaykiller Apr 29, 2026
6a09c55
* AsIntCon instead AsIntConCommon
BoyBaykiller Apr 29, 2026
3fffbe1
* AsIntCommon instead AsIntCon
BoyBaykiller Apr 29, 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
22 changes: 22 additions & 0 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7768,6 +7768,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

case GT_EQ:
case GT_NE:
// Change "CNS relop op2" to "op2 relop* CNS"
if (op1->IsIntegralConst() && tree->OperIsCompare() && gtCanSwapOrder(op1, op2))
Copy link
Copy Markdown
Member

@EgorBo EgorBo Apr 29, 2026

Choose a reason for hiding this comment

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

I see that there is already a logic that moves constants to the right, why do we repeat it here?

Copy link
Copy Markdown
Contributor Author

@BoyBaykiller BoyBaykiller Apr 29, 2026

Choose a reason for hiding this comment

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

I totally agreee. But the fact that its written in a switch makes it hard/ugly to freely share code. I noticed the exact thing when I was adding fgOptimizeDistributiveArithemtic from my other PR. Which personally motivates me to rewrite this whole thing to using classic if stmts without switch

{
std::swap(tree->AsOp()->gtOp1, tree->AsOp()->gtOp2);
tree->gtOper = GenTree::SwapRelop(tree->OperGet());

oper = tree->OperGet();
op1 = tree->gtGetOp1();
op2 = tree->gtGetOp2();
}

if (op2->IsIntegralConst())
{
tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp());
Expand Down Expand Up @@ -8808,6 +8819,17 @@ GenTree* Compiler::fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp)
fgUpdateConstTreeValueNumber(op2);
}

// Canonicalize '(A & pow2) == pow2' -> '(A & pow2) != 0'
if (cmp->OperIs(GT_EQ) && op1->OperIs(GT_AND) && op1->gtGetOp2()->IsIntegralConstUnsignedPow2())
Copy link
Copy Markdown
Contributor Author

@BoyBaykiller BoyBaykiller Apr 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The IsIntegralConstUnsignedPow2 check misses 1 << 31 (or 1 << 63). I need to know if there is only a single bit set. Naturally I would do Popcount(x) == 1, but how can I get the unsigned/not-sign-extend literal? All the APIs seem to sign extend first.

{
if (op1->gtGetOp2()->AsIntConCommon()->IntegralValue() == op2->IntegralValue())
{
cmp->SetOper(GT_NE, GenTree::PRESERVE_VN);
op2->SetIntegralValue(0);
fgUpdateConstTreeValueNumber(op2);
}
}

// Here we look for the following tree
//
// EQ/NE
Expand Down
Loading