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

Drop workarounds from BitConverter #64046

Closed
wants to merge 1 commit into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Jan 20, 2022

It seems like #11413 is already fixed so we don't need these workarounds. It's hard to trace down what exactly fixed that, a lot of struct/RA/cast-morphing work have been done since then.

double Foo(ulong x) => BitConverter.UInt64BitsToDouble(x);

codegen (before and after this change):

; Method Pp:Foo(long):double:this
G_M16062_IG01:
       vzeroupper 
G_M16062_IG02:
       vmovd    xmm0, rdx
G_M16062_IG03:
       ret      
; Total bytes of code: 9

Also, remove [MethodImpl(MethodImplOptions.AggressiveInlining)] All of these methods are less than 10 bytes of IL and always pass "below always_inline" for both RyuJIT and Mono.

@ghost
Copy link

ghost commented Jan 20, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

It seems like #11413 is already fixed so we don't need these workarounds. It's hard to trace down what exactly fixed that, a lot of struct/RA work have been done since then.

Also, remove [MethodImpl(MethodImplOptions.AggressiveInlining)] all of these method are less than 10 bytes of IL and always pass "below always_inline" for both RyuJIT and Mono.

Author: EgorBo
Assignees: -
Labels:

area-System.Numerics

Milestone: -

@EgorBo
Copy link
Member Author

EgorBo commented Jan 20, 2022

😄 looks like I've already tried to drop these workarounds and there is still a CQ issue when these casts are used in e.g. conditions #42348 (comment)

@EgorBo EgorBo closed this Jan 20, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant