Skip to content

Optimize more bound checks around logical operators #122263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
EgorBo wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Optimize more bound checks around logical operators #122263

EgorBo wants to merge 5 commits into from
+177 −181

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Dec 6, 2025

Closes #122150
Example:

int Test(byte inData)
{
    ReadOnlySpan<byte> base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="u8;
    return base64[((inData & 0x03) << 4) | ((inData & 0xf0) >> 4)];
}

Was:

; Method Program:Test(byte):int:this (FullOpts)
       sub      rsp, 40
       movzx    rax, dl
       mov      ecx, eax
       and      ecx, 3
       shl      ecx, 4
       and      eax, 240
       sar      eax, 4
       or       eax, ecx
       cmp      eax, 65
       jae      SHORT G_M54716_IG04
       mov      rcx, 0x196B4CC2D88      ; static handle
       movzx    rax, byte  ptr [rcx+rax]
       add      rsp, 40
       ret      
G_M54716_IG04:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code: 55

Now:

; Method Program:Test(byte):int:this (FullOpts)
       movzx    rax, dl
       mov      ecx, eax
       and      ecx, 3
       shl      ecx, 4
       and      eax, 240
       sar      eax, 4
       or       eax, ecx
       mov      rcx, 0x171D74A2D88      ; static handle
       movzx    rax, byte  ptr [rcx+rax]
       ret      
; Total bytes of code: 36

As the result, was able to optimize bound checks in Convert.ConvertToBase64Array

@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 Dec 6, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Dec 6, 2025

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@EgorBo
Copy link
Member Author

EgorBo commented Dec 6, 2025

@MihuBot

@MihuBot MihuBot mentioned this pull request Dec 6, 2025
Open
@build-analysis build-analysis bot mentioned this pull request Dec 6, 2025
Closed
am11
am11 reviewed Dec 7, 2025
View reviewed changes
src/libraries/System.Private.CoreLib/src/System/Convert.cs
int i;

// get a pointer to the base64 table to avoid unnecessary range checking
fixed (byte* base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="u8)
Copy link
Member

@am11 am11 Dec 7, 2025

Choose a reason for hiding this comment

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

Can we remove unsafe from ConvertToBase64Array's two callers?

Copy link
Member Author

@EgorBo EgorBo Jan 11, 2026

Choose a reason for hiding this comment

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

@am11 do you mean bounds checks over outChars? last I checked that still emits checks, I'll take a look separately

@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jan 6, 2026

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@EgorBo EgorBo reopened this Jan 9, 2026
@EgorBo EgorBo closed this Jan 11, 2026
@EgorBo EgorBo force-pushed the simplify-range-check branch from 90ae7aa to d2e3c3c Compare January 11, 2026 19:43
@EgorBo EgorBo reopened this Jan 11, 2026
@azure-pipelines

This comment was marked as off-topic.

Sign in to view
@EgorBo
Copy link
Member Author

EgorBo commented Jan 11, 2026

/azp run Fuzzlyn

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 11, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo EgorBo marked this pull request as ready for review January 11, 2026 21:12
Copilot AI review requested due to automatic review settings January 11, 2026 21:12
Copilot AI reviewed Jan 11, 2026
View reviewed changes
Copy link
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 optimizes bounds check elimination for expressions involving bitwise AND, OR, and shift operations. The JIT compiler's range check analysis is enhanced to understand value ranges produced by these operations, enabling automatic elimination of bounds checks in patterns like base64 encoding where bitwise operations produce values that are provably within array bounds.

Changes:

  • Enhanced JIT range check analysis to compute ranges for GT_AND, GT_OR, and GT_UMOD operations
  • Refactored ComputeRangeForBinOp to use a cleaner switch-based dispatch and lambda for range computation
  • Modified System.Private.CoreLib's Convert.ConvertToBase64Array to use ReadOnlySpan instead of fixed pointers, relying on the enhanced range check elimination
  • Added natvis debugging visualizations for Limit and Range types

Reviewed changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/Convert.cs Changed ConvertToBase64Array from using fixed pointer to ReadOnlySpan, removing one level of indentation; logic otherwise unchanged
src/coreclr/jit/rangecheck.h Added new range operations: ShiftLeft, And, Or, and UnsignedMod to compute value ranges for these binary operations
src/coreclr/jit/rangecheck.cpp Refactored ComputeRangeForBinOp to use switch statement dispatch and added support for GT_OR; removed GT_RSZ from range computation
src/coreclr/jit/clrjit.natvis Added debugging visualizers for Limit and Range types

@EgorBo EgorBo force-pushed the simplify-range-check branch from 1e09cca to b082f0e Compare January 11, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@am11 am11 am11 left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@EgorBo EgorBo

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI reduce-unsafe

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bound Check for arr[(X & 0x3) << 4] pattern

2 participants

@EgorBo @am11