JIT: create phase for incorporation of profile data #47646
Conversation
Instead of trying to set block weight data when initially building the flowgraph, do it as a phase just before importation. The importer looks at block weights, so these must be set before importation. Note profile incorporation relies only on the IL level view of the method, and does not depend on the IR.
ghost
added
the
area-CodeGen-coreclr
|
cc @dotnet/jit-contrib One more "last" refactor in anticipation of sparse profiles (#46882). Sparse profile data can't be added incrementally as we build the flow graph, as it depends on the final flowgraph shape. So I've extracted the whole thing into a phase. This will also simplify other add-on work (early normalization, successor likelihoods, and maybe someday profile synthesis). No diffs per SPMI. I verified that there are indeed IBC methods replayed in our collection. I can get a count of these, if desired. |
src/coreclr/jit/compiler.cpp
Outdated
| switch (fgPgoSchema[iSchema].InstrumentationKind) | ||
| { | ||
| fgNumProfileRuns += fgPgoSchema[iSchema].Other; | ||
| case ICorJitInfo::PgoInstrumentationKind::NumRuns: |
There was a problem hiding this comment.
compInitOptions seems like the wrong place for all this profile code. Can't is be done in the caller (compCompileHelper)? You just want it done before fgFindBasicBlocks? Maybe it would be useful to leave a comment here about the ordering requirements?
There was a problem hiding this comment.
Good point, I think it can all move into fgIncorporateProfileData.
There was a problem hiding this comment.
At least the analysis bit can move -- the VM request probably needs to stay there.
There was a problem hiding this comment.
Just to elaborate -- currently there is a thin dependence on checking availability of profile data early like this, basically we have dump output that describes what is happening.
But longer-term we might actually want to alter jit behavior if asked to do BBOPT but no profile data is available, so it seems reasonable (for now) to leave this bit of logic where it is.
src/coreclr/jit/fgprofile.cpp
Outdated
|
|
||
| block->setBBProfileWeight(profileWeight); | ||
|
|
||
| if (profileWeight == 0) |
There was a problem hiding this comment.
Should this use 0.0f, or BB_ZERO_WEIGHT?
|
@BruceForstall updated per your feedback. |
|
Sadly, turns out this is incomplete; we're not actually incorporating profile data early. Will need a follow-up PR. |
Change dotnet#47646 did not remove the code in `fgMakeBasicBlocks` that set profile data. That masked some flaws in `fgIncorporateProfileData`: * it should process all blocks; nothing has been imported yet * it must honor a constraint that some handler blocks can't be rare. This fixes the above, and removes the early profile setting.
Change #47646 did not remove the code in `fgMakeBasicBlocks` that set profile data. That masked some flaws in `fgIncorporateProfileData`: * it should process all blocks; nothing has been imported yet * it must honor a constraint that some handler blocks can't be rare. This fixes the above, and removes the early profile setting.
ghost
locked as resolved and limited conversation to collaborators
2 participants
Instead of trying to set block weight data when initially building the
flowgraph, do it as a phase just before importation.
The importer looks at block weights, so these must be set before importation.
Note profile incorporation relies only on the IL level view of the method,
and does not depend on the IR.