Skip to content

JIT: fold isinst using reaching VNs and compareTypesForCast#128500

Merged
EgorBo merged 9 commits into
dotnet:mainfrom
EgorBo:jit-isinst-fold-reaching-vns
May 23, 2026
Merged

JIT: fold isinst using reaching VNs and compareTypesForCast#128500
EgorBo merged 9 commits into
dotnet:mainfrom
EgorBo:jit-isinst-fold-reaching-vns

Conversation

@EgorBo
Copy link
Copy Markdown
Member

@EgorBo EgorBo commented May 22, 2026

This PR extends optAssertionIsSubtype to support PHI nodes via optVisitReachingAssertions. Also it makes logic slightly less conservative than direct == for class handles when we iterate assertions.

Example: the following always-true cast is currently not folded by the JIT:

static void Test(int len)
{
    object bc;
    if (len > 100)
        bc = new DerivedClass1();
    else
        bc = new DerivedClass2();

    if (bc is BaseClass) // <--- always true, but we don't fold it.
        Console.WriteLine("bc is a BaseClass");
    else
        Console.WriteLine("bc is not a BaseClass");
}

class BaseClass {}
class DerivedClass1 : BaseClass {}
class DerivedClass2 : BaseClass {}

Codegen:

 ; Assembly listing for method Program:Test(int) (FullOpts)
 G_M29168_IG01:
        sub      rsp, 40
-G_M29168_IG02:
-       cmp      ecx, 100
-       jg       SHORT G_M29168_IG04
-G_M29168_IG03:
-       mov      rcx, 0x7FFEA76B8E60      ; DerivedClass2
-       call     CORINFO_HELP_NEWSFAST
-       jmp      SHORT G_M29168_IG05
-G_M29168_IG04:
-       mov      rcx, 0x7FFEA76BC140      ; DerivedClass1
-       call     CORINFO_HELP_NEWSFAST
-G_M29168_IG05:
-       mov      rdx, rax
-       mov      rcx, 0x7FFEA76B8CD8      ; BaseClass
-       call     CORINFO_HELP_ISINSTANCEOFCLASS
-       test     rax, rax
-       jne      SHORT G_M29168_IG07
-G_M29168_IG06:
-       mov      rcx, 0x26775762D58      ; 'bc is not a BaseClass'
-       call     [System.Console:WriteLine(System.String)]
-       jmp      SHORT G_M29168_IG08
-G_M29168_IG07:
+G_M29168_IG02:
        mov      rcx, 0x26775762D98      ; 'bc is a BaseClass'
        call     [System.Console:WriteLine(System.String)]
-G_M29168_IG08:
        nop
-G_M29168_IG09:
+G_M29168_IG03:
        add      rsp, 40
        ret
-; Total bytes of code 104
+; Total bytes of code 26

Extend optAssertionIsSubtype to:
* resolve target/source class handles via the embedded-handle map
  (AOT/R2R-safe);
* fold via compareTypesForCast == Must, not just handle equality;
* inspect VN-level type info (e.g. VNF_JitNew exact type);
* recurse through reaching VNs/assertions for PHI defs.

This lets the JIT prove always-true isinst checks where the object
comes from a PHI of typed allocations (e.g. new DerivedClass1() and
new DerivedClass2() joined into object then tested 'is BaseClass').

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 18:45
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 22, 2026
@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 22, 2026

@MihuBot

@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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 enhances CoreCLR JIT global assertion propagation so it can prove certain isinst/chkcast helper calls are redundant by combining (1) live assertions, (2) VN-level object type info, and (3) PHI reaching-VN analysis, using compareTypesForCast to validate cast success.

Changes:

  • Refactors optAssertionIsSubtype to return bool and to resolve the target type handle robustly (helper-arg handle or VN-encoded handle).
  • Introduces optAssertionVNIsSubtype to prove cast success via matching subtype/exact-type assertions, VN object-type recovery, and recursive PHI-edge analysis with a depth budget.
  • Updates the cast-helper handling in optAssertionProp_Call to consume the new boolean result and adjust debug logging.

Reviewed changes

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

File Description
src/coreclr/jit/compiler.h Updates assertion-prop helper signatures and declares the new VN-based subtype helper.
src/coreclr/jit/assertionprop.cpp Implements VN + reaching-assertion based subtype proof and applies it to folding redundant cast helpers.

Comment thread src/coreclr/jit/assertionprop.cpp Outdated
@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 22, 2026

@MihuBot

Copilot AI review requested due to automatic review settings May 22, 2026 20:29
EgorBo and others added 2 commits May 22, 2026 22:30
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/coreclr/jit/valuenum.h Outdated
Comment thread src/coreclr/jit/assertionprop.cpp Outdated
Comment thread src/coreclr/jit/assertionprop.cpp Outdated
EgorBo and others added 2 commits May 22, 2026 23:18
* valuenum.h: describe the pCls out-param and embedded-handle mapping.

* assertionprop.cpp: fix self-referential comment (cls -> castTo).

* Add regression test under JIT/opt/AssertionPropagation covering PHI-of-derived (folds), PHI-with-null (must not fold to true), direct alloc, and PHI-with-unrelated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 21:22
@EgorBo EgorBo marked this pull request as ready for review May 22, 2026 21:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.cs
Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.csproj
Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.cs
@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 22, 2026

PTAL @AndyAyersMS @dotnet/jit-contrib, SPMI diffs are empty due to new JIT-EE calls, jit-diffs

@EgorBo EgorBo requested a review from AndyAyersMS May 22, 2026 22:38
Copy link
Copy Markdown
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

I had the impression that optAssertionVNIsSubtype was not very useful. Am I wrong?

`GenTree::BashToConst<T>` repurposes an arbitrary node into a constant but
never resets `AsIntCon()->gtCompileTimeHandle`, so the field retains stale
data from the node's prior life. `optConstantAssertionProp` then re-applies
`GTF_ICON_CLASS_HDL` on top of the bashed node based on the VN's handle
flags, leaving a CNS_INT with `iconValue = realClassHandle`, the class-handle
flag set, but a stale `gtCompileTimeHandle`.

When `fgValueNumberTreeConst` later sees this node it overwrites the
existing entry in the EmbeddedHandleMap with that stale handle (via
`AddOrUpdate`), poisoning lookups. Consumers such as `IsVNTypeHandle` and
`VNEvalForCast` then read the garbage back, and `compareTypesForCast`
crashes when dereferencing the bogus type handle.

Three small fixes:

1. `GenTree::BashToConst<T>`: zero `gtCompileTimeHandle` alongside the
   existing `gtFieldSeq = nullptr`, matching the "node is now a fresh
   constant" semantics.

2. `fgValueNumberTreeConst`: only register an entry in the
   EmbeddedHandleMap when `gtCompileTimeHandle != 0`, so a re-flagged
   bashed node cannot overwrite a valid mapping placed by a real
   `gtNewIconEmbHndNode` that shares the same iconValue.

3. `fgValueNumberSpecialIntrinsic`: fix an inverted bail-out that proceeds
   to `getRuntimeTypePointer` with a 0 handle when the map lookup fails
   (`&&` + `clsHandle != 0` made the bail unreachable because `clsHandle`
   is initialized to 0). With fix #2, "no entry" becomes more common, and
   this site must bail explicitly.

Reproduced on System.Memory.Tests / System.Linq.Tests with
DOTNET_TieredCompilation=1 against the PR's `IsVNTypeHandle` consumer.
Tests pass after the fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 23, 2026 01:45
@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 23, 2026

I had the impression that optAssertionVNIsSubtype was not very useful. Am I wrong?

It seems it leads to +1,720 bytes regressions and 730 missing contexts if I remove it (in Libraries.pmi) so not sure. jit-diff hit just a few uses because these patterns often appear with PGO and jit-diff doesn't have it.

BTW, fixed #128061 bug along the way, BashToConst didn't zero out gtCompileTime handle.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.csproj
Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.cs
Comment thread src/tests/JIT/opt/AssertionPropagation/IsInstReachingVN.cs
@EgorBo EgorBo merged commit 6d63129 into dotnet:main May 23, 2026
143 of 145 checks passed
@EgorBo EgorBo deleted the jit-isinst-fold-reaching-vns branch May 23, 2026 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR 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.

3 participants