[experiment] JIT: Introduce KnownBits#129082
Draft
EgorBo wants to merge 3 commits into
Draft
Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new JIT-internal KnownBits analysis (LLVM-style “known zero/known one” bit lattice) and wires it into assertion propagation so the JIT can fold more comparisons (including TYP_LONG) and prove more casts redundant/overflow-safe using bit-level facts.
Changes:
- Introduces
KnownBits/KnownBitsOps(bit lattice + transfer functions) and aKnownBits::ComputeVN/assertion-driven analysis. - Uses KnownBits in global assertion propagation to fold relops and to remove/relax casts when provably safe.
- Updates JIT build wiring to compile the new implementation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/knownbits.h | New KnownBits lattice + transfer helpers (And/Or/UDiv/Cast/EvalRelop) and analysis entrypoint. |
| src/coreclr/jit/knownbits.cpp | Implements VN/assertion-based KnownBits computation, including PHI merging and assertion refinement. |
| src/coreclr/jit/CMakeLists.txt | Adds KnownBits sources/headers to the JIT build. |
| src/coreclr/jit/assertionprop.cpp | Hooks KnownBits into global relop folding and cast simplification paths; widens some assertion creation gates to include TYP_LONG. |
Adds a 32/64-bit fixed-width KnownBits lattice for integral VNs, plus an
assertion-aware analysis (KnownBits::Compute) that derives the known bits of
a value number from its VN structure and the incoming assertions. The struct
and transfer functions are ports of llvm::KnownBits from
llvm/Support/KnownBits.{h,cpp}; only the subset of operations needed by the
current consumers (And/Or/UDiv/Cast/EvalRelop) is included.
Three consumers wired in assertionprop.cpp:
* optAssertionProp_RangeProperties - sign-bit/non-zero from KnownBits
* optAssertionPropGlobal_RelOp - relop folding
* optAssertionProp_BndsChk - (uint)index < (uint)length
Plus a one-shot KnownBits refinement at the end of
RangeCheck::GetRangeFromAssertionsWorker that tightens the signed [lo, hi]
when an interval side is at the type extreme.
libraries.pmi: -14,515 / +6 bytes (489 contexts, 409 size improvements,
2 regressions).
libraries_tests.run: -246k / +0.4k bytes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The bounds-check consumer hardcoded width=32 in EvalRelop, which is unsound on 64-bit targets where fgMorphIndexAddr (morph.cpp:2995-3015) widens the bounds check to TYP_I_IMPL when the index is native int. With a TYP_LONG index whose low 32 bits are provably small but whose high 32 bits are unknown, the fold could prove (uint)idx < (uint)len using only the low half and incorrectly drop a required bounds check (memory corruption). Derive the width from the operand type, matching the other two consumers. Caught by Opus 4.7 (xhigh), Opus 4.8, and GPT-5.5 in parallel review. Also drop the now-dead #include "knownbits.h" from rangecheck.cpp left over from the previous cleanup commit. libraries.pmi: -13,920 bytes (was -14,515; ~600 byte loss is the correctness recovery -- those wins were unsound TYP_LONG bounds-check drops). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+4534
to
+4539
| // See if we can fold the relop based on known bits. This complements the range-based folding | ||
| // above (which is limited to TYP_INT) by reasoning about individual bits and TYP_LONG values. | ||
| if (varTypeIsIntegral(op1) && (op1VN != ValueNumStore::NoVN) && (op2VN != ValueNumStore::NoVN)) | ||
| { | ||
| const unsigned width = genTypeSize(genActualType(op1)) * BITS_PER_BYTE; | ||
| const KnownBits kb1 = KnownBits::Compute(this, op1VN, assertions); |
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.
Closes #105333
Closes #111567
Most optimizing compilers use KnownBits to track ranges, e.g. LLVM, MSVC vs tracking Min-Max values.
We have
Rangewich is 32-bit only and will likely require lots of efforts to make it 64-bit, fix casts between 32-bit and 64-bit, fight with all kinds of overflows and correctness issues and still it's only good fo representing continues ranges.KnownBits looks like this:
My estimate it can bring 500k-600k diffs for win-x64 if extended (I've seen 450k with +400 LOC), current version should be
-320kbon win-x64 with somewhat nices improvements in non-tests collections.I'm not sure it can fully replace Range just like Range can't replace KnownBits, but some usages definitely can be re-routed to KnownBits.
This PR is basically an attempt to mimic LLVM's impl
Not done:
Diffs -
-662244 byteson linux-arm64