Allow NecessaryTypeSymbol in generic dictionaries #117315
Merged
Conversation
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
github-actions
bot
added
the
needs-area-label
This was referenced Jul 4, 2025
Open
teo-tsirpanis
added
area-NativeAOT-coreclr
and removed
needs-area-label
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
/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 ```
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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
|
@jkoritzinsky would you be comfortable reviewing this? (We can also do this over a Teams call.) |
|
I'll take a look today. If I have questions I'll let you know. |
jkoritzinsky
approved these changes
Jul 10, 2025
|
Mmm, segfault in the macos leg. I'll re-trigger that leg just in case. |
Sign up for free
to 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