Ignore PhiArgs without matching actual preds by EgorBo · Pull Request #125093 · dotnet/runtime

EgorBo · 2026-03-03T01:41:09Z

A couple of diffs

dotnet-policy-service · 2026-03-03T01:42:46Z

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

Copilot

Pull request overview

Updates CoreCLR JIT’s assertion propagation through SSA PHI nodes to be more conservative when PHI arguments don’t correspond to actual CFG predecessors, and adds a regression test for #124507.

Changes:

Add a guard in optVisitReachingAssertions to abort PHI-based assertion inference when a PhiArg references a non-predecessor block.
Add a new JIT regression test Runtime_124507.
Wire the new test into Regression_ro_2.csproj.

Reviewed changes

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

File	Description
src/coreclr/jit/compiler.hpp	Abort PHI reaching-assertions walk when a PHI arg pred block is not an actual predecessor.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_124507` regression test source file.
src/tests/JIT/Regression/JitBlue/Runtime_124507/Runtime_124507.cs	Adds a reduced repro as an xUnit test entry point for #124507.

src/tests/JIT/Regression/JitBlue/Runtime_124507/Runtime_124507.cs

…ntime-1 into fix-optVisitReachingAssertions

Copilot

Pull request overview

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

src/coreclr/jit/compiler.hpp

Copilot

Pull request overview

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

EgorBo · 2026-03-03T16:10:39Z

PTAL @AndyAyersMS @dotnet/jit-contrib
I tried your suggestion with just checking two bitvectors in the end instead of the second pred walk, but Copilot suggested that it might be more expensive as we PHI don't have too many preds usually, but bitvectors might be long (fgMaxBlocks).

Also, I was not brave enough to just ignore PhiArgs without matching pred blocks (but if I do it, I get -200kb diff improvement).

AndyAyersMS · 2026-03-03T17:12:29Z

Can you describe what goes wrong without this fix?

Naively I'd expect that if we considered assertions from preds that no longer reached we would only be more conservative.

EgorBo · 2026-03-03T19:04:54Z

@AndyAyersMS it turned out the actual bug was in optGetEdgeAssertions that returned incorrect assertions for an edge that doesn't exist (because of RBO) for that Phi.

I propose we still keep the optVisitReachingAssertions change (it's no longer necessary to fix the bug) since the regression is pretty small just in case. We can alter the PhiVN if we want and remove dead preds somewhere (e.g. in GlobalAP)

AndyAyersMS · 2026-03-03T20:06:14Z

src/coreclr/jit/assertionprop.cpp

 ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* block, const BasicBlock* blockPred) const
 {
-    if ((blockPred->KindIs(BBJ_COND) && blockPred->TrueTargetIs(block)))
+    if (blockPred->KindIs(BBJ_COND))


It seems odd that we're only suspicious of BBJ_COND preds here. Shouldn't we do the same check no matter what kind of pred it is?

If block is not a successor of blockPred, return an empty BV.

@AndyAyersMS oops, that was exactly what I had in my mind, but I put it under a wrong branch. Fixed. thanks for noticing

oops, wrong commit again

AndyAyersMS · 2026-03-03T20:26:24Z

src/coreclr/jit/assertionprop.cpp

-    // Return empty set if the edge doesn't exist
-    // (e.g. a stale PHI arg pred after edge redirection by RBO).
-    if (!blockPred->FalseTargetIs(block))
+    if (blockPred->GetUniqueSucc() == block)


This may be too restrictive now?

It should still be ok to propagate assertions from BBJ_SWITCH or even maybe even BBJ_EHINALLYRET which can have multiple successors, so long as block is one of them.

@AndyAyersMS addressed

Copilot

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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

ignore PhiArgs without real preds

64b10db

Copilot AI review requested due to automatic review settings March 3, 2026 01:41

dotnet-policy-service bot assigned EgorBo Mar 3, 2026

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

Copilot started reviewing on behalf of EgorBo March 3, 2026 01:42 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/tests/JIT/Regression/JitBlue/Runtime_124507/Runtime_124507.cs Show resolved Hide resolved

Update Runtime_124507.cs

338b4fb

egorbot added 2 commits March 3, 2026 13:48

feedback

6fc1709

Merge branch 'fix-optVisitReachingAssertions' of github.com:EgorBo/ru…

2ea5141

…ntime-1 into fix-optVisitReachingAssertions

Copilot AI review requested due to automatic review settings March 3, 2026 12:48

Copilot started reviewing on behalf of EgorBo March 3, 2026 12:49 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/coreclr/jit/compiler.hpp Show resolved Hide resolved

src/coreclr/jit/compiler.hpp Outdated Show resolved Hide resolved

egorbot added 2 commits March 3, 2026 14:07

feedback

c26fae7

cleanup

62011af

Copilot AI review requested due to automatic review settings March 3, 2026 13:08

fix comment

5c2028c

Copilot started reviewing on behalf of EgorBo March 3, 2026 13:10 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

EgorBo marked this pull request as ready for review March 3, 2026 15:04

Merge branch 'main' into fix-optVisitReachingAssertions

1863cc0

EgorBo requested a review from AndyAyersMS March 3, 2026 16:08

fix optGetEdgeAssertions

203d698

Copilot AI review requested due to automatic review settings March 3, 2026 19:02

build-analysis bot mentioned this pull request Mar 3, 2026

[android] Android.Device_Emulator.JIT.Test failing on emulators with CoreCLR #112633

Open

AndyAyersMS reviewed Mar 3, 2026

View reviewed changes

egorbot added 2 commits March 3, 2026 21:08

feedback

81d00d6

fix

213d343

AndyAyersMS reviewed Mar 3, 2026

View reviewed changes

fix

9883ea3

Copilot AI reviewed Mar 3, 2026

View reviewed changes

feedback

0e6bbcc

Copilot AI review requested due to automatic review settings March 3, 2026 20:50

Copilot started reviewing on behalf of EgorBo March 3, 2026 20:51 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Conversation

EgorBo commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

EgorBo commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyAyersMS commented Mar 3, 2026

Uh oh!

EgorBo commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyAyersMS Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EgorBo commented Mar 3, 2026 •

edited

Loading

EgorBo commented Mar 3, 2026 •

edited

Loading

EgorBo commented Mar 3, 2026 •

edited

Loading