Update fgMorphSmpOp to canonicalize commutative and compare ops at the start of POST-ORDER by tannergooding · Pull Request #127661 · dotnet/runtime

tannergooding · 2026-05-01T20:09:58Z

No description provided.

dotnet-policy-service · 2026-05-01T20:11:29Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR adjusts CoreCLR JIT morphing to canonicalize operand ordering for commutative operators and comparisons at the start of fgMorphSmpOp post-order processing, improving consistency and enabling post-order optimizations that rely on constants being placed predictably.

Changes:

Canonicalize commutative and comparison trees in fgMorphSmpOp post-order by swapping when the left operand is a constant (and swapping relops as needed).
Move the ((a % pow2) ==/!= 0) => (a & (pow2-1) ==/!= 0) pattern optimization from pre-order into the GT_EQ/GT_NE post-order handling.
Strengthen/debug-guard morphing assumptions by asserting fgMorphTree is not invoked during the value-number CSE phase.

…e start of POST-ORDER

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

tannergooding · 2026-05-02T06:01:45Z

@EgorBo, this should be ready now. There's a few regressions where folding looks to stop happening. It needs to be investigated, but I don't think we should block on it. There's likely some other places that aren't looking for or handling the right edges.

EgorBo · 2026-05-02T16:39:51Z

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


I do not understand why we have to call it here and above + more code above vs what I suggested in https://github.com/dotnet/runtime/compare/main...EgorBo:runtime-1:swap-cns?expand=1 - much smaller change with better diffs

Because it had a significant throughput hit, nearly 0.20% in minopts

It is simply not efficient to have every single simple op check if they are a compare or commutative node.

EgorBo · 2026-05-02T16:42:29Z

-                            op1->SetOper(GT_AND);                                 // Change % => &
-                            op1op2->AsIntConCommon()->SetIconValue(modValue - 1); // Change c => c - 1
-                            fgUpdateConstTreeValueNumber(op1op2);
+                            otherNode->SetOper(GT_AND); // Change % => &


we need to preserve VN, otherwise it makes no sense to call fgUpdateConstTreeValueNumber below

What isn't being preserved for VN here? This is doing the exact same thing as before just handling either op1 or op2 being the constant

What isn't being preserved for VN here? This is doing the exact same thing as before just handling either op1 or op2 being the constant

SetOper erases it by default, PRESERVE_VN should be used

EgorBo · 2026-05-02T16:45:08Z

+                else if (op1->IsIntegralConst(0))
+                {
+                    cnsNode   = op1;
+                    otherNode = op2;


so why do we rotate constants by hands here when you introduced a helper?

Because this isn't actually swapping the constants. It's keeping them as is and simply tracking which is constant.

They are swapped further down in post order, which avoids the extra expense and repeated work.

EgorBo · 2026-05-02T16:46:59Z

There's a few regressions

any idea why osx-arm64 seems to be a pure regression?

tannergooding · 2026-05-02T16:54:57Z

any idea why osx-arm64 seems to be a pure regression?

It has more opts being done, in part because it has a higher ISA baseline.

Copilot AI review requested due to automatic review settings May 1, 2026 20:09

Copilot started reviewing on behalf of tannergooding May 1, 2026 20:10 View session

github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 1, 2026

dotnet-policy-service Bot assigned tannergooding May 1, 2026

Copilot AI reviewed May 1, 2026

View reviewed changes

tannergooding force-pushed the fgMorphSmpOp branch from c1f0ad2 to c4471af Compare May 1, 2026 20:46

Update fgMorphSmpOp to canonicalize commutative and compare ops at th…

b3541f8

…e start of POST-ORDER

Copilot AI review requested due to automatic review settings May 1, 2026 20:48

tannergooding force-pushed the fgMorphSmpOp branch from c4471af to b3541f8 Compare May 1, 2026 20:48

Copilot started reviewing on behalf of tannergooding May 1, 2026 20:48 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/jit/morph.cpp Outdated

EgorBo reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/jit/morph.cpp Outdated

EgorBo reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/jit/morph.cpp Outdated

build-analysis Bot mentioned this pull request May 1, 2026

XHarness package install failure on iOS due to devicectl NSPOSIXErrorDomain error 49 #123796

Open

tannergooding added 2 commits May 1, 2026 16:07

Try to reduce the TP impact

0942698

Fix the build

ad994f6

Copilot AI review requested due to automatic review settings May 1, 2026 23:50

Copilot started reviewing on behalf of tannergooding May 1, 2026 23:50 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Copilot stopped reviewing on behalf of tannergooding due to an error May 2, 2026 00:30
The "Install Dependencies" custom setup step from your .github/workflows/copilot-setup-steps.yml file failed.

tannergooding added 2 commits May 1, 2026 18:33

Don't regress for cases where GT_MOD is morphed to a helper

ab9c316

Handle the other scenario for mac

13934d9

Copilot AI review requested due to automatic review settings May 2, 2026 01:50

Copilot started reviewing on behalf of tannergooding May 2, 2026 01:50 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Comment thread src/coreclr/jit/morph.cpp Outdated

Comment thread src/coreclr/jit/morph.cpp Outdated

Comment thread src/coreclr/jit/morph.cpp Outdated

tannergooding added 2 commits May 1, 2026 19:18

Fix a few issues

1dd4aa6

Don't touch the MorphAddrContext handling

bbfd6cd

Copilot AI review requested due to automatic review settings May 2, 2026 04:18

Copilot started reviewing on behalf of tannergooding May 2, 2026 04:19 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Comment thread src/coreclr/jit/morph.cpp

tannergooding marked this pull request as ready for review May 2, 2026 05:59

build-analysis Bot mentioned this pull request May 2, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

EgorBo reviewed May 2, 2026

View reviewed changes

Conversation

tannergooding commented May 1, 2026

Uh oh!

dotnet-policy-service Bot commented May 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

tannergooding commented May 2, 2026

Uh oh!

EgorBo May 2, 2026

Choose a reason for hiding this comment

Uh oh!

tannergooding May 2, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo May 2, 2026

Choose a reason for hiding this comment

Uh oh!

tannergooding May 2, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo May 2, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo May 2, 2026

Choose a reason for hiding this comment

Uh oh!

tannergooding May 2, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo commented May 2, 2026

Uh oh!

tannergooding commented May 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants