Skip to content

Conversation

@AndyAyersMS
Copy link
Member

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.

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.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 1, 2021
@AndyAyersMS
Copy link
Member Author

cc @dotnet/jit-contrib

No diffs per SPMI (which includes PGO runs in crossgen). Verified locally dynamic PGO still happy.

The artificial profile counts on handler entries lead to some code bloat we might want to avoid. Per SPMI impact is small, so not a high priority. Note there were some regressions too, my guess is those are cases where it can pay off to inline small methods in cold blocks.

Total bytes of delta: -1695 (-1.61% of base)
116 total methods with Code Size differences (88 improved, 28 regressed), 2 unchanged.

Copy link
Contributor

@briansull briansull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine

@AndyAyersMS AndyAyersMS merged commit e7f4dcd into dotnet:master Feb 2, 2021
@AndyAyersMS AndyAyersMS deleted the RefactorProfileReadingPart2 branch February 2, 2021 01:14
@AndyAyersMS AndyAyersMS mentioned this pull request Feb 6, 2021
54 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2021
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