Optimize out metadata for typeof(X).IsValueType and .IsEnum
#118528
Conversation
We don't need reflection metadata to answer this. Change salvaged out of dotnet#118479, rt-sz will decide if we want it.
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the AOT compilation process by avoiding the generation of unnecessary reflection metadata for simple type queries. The optimization specifically targets patterns like typeof(X).IsValueType and typeof(X).IsEnum where the answer can be determined at compile time without requiring full reflection metadata.
Key changes:
- Adds pattern detection for
typeof(X).IsValueTypeandtypeof(X).IsEnumcall sequences - Modifies helper ID assignment to use
NecessaryTypeHandleinstead of full metadata when these patterns are detected
src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
The savings are borderline, but maybe worth it given the low complexity: Size statisticsPull request #118528
|
…r.cs Co-authored-by: Copilot <[email protected]>
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| { | ||
| if (reader.HasNext | ||
| && reader.ReadILOpcode() is ILOpcode.callvirt or ILOpcode.call | ||
| && _methodIL.GetObject(reader.ReadILToken()) is MethodDesc { Name: "get_IsValueType" or "get_IsEnum"}) |
There was a problem hiding this comment.
Add IsPrimitive too? It is used in some places in BCL.
There was a problem hiding this comment.
Let's see what rt-sz says. We can do this for several other properties but I wasn't sure how much of a coupling between the compiler and the BCL we are okay with in this area.
There was a problem hiding this comment.
If we wanted to avoid coupling between the compiler and the BCL, we would make this pattern must-expand in the JIT. It would reduce the coupling to JIT/scanner that exists in many places.
There was a problem hiding this comment.
Do we have a test coverage for the (rare) situation when the JIT does not expand this pattern as an intrinsic?
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
We don't need reflection metadata to answer this.
Change salvaged out of #118479, rt-sz will decide if we want it.