dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.06k stars 4.69k forks source link

Make Type.IsGenericType and GetGenericTypeDefinition() JIT intrinsics (and JIT time constants) #96898

Closed Sergio0694 closed 2 months ago

Sergio0694 commented 8 months ago

Overview

This is one more codegen improvement we could leverage for CsWinRT, related to #95929. One scenario we have (here) is to check whether a given type T is some KeyValuePair<,> instantiation. This needs to be marshalled in a specific way to match the expectations of WinRT. Right now however, the only way we have to check for this case is to use IsGenericType and GetGenericTypeDefinition(), which are not JIT intrinsics. This makes the linker not see this special case, and therefore not correctly trim the code away (or just specialize it on the other hand). This is both marginally slower, but most importantly leaves some codegen size reduction opportunities on the table. We're seeing a bunch of KeyValuePair<__Canon, __Canon> instantiations in sizoscope even in sample WinRT components that never use KeyValuePair<,> at all anywhere. It would be nice if this could just be properly inlined as a constant instead.

Codegen

using SharpLab.Runtime;

[JitGeneric(typeof(System.Collections.Generic.KeyValuePair<int, int>))]
[JitGeneric(typeof(int))]
static bool IsKVP<T>()
{
    return typeof(T).IsGenericType && typeof(T).GetGenericTypeDefinition() == typeof(System.Collections.Generic.KeyValuePair<,>);
}

Current codegen on .NET 8 x64 (sharplab):

Program.<<Main>$>g__IsKVP|0_0[[System.Collections.Generic.KeyValuePair`2[[System.Int32, System.Private.CoreLib],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]]()
    L0000: sub rsp, 0x28
    L0004: mov rcx, 0x23e784e3260
    L000e: call qword ptr [0x7ffb1060a518]
    L0014: test eax, eax
    L0016: je short L0040
    L0018: mov rcx, 0x23e784e3260
    L0022: call qword ptr [0x7ffb1060a568]
    L0028: mov rcx, 0x23e783e2728
    L0032: cmp rax, rcx
    L0035: sete al
    L0038: movzx eax, al
    L003b: add rsp, 0x28
    L003f: ret
    L0040: xor eax, eax
    L0042: add rsp, 0x28
    L0046: ret

Program.<<Main>$>g__IsKVP|0_0[[System.Int32, System.Private.CoreLib]]()
    ; pretty much the same as above

Expected codegen:

Program.<<Main>$>g__IsKVP|0_0[[System.Collections.Generic.KeyValuePair`2[[System.Int32, System.Private.CoreLib],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]]()
    L0000: mov eax, 1
    L0005: ret

Program.<<Main>$>g__IsKVP|0_0[[System.Int32, System.Private.CoreLib]]()
    L0000: xor eax, eax
    L0002: ret

cc. @jkoritzinsky @MichalStrehovsky @EgorBo

ghost commented 8 months ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

Issue Details
### Overview This is one more codegen improvement we could leverage for CsWinRT, related to #95929. One scenario we have ([here](https://github.com/microsoft/CsWinRT/blob/eb066e4a179767e5598fd901652a625941f0b9db/src/WinRT.Runtime/Marshalers.cs#L1483C113-L1483C121)) is to check whether a given type `T` is some `KeyValuePair<,>` instantiation. This needs to be marshalled in a specific way to match the expectations of WinRT. Right now however, the only way we have to check for this case is to use `IsGenericType` and `GetGenericTypeDefinition()`, which are not JIT intrinsics. This makes the linker not see this special case, and therefore not correctly trim the code away (or just specialize it on the other hand). This is both marginally slower, but most importantly leaves some codegen size reduction opportunities on the table. We're seeing a bunch of `KeyValuePair<__Canon, __Canon>` instantiations in sizoscope even in sample WinRT components that never use `KeyValuePair<,>` at all anywhere. It would be nice if this could just be properly inlined as a constant instead. ### Codegen ```csharp using SharpLab.Runtime; [JitGeneric(typeof(System.Collections.Generic.KeyValuePair))] [JitGeneric(typeof(int))] static bool IsKVP() { return typeof(T).IsGenericType && typeof(T).GetGenericTypeDefinition() == typeof(System.Collections.Generic.KeyValuePair<,>); } ``` **Current codegen on .NET 8 x64 ([sharplab](https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABABgAIBlAC2ygAcAZbYAOgCUBXAOwwEt8MANwBYAFDiA2gCk+GAOIxuMKHzAAKDAE96MCADN1xAIwpWAaRhaAatgA2nGAAVsfKAB4+vNOS8YAfACUgQC64iZI5MAQEHbkAJK45tZO7gAq/uqB4gDe4uQF5MQA7OTaugbqaYGsiYrKqmBpOjDkAGRtZS2V1ayKCkoqas26ACIw+l5yfBDcWeQAvAtdFYYmZpY29o4ubu5oQaJiAL5AA===))**: ```asm Program.<
$>g__IsKVP|0_0[[System.Collections.Generic.KeyValuePair`2[[System.Int32, System.Private.CoreLib],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]]() L0000: sub rsp, 0x28 L0004: mov rcx, 0x23e784e3260 L000e: call qword ptr [0x7ffb1060a518] L0014: test eax, eax L0016: je short L0040 L0018: mov rcx, 0x23e784e3260 L0022: call qword ptr [0x7ffb1060a568] L0028: mov rcx, 0x23e783e2728 L0032: cmp rax, rcx L0035: sete al L0038: movzx eax, al L003b: add rsp, 0x28 L003f: ret L0040: xor eax, eax L0042: add rsp, 0x28 L0046: ret Program.<
$>g__IsKVP|0_0[[System.Int32, System.Private.CoreLib]]() ; pretty much the same as above ``` **Expected codegen**: ```asm Program.<
$>g__IsKVP|0_0[[System.Collections.Generic.KeyValuePair`2[[System.Int32, System.Private.CoreLib],[System.Int32, System.Private.CoreLib]], System.Private.CoreLib]]() L0000: mov eax, 1 L0005: ret Program.<
$>g__IsKVP|0_0[[System.Int32, System.Private.CoreLib]]() L0000: xor eax, eax L0002: ret ``` cc. @jkoritzinsky @MichalStrehovsky @EgorBo
Author: Sergio0694
Assignees: -
Labels: `area-CodeGen-coreclr`
Milestone: -
Sergio0694 commented 8 months ago

Sharing some notes from Discord:

EgorBo commented 8 months ago

grep.app:

Sergio0694 commented 8 months ago

I agree there's not many hits, but this would be beneficial for CsWinRT. We're really trying to minimize the binary size in minimal applications and WinRT components, to make the adoption of C# easier to justify in place of C++ (and binary size is one of the main problems you immediately hit when doing so). We have lots of heavily generic code especially in our marshallers, and they all end up being instantiated over KeyValuePair<,> today because we can't tell the linker that branch is really not used. Having a linker-friendly way of doing that check would be nice to have, is all 🙂

GerardSmit commented 8 months ago

There are more hits with GitHub Search:

teo-tsirpanis commented 8 months ago

It would also help with F#'s typedefof.

hez2010 commented 3 months ago

Can we also intrinsify typeof(T).IsConstructedGenericType as well?

Sergio0694 commented 3 months ago

Thank you for volunteering! 😄