Uh oh!
There was an error while loading. Please reload this page.
Allow NecessaryTypeSymbol in generic dictionaries - #117315
Merged
Merged
Conversation
This was referenced Jul 4, 2025
Open
Contributor
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
MichalStrehovsky
commented
Jul 8, 2025
MemberAuthor
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Jul 8, 2025
AOT-incompatible code that worked due to luck. ``` System.NotSupportedException : 'System.Nullable`1[System.DayOfWeek][]' is missing native code or metadata. This can happen for code that is not compatible with trimming or AOT. Inspect and fix trimming and AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility at Internal.Runtime.Augments.RuntimeAugments.EnsureMethodTableSafeToAllocate(MethodTable*) + 0x58 at Internal.Runtime.Augments.RuntimeAugments.NewArray(RuntimeTypeHandle, Int32) + 0x1c at System.Array.InternalCreate(RuntimeType elementType, Int32 rank, Int32* pLengths, Int32* pLowerBounds) + 0x1f8 at System.Array.CreateInstance(Type elementType, Int32 length) + 0x5c at System.Linq.Expressions.Interpreter.NewArrayInitInstruction.Run(InterpretedFrame) + 0x24 at System.Linq.Expressions.Interpreter.Interpreter.Run(InterpretedFrame) + 0x48 at System.Linq.Expressions.Interpreter.LightLambda.Run(Object[] arguments) + 0x90 ```
MichalStrehovsky
commented
Jul 9, 2025
MemberAuthor
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
MichalStrehovsky
marked this pull request as ready for review
July 9, 2025 05:36
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using NecessaryTypeSymbol in generic dictionaries to optimize non-allocating type loads and extends this across the JIT interface, IL importer, dictionary layout, codegen backends, and the generic lookup infrastructure. It also introduces a smoke test to validate trimming behavior for necessary type handles in generic dictionaries.
- Introduced
TestTypeHandlesInGenericDictionariessmoke test in native AOT trimming behaviors. - Updated
CorInfoImpl.RyuJitandILImporter.Scannerto requestNecessaryTypeHandlewhere appropriate. - Extended generic lookup nodes, dictionary layout, and target-specific helper emitters to handle
NecessaryTypeHandle.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs | Added TestTypeHandlesInGenericDictionaries test |
| src/libraries/System.Linq.Expressions/tests/default.rd.xml | Exposed trimming for DayOfWeek[] and its nullable |
| src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | Handle Method token and runtime‐determined types |
| src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs | Swap to NecessaryTypeHandle for unbox, ldtoken, ldelema |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs | Deduplicate necessary vs. constructed type lookups |
| Multiple target‐specific helper nodes | Added case for NecessaryTypeHandle in emitters |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs | Support NecessaryTypeHandle in lookup signatures |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs | New cache and API for necessary type lookups |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs | Introduced NecessaryTypeHandleGenericLookupResult |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DictionaryLayoutNode.cs | Allow fallback from necessary to constructed slot |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs | Removed normalization of necessary to type handles |
Comments suppressed due to low confidence (1)
src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs:1136
- C# requires nested class declarations to include a body. Replace
class InvisibleType1;withclass InvisibleType1 {}(and similarly for InvisibleType2 and VisibleType1) to ensure valid syntax and successful compilation.
class InvisibleType1;
This was referenced Jul 9, 2025
MichalStrehovsky
commented
Jul 10, 2025
MemberAuthor
@jkoritzinsky would you be comfortable reviewing this? (We can also do this over a Teams call.) |
jkoritzinsky
commented
Jul 10, 2025
Member
I'll take a look today. If I have questions I'll let you know. |
jkoritzinsky
approved these changes
Jul 10, 2025
Uh oh!
There was an error while loading. Please reload this page.
MichalStrehovsky
commented
Jul 11, 2025
MemberAuthor
Mmm, segfault in the macos leg. I'll re-trigger that leg just in case. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this change, whenever we needed to refer to a type from a generic dictionary, we used the fully loaded "allocated" form of it. But we can do the same optimization we do for non-generic code where we distinguish between MethodTables that we need for casting-like operations and MethodTables that are actually for allocation.
Cc @dotnet/ilc-contrib