dotnet / runtime

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

F# typeof comparison suboptimal codegen #82394

Open EgorBo opened 1 year ago

EgorBo commented 1 year ago

The following F# snippet:

let test() = typeof<float> = typeof<bool>

Emits this IL:

ldtoken !!a
call class [netstandard]System.Type [netstandard]System.Type::GetTypeFromHandle(valuetype [netstandard]System.RuntimeTypeHandle)
ldtoken [System.Runtime]System.Boolean
call class [netstandard]System.Type [netstandard]System.Type::GetTypeFromHandle(valuetype [netstandard]System.RuntimeTypeHandle)
tail.
call bool [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/HashCompare::GenericEqualityIntrinsic<class [System.Runtime]System.Type>(!!0, !!0)
ret

and this codegen:

_.test()
    L0000: push rsi
    L0001: sub rsp, 0x20
    L0005: mov rcx, 0x7ff9f93c77a8
    L000f: call 0x00007ffa58e77670
    L0014: mov rsi, rax
    L0017: mov rcx, 0x7ff9f934b3c0
    L0021: call 0x00007ffa58e77670
    L0026: mov r8, rax
    L0029: mov rdx, rsi
    L002c: mov rcx, 0x7ff9febc1e88
    L0036: mov rax, 0x7ff9feb43be8
    L0040: add rsp, 0x20
    L0044: pop rsi
    L0045: jmp qword ptr [rax]

expected codegen:

_.test()
    L0000: xor eax, eax
    L0002: ret
EgorBo commented 1 year ago

@vzarytovskii can F# here emit

call bool [System.Runtime]System.Type::op_Equality(class [System.Runtime]System.Type, class [System.Runtime]System.Type)

instead of

call bool [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/HashCompare::GenericEqualityIntrinsic<class [System.Runtime]System.Type>(!!0, !!0

? Or otherwise we need to do special handling for GenericEqualityIntrinsic in JIT

ghost commented 1 year ago

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

Issue Details
The following F# snippet: ```fsharp let test() = typeof = typeof ``` Emits this IL: ```cil ldtoken !!a call class [netstandard]System.Type [netstandard]System.Type::GetTypeFromHandle(valuetype [netstandard]System.RuntimeTypeHandle) ldtoken [System.Runtime]System.Boolean call class [netstandard]System.Type [netstandard]System.Type::GetTypeFromHandle(valuetype [netstandard]System.RuntimeTypeHandle) tail. call bool [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/HashCompare::GenericEqualityIntrinsic(!!0, !!0) ret ``` and this codegen: ```asm _.test() L0000: push rsi L0001: sub rsp, 0x20 L0005: mov rcx, 0x7ff9f93c77a8 L000f: call 0x00007ffa58e77670 L0014: mov rsi, rax L0017: mov rcx, 0x7ff9f934b3c0 L0021: call 0x00007ffa58e77670 L0026: mov r8, rax L0029: mov rdx, rsi L002c: mov rcx, 0x7ff9febc1e88 L0036: mov rax, 0x7ff9feb43be8 L0040: add rsp, 0x20 L0044: pop rsi L0045: jmp qword ptr [rax] ``` expected codegen: ```asm _.test() L0000: xor eax, eax L0002: ret ```
Author: EgorBo
Assignees: -
Labels: `area-CodeGen-coreclr`
Milestone: -
En3Tho commented 1 year ago

https://github.com/dotnet/fsharp/issues/526 Same with >, >=, <, <= etc operators

NinoFloris commented 1 year ago

By default F# = evaluates structural equality so it will always go through these helpers.

Some relevant discussions (it's a known problem): https://github.com/dotnet/fsharp/issues/8447#issuecomment-583668989 https://github.com/dotnet/fsharp/issues/5019#issuecomment-392964318 https://github.com/dotnet/fsharp/issues/6228

vzarytovskii commented 1 year ago

I guess we can generate op_Equality for System.Type (and other known types like DateTime, TimeSpan, UtcDateTime, Range, Guid, etc).

NinoFloris commented 1 year ago

@vzarytovskii afaik @TIHan mentioned at some point this is not sound due to System.Type being a derivable type. We could limit that optimization to immediate invocations and comparisons on typeof instances though. I.e. arguments like (ty: Type) would still generate structural equality. For DateTime and other such non-collection structs I agree it should be entirely safe to generate op_Equality.

vzarytovskii commented 1 year ago

@vzarytovskii afaik @TIHan mentioned at some point this is not sound due to System.Type being a derivable type. We could limit that optimization to immediate invocations and comparisons on typeof instances though. I.e. arguments like (ty: Type) would still generate structural equality.

Yeah, and I think we do it in Type Providers ourselves, and yeah, I meant that we can limit it to the System.Type only, and not derivatives.

charlesroddie commented 1 year ago

Type testing at runtime is not needed in F#.

huoyaoyuan commented 11 months ago

Is this still the case when (non-collectible) typeof becoming constant at runtime?

huoyaoyuan commented 11 months ago

Codegen for 8.0 RC 2:

; Program+Program.f()
;         typeof<int> = typeof<float>
;         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
       mov       rcx,offset MD_Microsoft.FSharp.Core.LanguagePrimitives+HashCompare.GenericEqualityIntrinsic[[System.Type, System.Private.CoreLib]](System.Type, System.Type)
       mov       rdx,19080205140
       mov       r8,19080205898
       jmp       qword ptr [7FFA6643F9F0]; Microsoft.FSharp.Core.LanguagePrimitives+HashCompare.GenericEqualityIntrinsic[[System.__Canon, System.Private.CoreLib]](System.__Canon, System.__Canon)
; Total bytes of code 36

GenericEqualityIntrinsic is still not inlined.

Thorium commented 5 months ago

Is there any workaround or better way to do this if you have to compare many types?

let test myType = 
   if myType = typeof<bool> then ...
   elif myType = typeof<float> then ...
   elif myType = typeof<ValueOption<decimal>> then ...
   elif ...