Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); JIT: Move loop inversion to after loop recognition by amanasifkhalid · Pull Request #115850 · dotnet/runtime · GitHub
Skip to content

JIT: Move loop inversion to after loop recognition - #115850

Merged
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion
Jun 14, 2025
Merged

JIT: Move loop inversion to after loop recognition#115850
amanasifkhalid merged 6 commits into
dotnet:mainfrom
amanasifkhalid:move-loop-inversion

Conversation

@amanasifkhalid

Copy link
Copy Markdown
Contributor

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

CopilotAI review requested due to automatic review settings May 21, 2025 21:11
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 21, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

  • Add single-predecessor block compaction/removal in optInvertWhileLoop
  • Recompute the DFS tree and re-run loop finding after any loop inversions
  • Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

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

FileDescription
optimizer.cppInserted block compaction/removal and DFS invalidation
compiler.cppMoved the loop inversion phase to a later point in compCompile
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

  • Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.
DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

Comment threadsrc/coreclr/jit/optimizer.cpp
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

  • Loops found: 29252
  • Loops inverted: 10694
  • Loops cloned: 1885
  • Loops unrolled: 12
  • Loops IV widened: 3047
  • Widened IVs: 3047
  • Unused IVs removed: 4539
  • Loops downward counted: 1873
  • Loops strength reduced: 1702
  • RBO: 30708
  • Jump threadings: 9735

Diff:

  • Loops found: 29085 (-167)
  • Loops inverted: 9074 (-1620)
  • Loops cloned: 3596 (+1711)
  • Loops unrolled: 12
  • Loops IV widened: 2999 (-48)
  • Widened IVs: 2999 (-48)
  • Unused IVs removed: 4498 (-41)
  • Loops downward counted: 1863 (-10)
  • Loops strength reduced: 1693 (-9)
  • RBO: 33859 (+3151)
  • Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

@AndyAyersMS

Copy link
Copy Markdown
Member

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

@amanasifkhalid

amanasifkhalid commented May 27, 2025

Copy link
Copy Markdown
ContributorAuthor

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

@EgorBo

Copy link
Copy Markdown
Member

CI failures indicate we will need the fix in #113935 to proceed. @EgorBo are you able to revive that work?

Ah, sure, let me revive it

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

I'm removing fgRenumberBlocks while I'm here to avoid opening another PR, FYI.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

Diffs show yet another round of large size increases, though most of this seems to be driven by coreclr_tests. In particular, it looks like we're doing a lot more loop cloning in our HW intrinsics code:

Top method regressions (bytes):
3902 (49.82 % of base) : 308137.dasm - CompareVectorWithZero:TestVector512Equality() (FullOpts)
3902 (49.82 % of base) : 308162.dasm - CompareVectorWithZero:TestVector512Inequality() (FullOpts)
3062 (107.21 % of base) : 323865.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3062 (107.21 % of base) : 323985.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321271.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3058 (107.07 % of base) : 321391.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
3032 (93.18 % of base) : 321266.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionInt64:RunMaskingValueScenario():this (FullOpts)
3032 (93.18 % of base) : 321386.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionUInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323860.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionInt64:RunMaskingValueScenario():this (FullOpts)
3028 (92.94 % of base) : 323980.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionUInt64:RunMaskingValueScenario():this (FullOpts)
2970 (100.00 % of base) : 321971.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 322091.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323084.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2970 (100.00 % of base) : 323204.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324099.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2902 (103.35 % of base) : 324213.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorUnaryOpTest__op_UnaryNegationUInt64:RunBroadcastAndMaskingScenario():this (FullOpts)
2888 (47.56 % of base) : 128125.dasm - VectorTest+VectorRelopTest`1[ulong]:VectorRelOp(ulong,ulong):int (Tier0-FullOpts)
2872 (82.62 % of base) : 322086.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323079.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyInt64:RunMaskingValueScenario():this (FullOpts)
2872 (82.62 % of base) : 323199.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyUInt64:RunMaskingValueScenario():this (FullOpts)
Top method improvements (bytes):
-526 (-8.20 % of base) : 321525.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 321755.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrSByte:RunMaskingValueScenario():this (FullOpts)
-526 (-8.20 % of base) : 322385.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrSByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321410.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseAndByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 321640.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_BitwiseOrByte:RunMaskingValueScenario():this (FullOpts)
-524 (-8.19 % of base) : 322270.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_ExclusiveOrByte:RunMaskingValueScenario():this (FullOpts)
-516 (-8.95 % of base) : 321874.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingZeroScenario():this (FullOpts)
-516 (-8.93 % of base) : 321994.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingZeroScenario():this (FullOpts)
-492 (-16.12 % of base) : 299397.dasm - SmallLoop1:TestEntryPoint():int (FullOpts)
-492 (-16.12 % of base) : 19722.dasm - SmallLoop1:TestEntryPoint():int (Tier0-FullOpts)
-438 (-7.39 % of base) : 321870.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionByte:RunMaskingValueScenario():this (FullOpts)
-438 (-7.37 % of base) : 321990.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionSByte:RunMaskingValueScenario():this (FullOpts)
-434 (-7.66 % of base) : 321174.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 321294.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_AdditionSByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.64 % of base) : 322987.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplyByte:RunMaskingZeroScenario():this (FullOpts)
-434 (-7.61 % of base) : 323107.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_MultiplySByte:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 321922.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionInt16:RunMaskingZeroScenario():this (FullOpts)
-432 (-7.31 % of base) : 322042.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_DivisionUInt16:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.51 % of base) : 323768.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionByte:RunMaskingZeroScenario():this (FullOpts)
-426 (-7.49 % of base) : 323888.dasm - JIT.HardwareIntrinsics.General._Vector512_1.VectorBinaryOpTest__op_SubtractionSByte:RunMaskingZeroScenario():this (FullOpts)
Top method regressions (percentages):
86 (358.33 % of base) : 30264.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Instrumented Tier1)
86 (358.33 % of base) : 39176.dasm - System.Runtime.Intrinsics.Vector64:Create(double):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
86 (358.33 % of base) : 305146.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (FullOpts)
86 (358.33 % of base) : 29956.dasm - System.Runtime.Intrinsics.Vector64:Create[double](double):System.Runtime.Intrinsics.Vector64`1[double] (Tier0-FullOpts)
188 (348.15 % of base) : 65714.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[long],System.Runtime.Intrinsics.Vector64`1[long]):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
188 (348.15 % of base) : 65777.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[ulong],System.Runtime.Intrinsics.Vector64`1[ulong]):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
146 (347.62 % of base) : 54822.dasm - System.Runtime.Intrinsics.Vector64`1[double]:op_Addition(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[double] (Tier1)
40 (333.33 % of base) : 344367.dasm - SwitchTest:TestEntryPoint():int (FullOpts)
40 (333.33 % of base) : 124230.dasm - SwitchTest:TestEntryPoint():int (Tier0-FullOpts)
82 (292.86 % of base) : 30256.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 39215.dasm - System.Runtime.Intrinsics.Vector64:Create(float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
82 (292.86 % of base) : 30262.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
82 (292.86 % of base) : 54956.dasm - System.Runtime.Intrinsics.Vector64:Create[float](float):System.Runtime.Intrinsics.Vector64`1[float] (Tier1)
204 (291.43 % of base) : 65655.dasm - System.Runtime.Intrinsics.Vector64:Narrow(System.Runtime.Intrinsics.Vector64`1[double],System.Runtime.Intrinsics.Vector64`1[double]):System.Runtime.Intrinsics.Vector64`1[float] (Instrumented Tier1)
68 (283.33 % of base) : 30252.dasm - System.Runtime.Intrinsics.Vector64:Create(int):System.Runtime.Intrinsics.Vector64`1[int] (Instrumented Tier1)
68 (283.33 % of base) : 39238.dasm - System.Runtime.Intrinsics.Vector64:Create(uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 305145.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (FullOpts)
68 (283.33 % of base) : 29952.dasm - System.Runtime.Intrinsics.Vector64:Create[int](int):System.Runtime.Intrinsics.Vector64`1[int] (Tier0-FullOpts)
68 (283.33 % of base) : 55016.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Instrumented Tier1)
68 (283.33 % of base) : 55288.dasm - System.Runtime.Intrinsics.Vector64:Create[uint](uint):System.Runtime.Intrinsics.Vector64`1[uint] (Tier1)

Diffs in our non-test collections, particularly the ones with Dynamic PGO enabled, are much less dramatic. Also, the TP improvement pays for #116017, which is nice. @AndyAyersMS are you ok with this going into Preview 6?

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, let's take this.

@amanasifkhalid

Copy link
Copy Markdown
ContributorAuthor

/ba-g unrelated wasm build failure, and a known issue

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@amanasifkhalid@AndyAyersMS@EgorBo