JIT: fix profile likelihood and overflow UB in optimizebools

[AndyAyersMS]

Fix two issues in optOptimizeBoolsUpdateTrees and GetIntersection:

1. The !sameTarget branch likelihood formula was computing (1-p1) + p1p2_false = 1 - p1p2_true, but the correct probability of reaching B2's true target is (1-p1) * p2_true (B1 falls through, then B2 jumps). Also fixed the misleading comment.

2. The normalize lambda in GetIntersection could overflow when cns == SSIZE_T_MAX and cmp == GT_GT (signed overflow is UB in C++). Added an explicit guard and made the lambda return bool to signal failure.

3. Updated a stale comment referencing the nonexistent GTF_BOOLEAN flag in optIsBoolComp.

[dotnet-policy-service]

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

[AndyAyersMS]

An AI review of optimizebools independently found the issue fixed in #128928 and also spotted a few other small things. These are the other small things.

Minimal diffs (mostly from the profile update fix).

@EgorBo PTAL
fyi @dotnet/jit-contrib

[Copilot]

Pull request overview

This PR tightens correctness and safety in the JIT boolean/range optimization code paths by (1) fixing a likelihood computation used during boolean-block folding and (2) removing undefined behavior risk in range-intersection normalization, plus a small comment cleanup.

Changes:

• Fixes the !sameTarget edge likelihood computation in OptBoolsDsc::optOptimizeBoolsUpdateTrees() to reflect the actual probability path to B2’s true target.
• Hardens GetIntersection by preventing signed overflow UB when normalizing X > SSIZE_T_MAX (and makes normalization explicitly fail-fast).
• Updates a stale comment in optIsBoolComp that referenced a nonexistent GTF_BOOLEAN flag.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #128951

Holistic Assessment

Motivation: All three fixes address real bugs: a mathematically incorrect profile likelihood formula, potential signed overflow UB in C++, and a misleading stale comment. These are legitimate correctness issues.

Approach: The fixes are minimal, surgical, and correct. Each change addresses exactly the identified problem without unnecessary restructuring.

Summary: ✅ LGTM. The three changes are independently correct. The likelihood formula fix properly computes P(B1 falls through) × P(B2 jumps) = (1-p1) × p2_true. The overflow guard handles the SSIZE_T_MAX edge case cleanly. The comment and redundant check cleanup are straightforward.

---

Detailed Findings

✅ Correctness — Likelihood formula fix (line 1253)

The old formula (1-p1) + p1*p2_false was computing a value that could exceed 1.0 and was semantically wrong. In the !sameTarget (AND) case, B2's true target is reached only when B1 falls through (probability 1-p1) AND B2 then jumps (probability p2_true). The corrected formula (1-p1) * p2_true is the correct joint probability.

✅ Correctness — Overflow guard in GetIntersection (line 455)

When cns == SSIZE_T_MAX and cmp == GT_GT, computing cns + 1 is signed integer overflow (undefined behavior in C++). The early return is the right fix. The GT_LT case is safe because the function already rejects negative constants at line 444, so cns - 1 when cns >= 0 produces at minimum -1, which is representable in ssize_t.

✅ Cleanup — Redundant OperIs check removed (line 1423)

IsIntegralConst(0) already checks OperIs(GT_CNS_INT) internally, so the prior opr1->OperIs(GT_CNS_INT) guard was redundant. The updated comment accurately reflects the current logic.

💡 Minor — Consider SSIZE_T_MIN guard for GT_LT

The comment on line 465 explains why GT_LT subtraction is safe (cns is non-negative). This is correct. However, if the non-negative check at line 444 were ever relaxed in the future, the GT_LT path would need a similar guard for cns == SSIZE_T_MIN. This is a follow-up consideration, not a blocking issue.

Generated by Code Review for issue #128951 · ● 2.5M · ◷

[AndyAyersMS]

diffs