Skip to content

Remove bounds check for arr1[arr2.Length] pattern#115980

Merged
EgorBo merged 1 commit intodotnet:mainfrom
EgorBo:fix-bce-regression
May 26, 2025
Merged

Remove bounds check for arr1[arr2.Length] pattern#115980
EgorBo merged 1 commit intodotnet:mainfrom
EgorBo:fix-bce-regression

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented May 25, 2025 •
edited
Loading

Closes #115793

static bool Repro(string prefix, string path)
{
    if (prefix.Length < path.Length)
        return (path[prefix.Length] == '/');

    return false;
}
 ; Method Program:Repro(System.String,System.String):ubyte (FullOpts)
-      sub      rsp, 40
       mov      ecx, dword ptr [rcx+0x08]
-      mov      r8d, dword ptr [rdx+0x08]
-      cmp      ecx, r8d
+      mov      eax, dword ptr [rdx+0x08]
+      cmp      ecx, eax
       jl       SHORT G_M15110_IG05
       xor      eax, eax
-      add      rsp, 40
       ret      
 G_M15110_IG05:
-      cmp      ecx, r8d
-      jae      SHORT G_M15110_IG07
       mov      eax, ecx
       cmp      word  ptr [rdx+2*rax+0x0C], 47
       sete     al
       movzx    rax, al
-      add      rsp, 40
       ret      
-G_M15110_IG07:
-      call     CORINFO_HELP_RNGCHKFAIL
-      int3     
-; Total bytes of code: 53
+; Total bytes of code: 28

A few diffs

@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 May 25, 2025
@build-analysis build-analysis bot mentioned this pull request May 26, 2025
Open
3 tasks
@EgorBo EgorBo marked this pull request as ready for review May 26, 2025 09:49
Copilot AI review requested due to automatic review settings May 26, 2025 09:49
Copilot AI reviewed May 26, 2025
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 updates the JIT range-check logic to recognize the new GT_ARR_LENGTH node and remove an unnecessary bounds check for patterns like arr1[arr2.Length], improving codegen size and performance.

  • Treat GT_ARR_LENGTH as non-overflowing in ComputeDoesOverflow
  • Provide a concrete range (0 to CORINFO_Array_MaxLength) for GT_ARR_LENGTH in ComputeRange
  • Eliminate redundant range‐check helper paths, reducing code size
Comments suppressed due to low confidence (1)

src/coreclr/jit/rangecheck.cpp:1647

  • Consider adding unit tests for the GT_ARR_LENGTH case to validate that the computed range (0 to CORINFO_Array_MaxLength) behaves as expected and guard against future regressions in this scenario.
else if (expr->OperIs(GT_ARR_LENGTH))

@EgorBo
Copy link
Member Author

EgorBo commented May 26, 2025

PTAL @jakobbotsch @dotnet/jit-contrib small change, a couple of diffs

@EgorBo EgorBo requested a review from jakobbotsch May 26, 2025 11:04
@EgorBo EgorBo enabled auto-merge (squash) May 26, 2025 17:14
@EgorBo
Copy link
Member Author

EgorBo commented May 26, 2025

/ba-g "NativeAOT timeouts"

@EgorBo EgorBo merged commit ac7f8e0 into dotnet:main May 26, 2025
107 of 111 checks passed
@EgorBo EgorBo deleted the fix-bce-regression branch May 26, 2025 17:15
@rameel rameel mentioned this pull request May 28, 2025
Open
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@jakobbotsch jakobbotsch jakobbotsch approved these changes

Assignees

@EgorBo EgorBo

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression: Unnecessary bounds check generated

3 participants

@EgorBo @jakobbotsch