-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Use compiler-lowered DAM when trimming #117624
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
Conversation
Adds support for checking TargetFrameworkAttribute. For .NET10 we don't want to use the heuristics to decode compiler-generated state machines. Adds test support for injecting TargetFrameworkAttribute. Adds test coverage for .NET 9 and .NET 10 behavior.
This behavior can be turned on by default when we pick up a version of Roslyn with a fix for dotnet/roslyn#79333
In a "copy" assembly, the DAM and CompilerLoweringPreserve types that are compiled in are kept, but the test infrastructure was filtering these out of the "linkedMembers", leading to validation errors.
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 adds the CompilerLoweringPreserve
attribute to DynamicallyAccessedMembersAttribute
, wires a new GenerateTargetFrameworkAttribute
flag through the test compilation pipeline, and updates both Linker and AOT Compiler to optionally use real TargetFrameworkAttribute
for .NET 10+ assemblies.
- Introduces
GenerateTargetFrameworkAttribute
in test runners, options, and metadata providers - Polyfills and applies
CompilerLoweringPreserve
on generated code and corelib attributes - Updates Linker and ILCompiler pipelines to respect a new
--disable-generated-code-heuristics
switch and detect the target framework version
Reviewed Changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/tools/illink/test/Trimming.Tests.Shared/TestRunner.cs | Include common source files and new generateTargetFrameworkAttribute argument in compiles |
src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs | Adjust generated-type attribute lookup to respect the new heuristics flag |
src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/DynamicallyAccessedMembersAttribute.cs | Add CompilerLoweringPreserveAttribute polyfill for DAM on compiler-lowered code |
src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/DynamicallyAccessedMembersAttribute.cs | Annotate corelib DAM attribute with CompilerLoweringPreserve |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerGeneratedState.cs | Pass the new disable-heuristics flag into AOT Compiler’s generated-code handling |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AssemblyExtensions.cs | Add GetTargetFrameworkVersion helper for AOT Compiler |
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompilationMetadataProvider.cs | Define GetGenerateTargetFrameworkAttribute to read the new attribute metadata |
(and similar updates in ILLink.RoslynAnalyzer.Tests, csproj files, driver entry points, etc.) |
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TestCaseCompilationMetadataProvider.cs:128
- The call to
GetOptionAttributeValue
usesnameof(GetGenerateTargetFrameworkAttribute)
but the metadata attribute is namedGenerateTargetFrameworkAttribute
. Change the parameter tonameof(GenerateTargetFrameworkAttribute)
so the option value is read correctly.
string runtimeDir = Path.GetDirectoryName(typeof(object).Assembly.Location)!;
Tagging subscribers to this area: @dotnet/illink |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs
Show resolved
Hide resolved
...tem.Private.CoreLib/src/System/Runtime/CompilerServices/CompilerLoweringPreserveAttribute.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs
Show resolved
Hide resolved
- Fix doc comment - Fix formatting
/ba-g "timeouts" |
This is blocking dotnet/dotnet#1546. Looks like this got lost in all the WASM timeout noise in dotnet#117624. We can either suppress or add to ref assembly, but ref assembly only has things ILLink custom steps depend on. So suppression it is. I just regenerated it with `/p:ApiCompatGenerateSuppressionFile=true` so it shuffled two unrelated lines.
This is blocking dotnet/dotnet#1546. Looks like this got lost in all the WASM timeout noise in #117624. We can either suppress or add to ref assembly, but ref assembly only has things ILLink custom steps depend on. So suppression it is. I just regenerated it with `/p:ApiCompatGenerateSuppressionFile=true` so it shuffled two unrelated lines.
This is blocking dotnet/dotnet#1546. Looks like this got lost in all the WASM timeout noise in #117624. We can either suppress or add to ref assembly, but ref assembly only has things ILLink custom steps depend on. So suppression it is. I just regenerated it with `/p:ApiCompatGenerateSuppressionFile=true` so it shuffled two unrelated lines.
This is blocking dotnet/dotnet#1546. Looks like this got lost in all the WASM timeout noise in #117624. We can either suppress or add to ref assembly, but ref assembly only has things ILLink custom steps depend on. So suppression it is. I just regenerated it with `/p:ApiCompatGenerateSuppressionFile=true` so it shuffled two unrelated lines. Co-authored-by: Michal Strehovský <[email protected]>
This uses compiler-lowered DAM for trimming compiler-generated code, and adds test infrastructure and coverage for DynamicallyAccessedMembers that is lowered by us for the compiler (when it has [CompilerLoweringPreserve], which is being added to the framework in #117729).
This adds CompilerLoweringPreserve, proposed in #103430, and applies it to DynamicallyAccessedMembers to ensure that DAM flows to compiler-generated types/members.(edit: moved to #117729)This makes it possible to use annotations on primary constructor parameters, fixing #101861.We also want to use the compiler-lowered attributes for generated state machines, when trimming a .NET10+ assembly (leaving the old heuristics for .NET9 and below, to avoid breaking trim analysis for existing assemblies). However, the tests uncovered an issue with the Roslyn implementation: dotnet/roslyn#79333. Until that is fixed, we don't rely on the lowered attribute for mapping type parameters by default (the new behavior is opt-in under a new flag). Once we get a fix we can remove this.
The old heuristics are still run for all TFMs simply for coverage (based on PR feedback), but we don't use the result.
This adds test infrastructure (building on #117449) to let us test against a polyfilled DAM with CompilerLoweringPreserve (since we don't run ILLink/ILC tests against a live corelib).
The existing ILLink tests (including those shared with ILC trimming tests) are now built with TargetFrameworkAttribute marking the assembly as targeting .NET 10. The compiler-generated code tests have been duplicated for .NET 9 and .NET 10 to test both the old and new behaviors.