Skip to content
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

F# typeof comparison suboptimal codegen #82394

Open
EgorBo opened this issue Feb 20, 2023 · 13 comments
Open

F# typeof comparison suboptimal codegen #82394

EgorBo opened this issue Feb 20, 2023 · 13 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Feb 20, 2023

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
Copy link
Member Author

EgorBo commented Feb 20, 2023

@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

@EgorBo EgorBo added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors labels Feb 20, 2023
@EgorBo EgorBo added this to the Future milestone Feb 20, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 20, 2023
@ghost
Copy link

ghost commented Feb 20, 2023

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:

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
Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@SingleAccretion SingleAccretion removed the untriaged New issue has not been triaged by the area owner label Feb 20, 2023
@En3Tho
Copy link
Contributor

En3Tho commented Feb 20, 2023

dotnet/fsharp#526
Same with >, >=, <, <= etc operators

@NinoFloris
Copy link
Contributor

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

Some relevant discussions (it's a known problem):
dotnet/fsharp#8447 (comment)
dotnet/fsharp#5019 (comment)
dotnet/fsharp#6228

@vzarytovskii
Copy link
Member

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

@NinoFloris
Copy link
Contributor

@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
Copy link
Member

@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
Copy link

Type testing at runtime is not needed in F#.

@huoyaoyuan
Copy link
Member

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

@huoyaoyuan
Copy link
Member

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
Copy link
Contributor

Thorium commented Mar 30, 2024

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

@T-Gro
Copy link
Member

T-Gro commented Nov 7, 2024

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

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

If you open the built-in NonStructuralComparison module, it will locally override the meaning of equals and produce the desired codegen:

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

https://sharplab.io/#v2:DYLgZgzgNALiCGEC2AfA9gBwKYDsAEAygJ4QxZICwAUJrngHJo4EwBOArgMYzuvzABhNEgzxWASwhNq1YFhh4ypABQBKPAF5FRbGjAAeMMDTwYAPk3bdBgEZo0wM0A==

@En3Tho
Copy link
Contributor

En3Tho commented Nov 7, 2024

I'd introduce an inline function for this, something along the lines of

[<AbstractClass; Sealed; AutoOpen>]
type TypeTestsImpl() =
  static member inline typetest<'T>(type': Type) =
      Type.(=) (type', typeof<'T>)
  
  static member inline  typetest<'T, 'U>() =
      Type.(=) (typeof<'T>, typeof<'U>)

I guess I will add this to my own collection of extensions I never use :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

9 participants