Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 24, 2026

Description

RangeOps::And was producing incorrect range estimates when one operand had an unknown or wide range with a negative lower bound. This caused the JIT to make incorrect optimization decisions.

Repro:

public static sbyte s_4 = 1;
public static void Main()
{
    // Debug: condition is false (correct)
    // Release: condition was true (bug)
    if (35009 > (1264240267 & (sbyte)(-s_4)))
    {
        System.Console.WriteLine(1UL);
    }
}

[1264240267..1264240267] & [-128..127] = [0..127] which is incorrect

Changes

  • Range::IsSingleConstValue — New helper to check if both limits are equal constants
  • RangeOps::And — Rewritten to be more conservative:
    • Both operands single constants → return actual AND result
    • One operand is a non-negative single constant → return [0, constant]
    • Otherwise → return unknown (instead of potentially incorrect bounds)
  • RangeOps::UnsignedMod — Simplified to use IsSingleConstValue helper
  • Regression testRuntime_123583.cs
Original prompt

File a PR to fix #123583 bug.

This RangeOps::And implementation should fix it - use it:

static Range And(const Range& r1, const Range& r2)
{
    // Conservatively expect at least one operand to be a single-value constant.
    // The math gets too complicated otherwise (and rarely useful in practice).
    int r1ConstVal, r2ConstVal;
    bool r1IsConstVal = r1.IsSingleConstValue(&r1ConstVal);
    bool r2IsConstVal = r2.IsSingleConstValue(&r2ConstVal);

    // Both ranges are single constant values.
    // Example: [7..7] & [3..3] = [3..3]
    if (r1IsConstVal && r2IsConstVal)
    {
        return Range(Limit(Limit::keConstant, r1ConstVal & r2ConstVal));
    }

    // One of the ranges is a single constant value.
    // Example: [unknown..unknown] & [3..3] = [0..3]
    if (r1IsConstVal && (r1ConstVal >= 0))
    {
        return Range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, r1ConstVal));
    }
    if (r2IsConstVal && (r2ConstVal >= 0))
    {
        return Range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, r2ConstVal));
    }

    // Otherwise, we cannot determine the result range.
    return Range(Limit(Limit::keUnknown));
}

implement Range::IsSingleConstValue.

Also, add a regression test for this change, use src/tests/JIT/Regression/JitBlue/Runtime_122288/Runtime_122288.cs as a reference, but name it Runtime_123583 and use the repro from the issue description.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix RangeOps::And implementation to resolve issue #123583 Fix RangeOps::And incorrect range analysis for bitwise AND Jan 24, 2026
Copilot AI requested a review from EgorBo January 24, 2026 23:22
@EgorBo
Copy link
Member

EgorBo commented Jan 24, 2026

@copilot add a comment on top of IsSingleConstValue:

// Check if the range represents a single constant value. Example: [7..7]

Also, use IsSingleConstValue in RangeOps::UnsignedMod (to simplify impl)

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 24, 2026

@copilot add a comment on top of IsSingleConstValue:

// Check if the range represents a single constant value. Example: [7..7]

...

Done in c11cc3a. Added the comment and simplified UnsignedMod to use IsSingleConstValue.

@EgorBo EgorBo marked this pull request as ready for review January 25, 2026 16:30
Copilot AI review requested due to automatic review settings January 25, 2026 16:30
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 fixes a critical bug in the JIT compiler's range analysis for bitwise AND operations. The bug caused incorrect optimization decisions when one operand had a wide range with a negative lower bound, leading to wrong program behavior in release builds.

Changes:

  • Fixed RangeOps::And to be more conservative and correct in its range analysis
  • Added Range::IsSingleConstValue helper method to check if a range represents a single constant
  • Simplified UnsignedMod to use the new helper
  • Added regression test to prevent future regressions

Reviewed changes

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

File Description
src/coreclr/jit/rangecheck.h Added IsSingleConstValue helper; rewrote And to fix incorrect range calculation; simplified UnsignedMod; improved const-correctness
src/tests/JIT/Regression/JitBlue/Runtime_123583/Runtime_123583.cs Added regression test to verify the fix prevents incorrect optimization of bitwise AND with mixed-sign ranges

@dotnet-policy-service
Copy link
Contributor

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

@EgorBo
Copy link
Member

EgorBo commented Jan 26, 2026

@copilot resolve the conflicts

Copy link
Contributor Author

Copilot AI commented Jan 26, 2026

@copilot resolve the conflicts

I'm unable to resolve merge conflicts directly. My environment doesn't support pulling branches from GitHub or force pushing changes needed for conflict resolution.

Could you please merge or rebase the branch locally? The conflicts are in src/coreclr/jit/rangecheck.h - the main branch added IsUndef() and IsFullRange() methods to the Range struct, which need to be merged alongside my IsSingleConstValue method.

@EgorBo
Copy link
Member

EgorBo commented Jan 30, 2026

PTAL @jakobbotsch @dotnet/jit-contrib

Simplifies the RangeOps:And to fix the Fuzzlyn bug, it now expects one of the ranges to be a constant.

@EgorBo EgorBo requested a review from jakobbotsch January 30, 2026 09:21
@jakobbotsch
Copy link
Member

I guess this is fixing #123790 too? (Not sure if it is a duplicate or not, I couldn't find an open issue)

@EgorBo
Copy link
Member

EgorBo commented Jan 30, 2026

I guess this is fixing #123790 too? (Not sure if it is a duplicate or not, I couldn't find an open issue)

Let me check

@EgorBo EgorBo enabled auto-merge (squash) January 30, 2026 10:04
@EgorBo
Copy link
Member

EgorBo commented Jan 30, 2026

/ba-g deadletter

@EgorBo EgorBo merged commit d2a6e3b into main Jan 30, 2026
124 of 131 checks passed
@EgorBo EgorBo deleted the copilot/fix-range-ops-and-implementation branch January 30, 2026 10:05
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fuzzlyn: incorrect RangeOps::And calculation

4 participants