JIT: fix profile propagation in LowerSwitch

[AndyAyersMS]

Recompute switch target block weights rather than trying to fix them incrementally.

Fixes #130785

[AndyAyersMS]

@EgorBo PTAL
fyi @dotnet/jit-contrib

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR adjusts CoreCLR JIT profile-consistency handling in two places: (1) it adds a “dump-only” mode to the debug profile-weight checker so it can log details before asserting, and (2) it ensures edge-likelihood invariants are maintained when conditional branches are collapsed to unconditional branches.

Changes:

• Extend fgDebugCheckProfileWeights with a dump mode and re-run the check with verbose=true to emit diagnostics prior to asserting on inconsistent profile data.
• Normalize the successor edge likelihood when converting a degenerate BBJ_COND to BBJ_ALWAYS.
• Update the Compiler declaration to match the new fgDebugCheckProfileWeights signature.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/fgprofile.cpp	Adds dump mode and a verbose re-run to print profile inconsistency details before asserting.
src/coreclr/jit/fgopt.cpp	Adjusts edge likelihood handling when removing a conditional jump that always targets the same block.
src/coreclr/jit/compiler.h	Updates the fgDebugCheckProfileWeights declaration to include the new optional dump parameter.

Comments suppressed due to low confidence (1)

src/coreclr/jit/fgprofile.cpp:4867

• In dump mode, the unflaggedBlocks path is completely suppressed (no JITDUMP, no assert). This defeats the purpose of re-running with dump=true to log details before asserting, since missing BBF_PROF_WEIGHT flags will still assert without any diagnostic output when verbose was originally false. Consider always emitting the JITDUMP, and only gating the assert on !dump.

    if ((unflaggedBlocks > 0) && !dump)
    {
        JITDUMP("%d blocks are missing BBF_PROF_WEIGHT flag.\n", unflaggedBlocks);
        assert(!"Missing BBF_PROF_WEIGHT flag");
    }

[JulieLeeMSFT]

LGTM.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[AndyAyersMS]

@EgorBo can you re-review. Got a repro finally.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "b78cb450324ea0383476421f9d2765e81c671a4b",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "e7c3d6567c4d78e0c10627fa89939767464478c2",
  "last_reviewed_commit": "b78cb450324ea0383476421f9d2765e81c671a4b",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "e7c3d6567c4d78e0c10627fa89939767464478c2",
  "last_recorded_worker_run_id": "29688126480",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "b78cb450324ea0383476421f9d2765e81c671a4b",
      "review_id": 4730808760
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Fixes #130785, an Inconsistent profile data assertion in Lowering. When LowerSwitch peels the default case and then distributes case likelihoods equally (the zero-remaining-likelihood path), it was updating each switch target's block weight incrementally via increaseBBProfileWeight(newEdgeWeight - oldEdgeWeight). Because the earlier default-peel had already scaled afterDefaultCondBlock's weight but left the target weights stale, the incremental delta accumulated on top of a stale base, producing inconsistent profile weights and tripping the debug checker.

Approach: In the equal-distribution loop, the target's weight is now recomputed from scratch from its incoming edges: targetBlock->setBBProfileWeight(targetBlock->computeIncomingWeight()), replacing the fragile incremental adjustment. This mirrors the established pattern already used across the JIT after edge-likelihood changes (optimizebools.cpp, optimizer.cpp, helperexpansion.cpp). The now-unused oldEdgeWeight/newEdgeWeight locals are removed. Separately, fgDebugCheckProfileWeights gains a dump parameter (debug-only): before asserting on inconsistency, it re-runs itself with dumping forced on so the offending blocks are logged in the JIT dump prior to the assert; the missing-flag assert and the recursion are correctly guarded by !dump to avoid double-assert/infinite recursion.

Summary: This is a small, well-targeted correctness fix in profile-weight propagation plus a helpful debug diagnostic. The recompute-from-incoming-edges approach is consistent with existing JIT conventions and is robust against stale base weights. The dump re-run recursion terminates (guarded by !dump) and restores verbose. Note the sibling scaleFactor branch (the non-zero remaining-likelihood path) only adjusts edge likelihoods and does not touch block weights, so it is unaffected by this class of bug. I found no correctness, performance, or convention issues in the changed lines. Behavior in release builds is limited to the one-line weight recomputation; the rest is DEBUG-only. LGTM.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 60.6 AIC · ⌖ 10.5 AIC · ⊞ 10K