[Bounds Checks] Make union/intersect operations more precise #113862

EgorBo · 2025-03-24T23:35:53Z

Closes #113809
Partially addresses #104890 (removes bounds check)

public static int UnrolledSum(Span<int> arr)
{
    int sum = 0;
    int i = 0;
    for (; i < arr.Length - 3; i += 4)
    {
        sum += arr[i + 0];
        sum += arr[i + 1];
        sum += arr[i + 2];
        sum += arr[i + 3];
    }

    // Handle the remaining elements
    for (; i < arr.Length; i++)
        sum += arr[i];
    return sum;
}

We are now able to drop bounds checks completely here:

; Method Tests:UnrolledSum(System.Span`1[int]):int (FullOpts)
-      sub      rsp, 40
       mov      rax, bword ptr [rcx]
       mov      ecx, dword ptr [rcx+0x08]
       xor      edx, edx
       xor      r8d, r8d
       lea      r10d, [rcx-0x03]
       test     r10d, r10d
       jle      SHORT G_M39394_IG05
       align    [0 bytes for IG03]
G_M39394_IG03:
       mov      r9d, r8d
       add      edx, dword ptr [rax+4*r9]
       lea      r9d, [r8+0x01]
       add      edx, dword ptr [rax+4*r9]
       lea      r9d, [r8+0x02]
       add      edx, dword ptr [rax+4*r9]
       lea      r9d, [r8+0x03]
       add      edx, dword ptr [rax+4*r9]
       add      r8d, 4
       cmp      r8d, r10d
       jl       SHORT G_M39394_IG03
       jmp      SHORT G_M39394_IG05
-      align    [0 bytes for IG04]
+      align    [2 bytes for IG04]
G_M39394_IG04:
-      cmp      r8d, ecx
-      jae      SHORT G_M39394_IG08
       mov      r10d, r8d
       add      edx, dword ptr [rax+4*r10]
       inc      r8d
G_M39394_IG05:
       cmp      r8d, ecx
       jl       SHORT G_M39394_IG04
       mov      eax, edx
-      add      rsp, 40
       ret      
-G_M39394_IG08:
-      call     CORINFO_HELP_RNGCHKFAIL
-      int3     
-; Total bytes of code: 99
+; Total bytes of code: 82

src/coreclr/jit/rangecheck.h

EgorBo · 2025-03-25T12:30:41Z

src/coreclr/jit/rangecheck.cpp

-                return l1;
+                // Otherwise, prefer the BinOpArray(preferredBound) over the constant for the upper bound
+                // and the constant for the lower bound.
+                return isLower ? l2 : l1;


In this case we just tighten the range (from multiple facts), it's fine to be liberal here. Seems like for lower bound it's always better to report a constant than trying to be smart with adjusted "bnd + offset"

EgorBo · 2025-03-25T12:31:40Z

src/coreclr/jit/rangecheck.h

@@ -510,10 +510,15 @@ struct RangeOps
        {
            result.lLimit = r1lo;
        }
-        // Widen Upper Limit => Max(k, (a.len + n)) yields (a.len + n),
-        // This is correct if k >= 0 and n >= k, since a.len always >= 0


since a.len always >= 0

NOTE: we've been already relying on this fact in RangeOps::Merge before my change

EgorBo · 2025-03-25T12:33:15Z

Diffs

PTAL @jakobbotsch @dotnet/jit-contrib

jakobbotsch · 2025-03-25T14:43:59Z

src/coreclr/jit/rangecheck.h

+        if (r1lo.IsBinOpArray() && r2lo.IsConstant() && (r1lo.cns <= 0))
+        {
+            result.lLimit = Limit(Limit::keConstant, min(r1lo.cns, r2lo.cns));
+        }
+        if (r2lo.IsBinOpArray() && r1lo.IsConstant() && (r2lo.cns <= 0))
+        {
+            result.lLimit = Limit(Limit::keConstant, min(r2lo.cns, r1lo.cns));
+        }


This is adding more reliance on the unsound limits created based on IsVNCheckedBound, isn't it?
It's a little bit worrying...

Yep, but we either shouldn't do it at all (depends on the bound actually, if it's VNF_ArrLen then it's always fine to rely on) or keep using it. I hope the "is montonic"/overflow analysis make it safe

jakobbotsch

LGTM with a small worry around using facts created based on IsVNCheckedBound

EgorBo · 2025-03-25T15:16:01Z

/ba-g "azurelinux.3 issues"

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 24, 2025

dotnet-policy-service bot assigned EgorBo Mar 24, 2025

Improve RangeOps::Merge and tightenLimit

9545f62

EgorBo force-pushed the improve-range-union-intersection branch from 6f4b5c0 to 9545f62 Compare March 24, 2025 23:37

EgorBo added 3 commits March 25, 2025 01:32

Fix regression

6f694e0

cleanup

42cc471

Remove invalid rule

4c48aed

build-analysis bot mentioned this pull request Mar 25, 2025

System.Net.Quic tests timeout #107761

Open

IDisposable reviewed Mar 25, 2025

View reviewed changes

src/coreclr/jit/rangecheck.h Outdated Show resolved Hide resolved

Fix lLimit assignment in rangecheck.h

ea9c3e6

EgorBo marked this pull request as ready for review March 25, 2025 11:21

EgorBo commented Mar 25, 2025

View reviewed changes

EgorBo requested a review from jakobbotsch March 25, 2025 12:33

jakobbotsch reviewed Mar 25, 2025

View reviewed changes

jakobbotsch approved these changes Mar 25, 2025

View reviewed changes

EgorBo merged commit a92f652 into dotnet:main Mar 25, 2025
104 of 108 checks passed

EgorBo deleted the improve-range-union-intersection branch March 25, 2025 15:34

This was referenced Mar 25, 2025

Redundant bounds check #104890

Open

JIT: Redundant bounds check is not eliminated #109677

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bounds Checks] Make union/intersect operations more precise #113862

[Bounds Checks] Make union/intersect operations more precise #113862

EgorBo commented Mar 24, 2025 •

edited

Loading

EgorBo Mar 25, 2025 •

edited

Loading

EgorBo Mar 25, 2025

EgorBo commented Mar 25, 2025

jakobbotsch Mar 25, 2025

EgorBo Mar 25, 2025

jakobbotsch left a comment

EgorBo commented Mar 25, 2025

[Bounds Checks] Make union/intersect operations more precise #113862

[Bounds Checks] Make union/intersect operations more precise #113862

Conversation

EgorBo commented Mar 24, 2025 • edited Loading

EgorBo Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

EgorBo Mar 25, 2025

Choose a reason for hiding this comment

EgorBo commented Mar 25, 2025

jakobbotsch Mar 25, 2025

Choose a reason for hiding this comment

EgorBo Mar 25, 2025

Choose a reason for hiding this comment

jakobbotsch left a comment

Choose a reason for hiding this comment

EgorBo commented Mar 25, 2025

EgorBo commented Mar 24, 2025 •

edited

Loading

EgorBo Mar 25, 2025 •

edited

Loading