Increase number of assertions (GlobalAP) + VN cache#124132
Merged
EgorBo merged 2 commits intodotnet:mainfrom Feb 11, 2026
Merged
Increase number of assertions (GlobalAP) + VN cache#124132EgorBo merged 2 commits intodotnet:mainfrom
EgorBo merged 2 commits intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR increases Global Assertion Propagation capacity and adds a small VN-based cache to avoid unnecessary full-table assertion scans, benefiting range check/range analysis and assertion propagation throughput.
Changes:
- Adjusts
optMaxAssertionCountheuristics to allow tracking more assertions (especially for GlobalAP). - Introduces
optAssertionVNsMap+optAssertionHasAssertionsForVN()to quickly determine if any assertions exist for a given VN (with lazy initialization). - Uses the VN cache to early-out in range-check assertion merges and GlobalAP local-var assertion propagation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/coreclr/jit/rangecheck.cpp |
Uses the new VN cache to skip assertion scanning when no assertions exist for the VN. |
src/coreclr/jit/compiler.h |
Adds optAssertionVNsMap and declares optAssertionHasAssertionsForVN(). |
src/coreclr/jit/assertionprop.cpp |
Updates assertion table sizing, populates/queries the VN cache, and applies new early-out paths in GlobalAP. |
This was referenced Feb 7, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Member
Author
|
@AndyAyersMS @dotnet/jit-contrib PTAL. This is still a hacky way (I mean the heuristic) to deal with the TP regression, but it's better than nothing. Very few regressions, lots of improvements, small TP regression overall. |
This was referenced Feb 9, 2026
Member
Author
|
ping @AndyAyersMS |
AndyAyersMS
approved these changes
Feb 11, 2026
EgorBo
added a commit
that referenced
this pull request
Feb 12, 2026
Bad merge of two PRs at once. #124132 - introduced a VN cache to find out if we have any assertions for a VN. And #124242 introduced a VN decomposition inside MergeEdgeAssertion loop where it tries to match VNF_ADD(X, CNS). So the fix is to register VNF_ADD's op1 VN as well. As the result, #124242 couldn't find the matching assertion and the FILECHECK test failed due to bounds check in the codegen.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diffs
This PR does two things:
I initially tried to use this cache everywhere, but ended up keeping it only in a few places where it had a noticeable TP benefit. This change alone has zero size diffs and reduces the TP regression from change (2) by 2×.
With help from SPMI, I analyzed 1M methods and built a large table:
| BB count | TrackedLocals count | assertions created |. I then fed it to an AI agent, which iterated for a while, generated some Python scripts, and proposed a heuristic (after several rounds of SPMI TP/size feedback). This is the best trade-off between improvements and TP regression that I was able to find. We could potentially incorporate more parameters (e.g., number of conditional blocks against null/constant, number of indirects), but I think the current version is good enough for now.* I had 3 variants and ended up picking the 3rd one due to better TP results (obviously, less of =256 entities)
NOTE: If I unconditionally set
optMaxAssertionCountto 256 the size diffs become ~8% better and TP regresses by 3xThe worst-case +0.3% TP regression should be more than covered by my earlier refactoring PRs that improved TP (the combined net effect should be around -0.5%).