Invalidate PGO after Wasm SCC transform - #133122
Closed
lewing wants to merge 1 commit into
Closed
Conversation
The Wasm SCC transform reroutes SCC entry flow through a dispatcher, which can leave incorporated PGO block weights structurally inconsistent with the rewritten CFG. Mark the profile data inconsistent after the transform so checked profile validation does not assert on stale weights. Fixes #133120 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db8401cb-c388-4481-a625-34811ab8a269
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Azure Pipelines: Successfully started running 6 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. |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, matches an established pattern in the same file for CFG-rewriting passes, and the only feedback is a minor comment-wording nit.
Pull request overview
This PR updates the Wasm SCC transform phase to conservatively invalidate PGO “consistency” after CFG rewriting, preventing post-phase profile-weight verification asserts when multi-entry SCCs are rewritten through a dispatcher.
Changes:
- Mark
fgPgoConsistent = falseafterWasmTransformSccsruns (when irreducible loops are transformed). - Add a
JITDUMPnote explaining why profile data is considered inconsistent after SCC entry flow is rerouted.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/fgwasm.cpp | Invalidates PGO consistency after SCC transform rewires entry flow through a dispatcher to avoid checked profile verification asserts. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+1563
to
+1564
| // Weight moved from the SCC entry blocks through the dispatcher, so any | ||
| // profile data we had is no longer self-consistent. |
Member
|
I'd prefer to try and fix the profile, let me see if that looks tractable. If not, we can take this. |
Member
|
#133132 fixes the weights -- if you can validate it, I'd prefer that fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Wasm SCC transform rewrites multi-entry SCCs to route entry flow through a dispatcher. When profile data has already been incorporated, that rewrite can leave block weights structurally inconsistent with the new CFG and trigger checked JIT profile verification asserts.
This conservatively marks PGO data inconsistent after the SCC transform, matching the existing approach used by
fgWasmRepairTryEntrieswhen it reroutes weighted flow.Fixes #133120.
Validation
./build.sh clr+libs+hostpython3 src/coreclr/scripts/jitformat.py -r . -o osx -a arm64./build.sh clrNote
This PR description was generated by GitHub Copilot.