Skip to content
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

Simplify mibc usage in the build #50536

Merged

Conversation

davidwrighton
Copy link
Member

  • Produce a merged mibc with all scenarios squished together
  • Properly attach the mibc data to the incremental build for System.Private.CoreLib
    • This can't be done for the framework here. It will require mibc integration in the SDK
  • Enable pgo optimization in checked builds
  • Enable pgo optimization in framework compile for outerloop runs

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@davidwrighton
Copy link
Member Author

@dotnet/crossgen-contrib once #50364 is merged, I'm going to run a test pass to verify everything is working with this.

</ItemGroup>

<ItemGroup>
<PublishReadyToRunCrossgen2ExtraArgsList Include="--targetarch:$(TargetArchitecture)"/>

<!-- Only use mibc files if UsingToolIbcOptimization is false. Allows enabling/disabling using ibc instead of mibc data -->
<PublishReadyToRunCrossgen2ExtraArgsList Condition="'$(UsingToolIbcOptimization)' != 'true' and '$(Configuration)' == 'Release'" Include="@(OptimizationMibcFiles->'-m:%(Identity)')"/>
<PublishReadyToRunCrossgen2ExtraArgsList Condition="'$(UsingToolIbcOptimization)' != 'true' and '$(Configuration)' == 'Release'" Include="--embed-pgo-data"/>
<PublishReadyToRunCrossgen2ExtraArgsList Condition="'$(UsingToolIbcOptimization)' != 'true' and '$(EnableNgenOptimization)' == 'true'" Include="@(OptimizationMibcFiles->'-m:%(Identity)')"/>
Copy link
Member

Choose a reason for hiding this comment

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

Should EnableNgenOptimization be renamed?

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

@@ -45,6 +45,10 @@
<CrossGen2DllFiles Condition="'$(CrossDir)' != ''" Include="$(BinDir)/$(CrossDir)/crossgen2/*" />
</ItemGroup>

<ItemGroup>
<OptimizationMibcFiles Include="$(MibcOptimizationDataDir)/$(TargetOS)/$(TargetArchitecture)/**/*.mibc" />
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to produce some warnings when no MIBC data is available? Or, alternatively, should we print the files that were found or at least the number of files found? Especially for lab runs this might simplify various investigations.

Copy link
Member Author

Choose a reason for hiding this comment

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

The build log will contain the list of mibc files processed as part of the standard output from the dotnet-pgo tool. Also, it will fail if there isn't any input data.

@AndyAyersMS
Copy link
Member

This is flowing the same PGO data into Checked as it does into Release, right?

@davidwrighton
Copy link
Member Author

Yes. We only produce one set of data.

- Produce a merged mibc with all scenarios squished together
- Properly attach the mibc data to the incremental build for System.Private.CoreLib
  - This can't be done for the framework here. It will require mibc integration in the SDK
- Enable pgo optimization in checked builds
- Enable pgo optimization in framework compile for outerloop runs
@davidwrighton davidwrighton force-pushed the better_structured_pgo_processing branch from 33c01b0 to 5633627 Compare April 12, 2021 20:02
@davidwrighton davidwrighton merged commit fccdca0 into dotnet:main Apr 14, 2021
@davidwrighton davidwrighton deleted the better_structured_pgo_processing branch April 20, 2021 17:47
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants