Skip to content

Conversation

amanasifkhalid
Copy link
Contributor

Motivating example. BB_COLD_WEIGHT is currently 0.01, suggesting blocks that execute less than 1% of the time on average should be considered cold by block layout. However, BasicBlock::isBBWeightCold compares the block's normalized weight (with BB_UNITY_WEIGHT scaling) against BB_COLD_WEIGHT, so the actual cutoff for cold blocks is currently 0.01%. This change removes the BB_UNITY_WEIGHT scaling to get the intended behavior.

@Copilot Copilot AI review requested due to automatic review settings June 11, 2025 17:38
@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 Jun 11, 2025
Copy link
Contributor

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

Copy link
Contributor

@Copilot 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 adjusts BasicBlock::isBBWeightCold to compare the raw block weight against the product of call count and the cold-weight threshold, removing the previous unity-weight normalization.

  • Swaps getBBWeight(comp) for bbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT)
  • Aims to restore the intended 1% cutoff for cold blocks rather than 0.01%
Comments suppressed due to low confidence (2)

src/coreclr/jit/block.h:1309

  • There are no tests covering the updated isBBWeightCold behavior; consider adding unit tests for blocks exactly at, just below, and just above the computed cold threshold.
return bbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT);

src/coreclr/jit/block.h:1309

  • The new logic may misclassify blocks when getCalledCount(comp) returns zero, causing all blocks to appear hot. Consider guarding against a zero call count or falling back to a default behavior for unknown call counts.
return bbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT);

@amanasifkhalid
Copy link
Contributor Author

cc @dotnet/jit-contrib, @AndyAyersMS PTAL. Diffs have quite a bit of churn in both directions, though I'm taking solace in the relatively small PerfScore diffs, since this primarily affects the placement of cold code. I expect this to be a notable regression only in cases where we mistakenly give hot blocks near-zero weight, and we were likely already doing a subpar job with such cases (example). Thanks!

@amanasifkhalid
Copy link
Contributor Author

ping @AndyAyersMS

@amanasifkhalid
Copy link
Contributor Author

/ba-g unrelated wasm build failure

@amanasifkhalid amanasifkhalid merged commit d3e2f5e into dotnet:main Jun 13, 2025
107 of 109 checks passed
@amanasifkhalid amanasifkhalid deleted the cold-weight-cutoff branch June 13, 2025 21:53
@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

2 participants