-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Make Type.IsEnum and Type.GetEnumUnderlyingType intrinsics #71685
Conversation
Makes Type.GetTypeCode a JIT intrinsic for primitive types, enums and strings. Makes Type.IsEnum use intrinsics instead of IsSubclassOf Makes Enum.GetUnderlyingType use Type.GetTypeCode Moves the legacy FCall to InternalGetUnderlyingTypeImpl, so that the non-intrinsic GetTypeCode can use it. Introduces JIT tests checking the generated codegen.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsMakes Type.GetTypeCode a JIT intrinsic for primitive types, Makes Type.IsEnum use intrinsics instead of IsSubclassOf Makes Enum.GetUnderlyingType use Type.GetTypeCode Moves the legacy FCall to InternalGetUnderlyingTypeImpl, Introduces JIT tests checking the generated codegen. Fixes #70483.
|
This PR generates the desired codegen only with #71778. |
// Return Value: | ||
// Is the tree typeof() | ||
// | ||
bool Compiler::gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle) |
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.
Seems like you should try and leverage gtGetTypeProducerKind
and/or gtIsTypeHandleToRuntimeTypeHelper
here.
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.
gtGetTypeProducerKind
doesn't seem particularly useful here since we don't know the exact type with .GetType() and we don't want to return null types here so I've used gtIsTypeHandleToRuntimeTypeHelper
and added tests for NRE with Type.GetTypeFromHandle(default)
(which I assume generates the (actually only __reftype seems to generate it)._MAYBENULL
helper variant)
Yes, I think it is okay to have that method in Type.cs. Nit: I would call the method |
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.
JIT/SPMI changes LGTM.
… parameters. Related: dotnet#71685.
#77763 will fix the mono failures. |
@MichalPetryka Could you please merge from main and resolve the conflicts? |
@jkotas |
Do you have #77715 ? |
Yes. For context:
|
For now I've worked around it by disabling TreatWarningsAsErrors in the generator. |
@jkotas Is there anything left to be done here on my part? |
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...ries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
Outdated
Show resolved
Hide resolved
...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs
Outdated
Show resolved
Hide resolved
…ion/Converters/Value/EnumConverter.cs
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 once the CI is green. Thank you!
Makes Type.IsEnum and Type.GetEnumUnderlyingType intrinsics.
Implements IsKnownConstant for Type.
Optimizes Type.GetTypeCode using IsKnownConstant.
Introduces a JIT helper for checking if a tree is typeof.
Introduces JIT tests checking the generated codegen.