Skip to content

Optimize typeof(T).IsValueType - #1157

Merged
jkotas merged 27 commits into
dotnet:masterfrom
EgorBo:intrinsify-type-isvaluetype
Dec 27, 2019
Merged

Optimize typeof(T).IsValueType#1157
jkotas merged 27 commits into
dotnet:masterfrom
EgorBo:intrinsify-type-isvaluetype

Conversation

@EgorBo

@EgorBoEgorBo commented Dec 26, 2019

Copy link
Copy Markdown
Member

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.:

classCase1{publicboolFoo<T>()=>typeof(T).IsValueType;publicboolTest1()=>Foo<int>();publicboolTest2()=>Foo<string>();}

Current codegen:

; Method Case1:Test1():bool:thisG_M15309_IG01:subrsp,40G_M15309_IG02:movrcx,0xD1FFAB1Ecall CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEmovrcx,raxmovrax, qword ptr [(reloc)]cmp dword ptr [rcx],ecxG_M15309_IG03:addrsp,40 rex.jmprax; Total bytes of code: 38; Method Case1:Test2():bool:thisG_M7114_IG01:subrsp,40G_M7114_IG02:movrcx,0xD1FFAB1Ecall CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEmovrcx,raxmovrax, qword ptr [(reloc)]cmp dword ptr [rcx],ecxG_M7114_IG03:addrsp,40 rex.jmprax; Total bytes of code: 38

New codegen:

; Method Case1:Test1():bool:thisG_M15308_IG02:moveax,1G_M15308_IG03:ret; Total bytes of code: 6; Method Case1:Test2():bool:thisG_M7113_IG02:xoreax,eaxG_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)

Comment threadsrc/coreclr/src/jit/importer.cpp Outdated
Comment threadsrc/coreclr/tests/src/JIT/Intrinsics/TypeIsIntrinsics_r.csproj Outdated
Comment threadsrc/coreclr/src/jit/importer.cpp Outdated
@jkotasjkotas added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 26, 2019
@EgorBo

EgorBo commented Dec 26, 2019

Copy link
Copy Markdown
MemberAuthor

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>

Comment threadsrc/coreclr/src/jit/importer.cpp Outdated
Comment threadsrc/coreclr/src/jit/importer.cpp Outdated
Comment threadsrc/coreclr/tests/src/JIT/Intrinsics/TypeIntrinsics.cs Outdated
Comment threadsrc/coreclr/tests/src/JIT/Intrinsics/TypeIntrinsics.cs Outdated
@jkotas

Copy link
Copy Markdown
Member

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

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

Comment threadsrc/coreclr/src/inc/corinfo.h Outdated
Comment threadsrc/coreclr/src/jit/importer.cpp Outdated
Comment threadsrc/libraries/System.Private.CoreLib/src/System/Type.cs Outdated
@EgorBo
EgorBoforce-pushed the intrinsify-type-isvaluetype branch from e3da25f to a6f6f13CompareDecember 27, 2019 13:41
@EgorBoEgorBo changed the title Optimize typeof(T).IsValueType, IsClass, IsPrimitiveTypeOptimize typeof(T).IsValueTypeDec 27, 2019

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

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.

Comment threadsrc/coreclr/src/jit/importer.cpp Outdated

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

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:masterDec 27, 2019
@EgorBo
EgorBo deleted the intrinsify-type-isvaluetype branch May 25, 2020 11:57
Sergio0694 added a commit to SixLabors/ImageSharp that referenced this pull request Sep 1, 2020
Can now be JITted to a constant on .NET 5, see dotnet/runtime#1157
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@EgorBo@jkotas@AndyAyersMS@MichalStrehovsky