JIT: Fix profile inconsistency asserts in flow graph optimization#127357
JIT: Fix profile inconsistency asserts in flow graph optimization#127357AndyAyersMS wants to merge 4 commits intodotnet:mainfrom
Conversation
Fix three sources of profile weight inconsistency in fgopt.cpp that cause assert failures under PGO stress modes (JitRandomEdgeCounts, JitRandomGuardedDevirtualization): 1. fgCompactBlock: When compacting a block with a target that had other predecessors removed by earlier transforms, the inherited weight may not match incoming edge weights. Detect this and mark fgPgoConsistent false. 2. fgOptimizeBranchToEmptyUnconditional: When fgRedirectEdge merges two edges of a BBJ_COND (TrueEdge == FalseEdge), the merged edge retains the old likelihood instead of 1.0. Fix by setting likelihood to 1.0 after merge. 3. fgOptimizeBranchToEmptyUnconditional: When decreaseBBProfileWeight would clamp a block's weight to zero, mark fgPgoConsistent false. Also fix three pre-existing misleading JITDUMP messages that said 'Data %s consistent' when marking data as inconsistent. Fixes dotnet#126381. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR addresses assert failures under PGO stress modes by tightening/repairing profile consistency tracking in fgopt.cpp during flow-graph optimizations, and fixes a few misleading JITDUMP messages that reported “consistent” while marking the profile as inconsistent.
Changes:
- In
fgCompactBlock, detects post-compaction mismatches between block weight and summed incoming edge flow and marksfgPgoConsistent = false. - In
fgOptimizeBranchToEmptyUnconditional, fixes the likelihood on a merged conditional edge (true/false now target the same block) by setting it to1.0. - Marks profile inconsistent when
decreaseBBProfileWeightwould clamp a block’s weight, and corrects several “Data %s consistent” JITDUMP strings to say “inconsistent”.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use block->hasProfileWeight() (post-inheritWeight) instead of the cached pre-inheritance value, so the check also fires when an unprofiled block inherits profile weight from a profiled target. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses several sources of PGO/profile-weight inconsistency in CoreCLR JIT flow-graph optimizations (fgopt.cpp) that can trigger asserts under PGO stress modes, and corrects misleading debug dump text when the JIT marks profile data as inconsistent.
Changes:
- In
fgCompactBlock, detect cases where compaction can introduce profile-weight mismatches and markfgPgoConsistent = false. - In
fgOptimizeBranchToEmptyUnconditional, correct likelihood afterfgRedirectEdgemerges conditional edges (ensuring the merged edge has likelihood 1.0), and markfgPgoConsistent = falsewhen profile weight clamping would occur. - Fix JITDUMP messages that previously said “Data %s consistent” while transitioning profile data to an inconsistent state.
hasLikelihood() is #ifdef DEBUG only and would break Release builds. Remove the guard and sum getLikelyWeight() unconditionally, since all edges have likelihoods set by this point in compilation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@dotnet/jit-contrib PTAL A very small set of diffs |
Fix three sources of profile weight inconsistency in fgopt.cpp that cause assert failures under PGO stress modes (JitRandomEdgeCounts, JitRandomGuardedDevirtualization):
fgCompactBlock: When compacting a block with a target that had other predecessors removed by earlier transforms, the inherited weight may not match incoming edge weights. Detect this and mark fgPgoConsistent false.
fgOptimizeBranchToEmptyUnconditional: When fgRedirectEdge merges two edges of a BBJ_COND (TrueEdge == FalseEdge), the merged edge retains the old likelihood instead of 1.0. Fix by setting likelihood to 1.0 after merge.
fgOptimizeBranchToEmptyUnconditional: When decreaseBBProfileWeight would clamp a block's weight to zero, mark fgPgoConsistent false.
Also fix three pre-existing misleading JITDUMP messages that said 'Data %s consistent' when marking data as inconsistent.
Fixes #126381.