JIT: Use integral ranges to narrow comparisons with memory operands - #134075
jakobbotsch merged 2 commits into
Conversation
Generalize comparison containment to constants and operands whose IntegralRange fits the small memory operand, without changing operand types or removing casts. Share GetCompareSize between lowering, codegen, and x86 register allocation. Adjust unsigned conditions after containment using established operand ranges, and pass comparison sizes directly to flag-reuse checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4c28b3c3-cf6c-4149-8eb1-964831492011
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
cc @dotnet/jit-contrib PTAL @dhartglassMSFT @EgorBo Diffs. Mainly fixing a source of regressions I noticed in #134048. The idea is that when we have a comparison done with a 1 or 2 byte load, then that load can be contained if we can do the comparison as a 1 or 2 byte comparison. We did that already when the other operand was small typed -- this PR expands it to allow the optimization in more cases via Some of the diffs have an unnecessary widening done before them. E.g. It isn't completely trivial because casts may have contained operands so deleting them is not completely straightforward. I will leave it for a potential follow-up. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The unsigned rewrite can trigger a DEBUG assertion, and a focused regression test is still needed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This pull request broadens x86/x64 JIT comparison containment using integral ranges and centralizes comparison-width calculation.
Changes:
- Enables narrower memory comparisons when operand ranges permit.
- Shares
GetCompareSize()across lowering, register allocation, and code generation. - Updates unsigned-condition handling and x86 byte-register constraints.
| File | Summary |
|---|---|
src/coreclr/jit/lsrabuild.cpp |
Applies comparison width to x86 register constraints. |
src/coreclr/jit/lowerxarch.cpp |
Adds range-aware containment and condition handling. |
src/coreclr/jit/lower.cpp |
Removes prior comparison narrowing logic. |
src/coreclr/jit/gentree.h |
Declares the comparison-size helper. |
src/coreclr/jit/gentree.cpp |
Implements comparison-size calculation. |
src/coreclr/jit/codegenxarch.cpp |
Uses shared width for emission and flag reuse. |
src/coreclr/jit/codegen.h |
Updates the flag-reuse helper signature. |
Avoid newly narrowing backend flag producers after their condition has moved to a consumer. Only set unsigned semantics on operators supported by SetUnsigned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4c28b3c3-cf6c-4149-8eb1-964831492011
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Requested x86/x64 coverage and codegen, benchmark, and compile-time results remain outstanding.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
Resolved since last review (2)
|
/azp run runtime, runtime-coreclr superpmi-diffs |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Generalize comparison containment to constants and operands whose IntegralRange fits the small memory operand, without changing operand types or removing casts.
Share GetCompareSize between lowering, codegen, and x86 register allocation. Adjust unsigned conditions after containment using established operand ranges, and pass comparison sizes directly to flag-reuse checks.