Skip to content

Optimize typeof(T).IsValueType #1157

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.

Sign up for GitHub

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

Jump to bottom
Merged
jkotas merged 27 commits into from
Dec 27, 2019
Merged

Optimize typeof(T).IsValueType #1157

jkotas merged 27 commits into from
Dec 27, 2019

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Dec 26, 2019 •
edited
Loading

Fixes https://github.com/dotnet/coreclr/issues/23208
Also, contributes to https://github.com/dotnet/coreclr/issues/2591
Also, dotnet/aspnetcore@a42fff6

Replaces patterns like typeof(T).IsValueType or var.GetType().IsValueType() with constants (true/false).
E.g.:

class Case1
{
    public bool Foo<T>() => typeof(T).IsValueType;


    public bool Test1() => Foo<int>();
    public bool Test2() => Foo<string>();
}

Current codegen:

; Method Case1:Test1():bool:this
G_M15309_IG01:
       sub      rsp, 40
G_M15309_IG02:
       mov      rcx, 0xD1FFAB1E
       call     CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
       mov      rcx, rax
       mov      rax, qword ptr [(reloc)]
       cmp      dword ptr [rcx], ecx
G_M15309_IG03:
       add      rsp, 40
       rex.jmp  rax
; Total bytes of code: 38


; Method Case1:Test2():bool:this
G_M7114_IG01:
       sub      rsp, 40
G_M7114_IG02:
       mov      rcx, 0xD1FFAB1E
       call     CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
       mov      rcx, rax
       mov      rax, qword ptr [(reloc)]
       cmp      dword ptr [rcx], ecx
G_M7114_IG03:
       add      rsp, 40
       rex.jmp  rax
; Total bytes of code: 38

New codegen:

; Method Case1:Test1():bool:this
G_M15308_IG02:
       mov      eax, 1
G_M15308_IG03:
       ret      
; Total bytes of code: 6


; Method Case1:Test2():bool:this
G_M7113_IG02:
       xor      eax, eax
G_M7113_IG03:
       ret      
; Total bytes of code: 3

This PR also implements Type.IsPrimitive and Type.IsClass. Probably can implement more for free, e.g. Type.IsArray and Type.IsEnum (this one is virtual)

jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
MichalStrehovsky
MichalStrehovsky reviewed Dec 26, 2019
View reviewed changes
@jkotas jkotas added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 26, 2019
@EgorBo
Copy link
Member Author

EgorBo commented Dec 26, 2019 •
edited
Loading

I find the following tests quite weird (matching current behaviour):

IsTrue (typeof(int*).IsClass)
IsTrue (typeof(Action<int>).IsClass);
IsFalse(typeof(ValueType).IsValueType);
IsTrue (typeof(ValueType).IsClass);

<removed>

jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 26, 2019
View reviewed changes
@jkotas
Copy link
Member

jkotas commented Dec 26, 2019

I find the following tests quite weird (matching current behaviour):

I believe that's because of the tests have a few copy&paste bugs.

jkotas
jkotas reviewed Dec 27, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 27, 2019
View reviewed changes
jkotas
jkotas reviewed Dec 27, 2019
View reviewed changes
@EgorBo EgorBo force-pushed the intrinsify-type-isvaluetype branch from e3da25f to a6f6f13 Compare December 27, 2019 13:41
@EgorBo EgorBo mentioned this pull request Dec 27, 2019
Merged
@EgorBo EgorBo changed the title Optimize typeof(T).IsValueType, IsClass, IsPrimitiveType Optimize typeof(T).IsValueType Dec 27, 2019
jkotas
jkotas approved these changes Dec 27, 2019
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

I would like somebody from the JIT team to sign-off on this as well before merging.

jkotas
jkotas reviewed Dec 27, 2019
View reviewed changes
AndyAyersMS
AndyAyersMS approved these changes Dec 27, 2019
View reviewed changes
Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too. Thanks.

@jkotas jkotas merged commit 55b1303 into dotnet:master Dec 27, 2019
@jkotas jkotas mentioned this pull request Dec 28, 2019
Merged
@BruceForstall BruceForstall mentioned this pull request Jan 3, 2020
Closed
This was referenced Feb 11, 2020
Closed
Merged
@khellang khellang mentioned this pull request Apr 13, 2020
Closed
@samsosa samsosa mentioned this pull request May 6, 2020
Closed
@EgorBo EgorBo deleted the intrinsify-type-isvaluetype branch May 25, 2020 11:57
@EgorBo EgorBo mentioned this pull request Aug 5, 2020
Closed
Sergio0694 added a commit to SixLabors/ImageSharp that referenced this pull request Sep 1, 2020
@Sergio0694
Improved Guard.MustBeValueType<T> codegen
e740fb6 
Can now be JITted to a constant on .NET 5, see dotnet/runtime#1157
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

@MichalStrehovsky MichalStrehovsky MichalStrehovsky left review comments

@AndyAyersMS AndyAyersMS AndyAyersMS approved these changes

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@EgorBo @jkotas @AndyAyersMS @MichalStrehovsky