Skip to content

JIT: Add RangeOps::Xor and RangeOps::Not#126553

Open
BoyBaykiller wants to merge 6 commits intodotnet:mainfrom
BoyBaykiller:add-xor-handling-to-rangecheck
Open

JIT: Add RangeOps::Xor and RangeOps::Not#126553
BoyBaykiller wants to merge 6 commits intodotnet:mainfrom
BoyBaykiller:add-xor-handling-to-rangecheck

Conversation

@BoyBaykiller
Copy link
Copy Markdown
Contributor

@BoyBaykiller BoyBaykiller commented Apr 4, 2026

Fix multiple regression from #126529.

One example: ParallelEnumerable.Range.
Simplified:

void TightenRangeXor(int count)
{
    if (count > 0 && count < 500)
    {
        // count range: [<1, 499>]
        // count ^ INT_MAX is equivalent to INT_MAX - x
        // new count range: [INT_MAX-499, INT_MAX - 1]
        // => can remove branch
        if ((count ^ int.MaxValue) < (count - 1000))
        {
            throw new ArgumentOutOfRangeException(nameof(count));
        }
    }
}
G_M64805_IG02:  ;; offset=0x0000
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

@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 Apr 4, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 4, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

@EgorBo
Copy link
Copy Markdown
Member

EgorBo commented Apr 5, 2026

CI is down so we can't see the diffs.

Original repro: ParallelEnumerable.Range.

um.. I don't see XOR there - does it appear there after some jit transformation?

@EgorBo EgorBo requested a review from Copilot April 5, 2026 13:15
@EgorBo
Copy link
Copy Markdown
Member

EgorBo commented Apr 5, 2026

@MihuBot

Copy link
Copy Markdown
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 CoreCLR JIT range analysis to understand xor so that range tightening (and downstream optimizations like branch removal) keep working after the CONST - xxor x, CONST transformation from #126529.

Changes:

  • Add RangeOps::Xor to compute ranges for a limited set of XOR patterns (constant-folding and ^ -1 / ^ INT_MAX).
  • Teach RangeCheck::GetRangeFromAssertions to compute XOR ranges from VN assertions (VNF_XOR).
  • Use RangeOps::Xor in RangeCheck::ComputeRangeForBinOp for GT_XOR (while preserving the existing Log2 special-case).

Reviewed changes

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

File Description
src/coreclr/jit/rangecheck.h Adds RangeOps::Xor to enable tighter range inference for certain XOR cases.
src/coreclr/jit/rangecheck.cpp Wires XOR into assertion-based range derivation and general binop range computation.

Comment thread src/coreclr/jit/rangecheck.h Outdated
Comment on lines +496 to +502
// x ^ -1 is equivalent to -1 - x
// x ^ INT_MAX is equivalent to INT_MAX - x
// Example: [3..5] ^ [-1..-1] = [-6..-4]
if (r1IsConstVal && (r1ConstVal == -1 || r1ConstVal == INT_MAX))
{
return Subtract(r1, r2);
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

RangeOps::Xor maps the ^ -1 case to Subtract, but Subtract delegates through Negate which returns unknown for constant ranges that include INT_MIN. Since XOR/bitwise-NOT has no overflow semantics, this unnecessarily loses precision for ranges like [INT_MIN..k] ^ [-1..-1]. Consider handling the -1 case directly (e.g., by computing bitwise-NOT of constant limits) so it can still produce a constant range when the input range is constant.

Copilot uses AI. Check for mistakes.
Comment thread src/coreclr/jit/rangecheck.h Outdated
{
return Subtract(r2, r1);
}

Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

Remove the trailing whitespace on this blank line to satisfy repo formatting/linting expectations.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +1555 to +1557
case GT_XOR:
r = RangeOps::Xor(op1Range, op2Range);
break;
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

This change is meant to prevent a performance regression by allowing range tightening through XOR (e.g., x ^ int.MaxValue) again. Please add a JIT regression test (likely under src/tests/JIT/opt/RangeChecks or similar) that fails without this change and verifies the relevant optimization (e.g., absence of the throw helper / simplified control flow) so future range-analysis tweaks don’t reintroduce the regression.

Copilot uses AI. Check for mistakes.
@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

um.. I don't see XOR there - does it appear there after some jit transformation?

Yes it gets produced by my other PR which I linked.
Currently this doesnt produce any diffs. But once #126529 is merged it will (fix the regression)

@EgorBo
Copy link
Copy Markdown
Member

EgorBo commented Apr 5, 2026

I'd say overflow doesn't exist for XOR but I saw OR also has some overflow handling

You're touching code GetRangeFromAssertions that doesn't rely on a separate path for overflow checking, so it's fine as long as it's correct.

@EgorBo
Copy link
Copy Markdown
Member

EgorBo commented Apr 5, 2026

um.. I don't see XOR there - does it appear there after some jit transformation?

Yes it gets produced by my other PR which I linked. Currently this doesnt produce any diffs. But once #126529 is merged it will (fix the regression)

well, then for now it's just a bunch of untested/never-triggered code without tests?

It looks correct to me, although, I think we should make Xor handling a bit smarter than focusing only on -1/maxvalue constants. E.g. the Log2 pattern involves ^ 31/^ 63 - if we can remove special handling for log2 and recognize it naturally..

@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

BoyBaykiller commented Apr 5, 2026

well, then for now it's just a bunch of untested/never-triggered code without tests

Well my hope is that the other PR gets merged first soon (which is why this is draft for now)

It looks correct to me, although, I think we should make Xor handling a bit smarter than focusing only on -1/maxvalue constants. E.g. the Log2 pattern involves ^ 31/^ 63 - if we can remove special handling for log2 and recognize it naturally

Yeah that'd be nice. But it seems like XOR alone is not enough to handle that. So maybe for an other PR? Although let me see if xor has a more general pattern

@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

Adding Not handling causes some diffs (even before #126529 is merged)

…ssion in System.Data.SqlTypes.SqlDecimal:AdjustScale 114664 libraries.pmi

* add Not handling to ComputeRange
@BoyBaykiller BoyBaykiller changed the title JIT: Add and use RangeOps::Xor JIT: Add RangeOps::Xor and RangeOps::Not Apr 12, 2026
EgorBo added a commit that referenced this pull request Apr 28, 2026
Given any `CONST - x` and range info for `x` obtained from
`IntegralRange::ForNode` see if we can transform it to `x ^ CONST`.
These changes motivate improvements in `IntegralRange::ForNode` and
`RangeOps` (#126553). It also made
me find a missing `GT_NOT` in asserprop to enable VN based folding.

Found by looking at diffs from
#126442

Example:
```cs
int Byte255(byte x)
{
    return 255 - x;
}
```
```assembly
;; ------ BASE
G_M64250_IG02:  ;; offset=0x0000
       movzx    rax, dl
       neg      eax
       add      eax, 255
						;; size=10 bbWeight=1 PerfScore 0.75
G_M64250_IG03:  ;; offset=0x000A
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

;; ------ DIFF
G_M64250_IG02:  ;; offset=0x0000
       movzx    rax, dl
       xor      eax, 255
						;; size=8 bbWeight=1 PerfScore 0.50
G_M64250_IG03:  ;; offset=0x0008
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00
```

```cs
int IntMin1(int x)
{
    return -1 - x;
}
```
```assembly
;; ------ BASE
G_M50835_IG02:  ;; offset=0x0000
       mov      eax, edx
       neg      eax
       dec      eax
						;; size=6 bbWeight=1 PerfScore 0.75
G_M50835_IG03:  ;; offset=0x0006
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00
;; ------ DIFF
G_M50835_IG02:  ;; offset=0x0000
       mov      eax, edx
       not      eax
						;; size=4 bbWeight=1 PerfScore 0.50
G_M50835_IG03:  ;; offset=0x0004
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00
```

---------

Co-authored-by: Egor Bogatov <egorbo@gmail.com>
@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

BoyBaykiller commented Apr 28, 2026

The XOR handling here is very general, but that's not enough to catch the log2 pattern of 31 ^ LZNCT(x | 1) (#113790). We also need a good range for LZNCT. Only when LZNCT(x) has a upper limit below 32 it works. In this case we have x | 1, so LZNCT must return below 32 which makes it safe. But we don't have that analysis. Right now we just return unknown range for LZNCT (VNF_LeadingZeroCount never gets hit should use VNF_HWI_AVX2_LeadingZeroCount etc). See https://godbolt.org/z/3fqjxhc1E

Here is an example showing that XOR can deal with it:

void Test(int value)
{
    value |= 1;
    value = (int)Lzcnt.LeadingZeroCount((uint)value);

    // Range for value must be [0, 31] but rangecheck doesn't
    // know so let's help it with this branch
    if (value >= 0 && value <= 31)
    {
        // Example array from https://github.com/dotnet/runtime/pull/113790
        ReadOnlySpan<byte> log2ToPow10 = stackalloc byte[32];

        // New! No bounds check, it sees XOR is equivalent to SUB here
        int index = 31 ^ value;
        _ = log2ToPow10[index];
    }
}

It shouldnt be too hard to implement better range for LZCNT(x | 1) on 32 bit (using the BitsetFromRange I've added), so that it gets handled naturally. But the original issues uses 64 bit which we don't support at all, so the hack must stay either way...

@BoyBaykiller BoyBaykiller marked this pull request as ready for review April 28, 2026 17:13
@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

@EgorBo PTAL.

return result;
}

static Range Not(const Range& range)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

last I checked Not produced 0 diffs, so we'd better remove it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#126553 (comment) Here I was talking about 2 large diffs in libraries_tests_no_tiered_compilation caused by NOT handling.

Additionally it fixes regressions from my recent transformation (#126529).

Example regression in libraries.pmi in SqlDecimal:op_Multiply (diffs):
image

Which this fixes by adding NOT handling (diffs):
image

Comment thread src/coreclr/jit/morph.cpp
knownBits &= (1ULL << (sizeInBits - 1)) - 1;

bool noCarry = (cns & knownBits) == knownBits;
if (noCarry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what does this rewrite do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just factored out the BitsetFromRange since we now use it in morph and rangecheck and adjusted the code a little. We can use BitsetFromRange to do other stuff for example transform ADD to OR if we wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants