More precise ranges for casts in GetRangeFromAssertions#124415
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves range analysis in the JIT's range check optimization pass by enabling more precise range propagation through cast operations and adding support for TYP_INT in GetRangeFromType.
Changes:
- Add early range tightening based on VN type for small types in GetRangeFromAssertions (though this appears to be dead code due to VN normalization)
- Change cast source type check from exact TYP_INT match to using genActualType, making the code more defensive
- Add TYP_INT case to GetRangeFromType to return the full int32 range instead of Unknown
3 tasks
2704ba3 to
e492c87
Compare
6546d64 to
a0cbca9
Compare
The varTypeIsGC(vnType) branch was added defensively for 32-bit, where TYP_I_IMPL == TYP_INT could in theory let GC-typed VNs reach this code. In practice, all callers gate on TYP_INT and recursive sites either gate on TYP_INT or operate on results that cannot produce GC operands of an INT VN. SPMI replay (x64 aspnet/libraries/benchmarks and x86 benchmarks) shows no behavior change after removal — asmdiffs are identical at -4,708 bytes net for aspnet. If a GC VN ever does leak in, the existing assert(genActualType(vnType) == TYP_INT) will surface it cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jakobbotsch
reviewed
May 10, 2026
Per @jakobbotsch review feedback: NOL locals can live in registers where the JIT does not insert any normalization without the cast. Retyping the LCL_VAR to its small type and dropping the cast lets codegen read the register without a normalizing load (e.g., movzx), exposing potentially undefined upper bits for register-resident NOL params. The reason morph.cpp:3230 can perform a similar retype-and-drop is that it gates on local subrange assertions, which only exist after a JIT-controlled store that normalized the bits. VN-based subrange proofs have no such guarantee — they can come from comparisons, casts, joins, or phi merges that say nothing about the physical register state. Restore the original PR behavior of skipping NOL locals entirely in the VN-based path. The local-prop path below is unchanged and still performs the safe retyping based on local assertions. Cost on benchmarks.run: +4,482 bytes vs the (unsafe) prior commit, but still -3,670 bytes net vs origin/main. SPMI replay clean on aspnet, libraries (incl. tests), benchmarks. JIT/opt/Or/IntOr disasm pattern preserved by the small-cast-on-non-LCL_VAR guard introduced earlier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reverts 0a3a858 ("Remove unreachable GC early return ...") which broke 32-bit crossgen2 of System.Private.CoreLib (Memmove and others asserting `genActualType(vnType) == TYP_INT` at rangecheck.cpp:698). Why the GC return is actually needed (and my prior analysis missed): On 32-bit targets TYP_INT/TYP_BYREF/TYP_REF are all 4 bytes, so the JIT can legally write a byref-valued expression (e.g. LCL_ADDR) into an int-typed local. The local is TYP_INT but its VN is a TYP_BYREF function such as PtrToLoc. Repro context (libraries.crossgen2 MC 146148, F# SubstHelperRaw): STORE_LCL_VAR int V06 <- LCL_ADDR int V15 # V06 has VN PtrToLoc(...) (BYREF) ...PHI int V06 across blocks $340 = PhiDef(...) NE int (LCL_VAR int V06, CNS_INT int 0) # asks for range of $340 PR dotnet#127117 (Apr 23) added PhiDef recursion in GetRangeFromAssertionsWorker; the visitor walks reaching VNs of the phi, and on 32-bit one of them can be the BYREF PtrToLoc VN even though the public caller passed an int-typed tree. The pre-existing GC early-return guard catches exactly this case; the explanatory comment is updated so the next reader doesn't repeat my mistake. x64 SPMI didn't catch it because TYP_BYREF (8 bytes) and TYP_INT (4 bytes) cannot alias in the same local, so `LCL_ADDR int` doesn't appear and the phi recursion never reaches a BYREF VN. Validation: x86 libraries.crossgen2 SPMI replay clean (279,521 ctx). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@EgorBot -arm -amd using BenchmarkDotNet.Attributes;
public class Benchmarks
{
private byte b = 42;
[Benchmark]
public string ByteToString() => b.ToString();
} |
Member
Author
|
I think this is ready, PTAL @jakobbotsch @dotnet/jit-contrib An example of what it folds: runtime/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs Lines 1934 to 1941 in 7900769 When Byte.cs pass itself on its ToString() we can fold the |
EgorBo
commented
May 12, 2026
jakobbotsch
reviewed
May 12, 2026
jakobbotsch
reviewed
May 12, 2026
jakobbotsch
approved these changes
May 12, 2026
Member
Author
|
/ba-g timeouts |
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