-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT: Don't run optSetBlockWeights
when we have PGO data
#111764
JIT: Don't run optSetBlockWeights
when we have PGO data
#111764
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
I would rather see us strive to get these flags set properly by each phase that manipulates flow. So yet another aspect of pgo post phase checks: if root method has PGO, all blocks must have |
That sounds like a better plan. I suppose a reasonable goal is to enable |
) Part of #107749. Prerequisite to #111764 (comment). Add a post-phase check that ensures BBF_PROF_WEIGHT is set for every reachable block when we have profile data. This check revealed only one site -- when incorporating an inlinee's blocks -- where we couldn't rely on normal flow propagation to set the flag.
Diffs are smaller this time around; some of these are probably from the |
Part of #107749. For PGO scenarios, we plan to rely entirely on profile synthesis to propagate profile-derived weights throughout the flowgraph. This is the first step in transitioning away from the existing weight propagation heuristics. We want to keep these around for non-PGO scenarios for now, though ideally, we'll eventually be able to replace these entirely with profile synthesis (plus branch prediction heuristics when we don't have profile data), and consolidate how the profile is maintained for PGO and non-PGO scenarios.
This had few diffs locally, as
optSetBlockWeights
already avoids modifying profile-derived weights. The diffs seem to stem fromBBF_PROF_WEIGHT
not being propagated to all blocks when we have profile data (such as during inlining). This doesn't seem concerning, since the next profile repair pass (which I've yet to add) will propagate the flag as needed.