-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add mechanism to allow CoreLib trimming to leverage whole program analysis #118769
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
Add mechanism to allow CoreLib trimming to leverage whole program analysis #118769
Conversation
…lysis A couple times in the past I needed a way to say "if X is not part of the program, eliminate the entire basic block". We can do this for allocated types (this is how branches under `is` checks elimination works), but we can't do this for more general "characteristics". This introduces a mechanism where AOT compiler and CoreLib (or System.Private.* universe in general) can define whole program tags such as "whole program has X in it" and CoreLib can condition code on the presence of this tag. This is easier shown than described, so I extracted the first use of this into a separate commit. In this commit, we eliminate code that tries looking for `StackTraceHiddenAttribute` if we know the whole program has no `StackTraceHiddenAttribute` in it. With this code eliminated, dotnet#118640 can then eliminate all custom attributes on methods, which in turn plays into dotnet#118718 and we can eliminate enum boxing even when StackTraceSupport is not set to false (right now dotnet#118718 really needs the StackTraceSupport=false to get rid of boxed enums; we get more boxed enums from method attributes). We have a new node that represents the characteristic. The node can be dropped into the graph wherever needed. ILScanner then uses this to condition parts of the method body on this characteristic node. We need similar logic in the substitution IL provider because we need to guarantee that RyuJIT is not going to see basic blocks we didn't scan. So we treat it as a substitution during codegen phase too.
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new mechanism that allows CoreLib to leverage whole program analysis by enabling conditional code elimination based on program-wide characteristics. The primary goal is to eliminate code paths when certain features or attributes are not used anywhere in the program.
Key changes include:
- Implementation of analysis characteristics through a new
AnalysisCharacteristicAttribute
andAnalysisCharacteristicNode
system - Enhanced dead code elimination for
StackTraceHiddenAttribute
checking when no such attributes exist in the program - Integration of characteristic-based branching in the IL scanner and substitution provider
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/tests/nativeaot/SmokeTests/AttributeTrimming/AttributeTrimming.csproj |
Test project configuration for validating attribute trimming functionality |
src/tests/nativeaot/SmokeTests/AttributeTrimming/AttributeTrimming.cs |
Test code that verifies method attributes are optimized away while type attributes remain |
src/coreclr/tools/aot/ILCompiler/Program.cs |
Updates ILCompiler to pass analysis characteristics to the substituted IL provider |
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj |
Adds the new AnalysisCharacteristicNode to the project |
src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs |
Implements detection of characteristic-based branching patterns in IL scanning |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs |
Adds support for substituting characteristic method calls with constants |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs |
Provides access to analysis characteristics from scan results |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs |
Factory method for creating analysis characteristic nodes |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/MethodMetadataNode.cs |
Tracks StackTraceHidden characteristic when methods have the attribute |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AnalysisCharacteristicNode.cs |
New dependency node representing program-wide characteristics |
src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/System.Private.StackTraceMetadata.csproj |
Includes the new attribute and intrinsic files |
src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs |
Implements characteristic-based conditional logic for StackTraceHidden processing |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/AnalysisCharacteristicAttribute.cs |
New attribute for marking intrinsic methods as characteristic checks |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs
Show resolved
Hide resolved
...eaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs
Show resolved
Hide resolved
Could we use this experience for the IDynamicInterfaceCastable support as well instead of the custom body substitutions I added? |
StackTraceHiddenAttribute is used on popular types/methods, e.g.
|
Quite possibly yes.
Stack trace metadata comes in two forms - a compact form where IsHidden is just a bit flag, and reflection metadata where it's an attribute. We emit the compact form when the method is not target of reflection and use reflection metadata otherwise. So we'd need to be unlucky and have a StackTraceHidden method that is target of reflection. In practice it very often isn't: MichalStrehovsky/rt-sz#171 measured a size saving even in Avalonia. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one doc thing (can be a follow up) and one clarifying question
...eaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/StackTraceMetadata.cs
Show resolved
Hide resolved
...ystem.Private.CoreLib/src/System/Runtime/CompilerServices/AnalysisCharacteristicAttribute.cs
Show resolved
Hide resolved
Implements suggestion from dotnet#118769 (comment). We could probably replace `CanHaveDynamicInterfaceImplementations` with this too, but we'd need to make it so that characteristics are available in `Compilation` class. They are currently hidden in `SubstitutionILProvider`.
Implements suggestion from #118769 (comment). We could probably replace `CanHaveDynamicInterfaceImplementations` with this too, but we'd need to make it so that characteristics are available in `Compilation` class. They are currently hidden in `SubstitutionILProvider`.
A couple times in the past I needed a way to say "if X is not part of the program, eliminate the entire basic block". We can do this for allocated types (this is how branches under
is
checks elimination works), but we couldn't do this for more general "characteristics".This introduces a mechanism where AOT compiler and CoreLib (or System.Private.* universe in general) can define whole program tags such as "whole program has X in it" and CoreLib can condition code on the presence of this tag.
This is easier shown than described, so I extracted the first use of this into a separate commit (383a4e8). In this commit, we eliminate code that tries looking for
StackTraceHiddenAttribute
if we know the whole program has noStackTraceHiddenAttribute
in it. With this code eliminated, #118640 can then eliminate all custom attributes on methods, which in turn plays into #118718 and we can eliminate enum boxing even when StackTraceSupport is not set to false (right now #118718 really needs the StackTraceSupport=false to get rid of boxed enums; we get more boxed enums from method attributes).We have a new node that represents the characteristic. The node can be dropped into the graph wherever needed. ILScanner then uses this to condition parts of the method body on this characteristic node. We need similar logic in the substitution IL provider because we need to guarantee that RyuJIT is not going to see basic blocks we didn't scan. So we treat it as a substitution during codegen phase too.
Cc @dotnet/ilc-contrib