KEMBAR78
Faster Math.Max/Min on arm64 by EgorBo · Pull Request #65584 · dotnet/runtime · GitHub
Skip to content

Faster Math.Max/Min on arm64 #65584

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Feb 19, 2022
Merged

Faster Math.Max/Min on arm64 #65584

merged 18 commits into from
Feb 19, 2022

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 18, 2022 •
edited
Loading

Math.Max and Math.Min do a lot of stuff for float and double to match IEEE754:2018 maximum/minimum. But we can at least rely on ARM's fmin/fmax, e.g.:

double Foo(double a, double b) => Math.Max(a, b);

Current codegen on arm64 (I am using R2R):

; Method Foo(double,double):double:this
G_M2729_IG01:
            stp     fp, lr, [sp,#-32]!
            mov     fp, sp
G_M2729_IG02:
            fcmp    d0, d1
            beq     G_M2729_IG06
G_M2729_IG03:
            fcmp    d0, d0
            bne     G_M2729_IG05
            fcmp    d1, d0
            blo     G_M2729_IG04
            b       G_M2729_IG08
G_M2729_IG04:
            fmov    d1, d0
            b       G_M2729_IG08
G_M2729_IG05:
            fmov    d1, d0
            b       G_M2729_IG08
G_M2729_IG06:
            str     d1, [fp,#24]
            ldr     x0, [fp,#24]
            cmp     x0, #0
            blt     G_M2729_IG07
            b       G_M2729_IG08
G_M2729_IG07:
            fmov    d1, d0
G_M2729_IG08:
            mov     v0.8b, v1.8b
G_M2729_IG09:
            ldp     fp, lr, [sp],#32
            ret     lr
; Total bytes of code: 88

New codegen:

; Method Foo(double,double):double:this
G_M2729_IG01:
            stp     fp, lr, [sp,#-16]!
            mov     fp, sp
G_M2729_IG02:
            fmax    d0, d0, d1
G_M2729_IG03:
            ldp     fp, lr, [sp],#16
            ret     lr
; Total bytes of code: 20

@ghost ghost assigned EgorBo Feb 18, 2022
@ghost ghost added the area-System.Numerics label Feb 18, 2022
@ghost
Copy link

ghost commented Feb 18, 2022

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

Issue Details

An issue from twitter

Math.Max/Min do a lot of stuff for floating point, e.g.:
image

Let's see if this passes our tests around edge cases values (e.g. min(+0.0, -0.0))
e.g. https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Extensions/tests/System/Math.cs#L1228

Author: EgorBo
Assignees: EgorBo
Labels:

area-System.Numerics
Milestone: -

tannergooding
tannergooding reviewed Feb 18, 2022
View reviewed changes
am11
am11 reviewed Feb 18, 2022
View reviewed changes
EgorBo and others added 2 commits February 18, 2022 22:39
@EgorBo @am11
Update src/libraries/System.Private.CoreLib/src/System/Math.cs
b477933 
Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
@EgorBo
Add generic IsKnownConstant
28c2bea
EgorBo
EgorBo commented Feb 18, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 18, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
EgorBo and others added 4 commits February 19, 2022 19:44
@EgorBo @tannergooding
Update src/coreclr/jit/codegenarmarch.cpp
8ec2cd6 
Co-authored-by: Tanner Gooding <tagoo@outlook.com>
@EgorBo @tannergooding
Update src/coreclr/jit/importer.cpp
0f6b767 
Co-authored-by: Tanner Gooding <tagoo@outlook.com>
@EgorBo
Fix OperSupportsReverseOps assert
535bac6
@EgorBo
Address feedback
c4177d1
@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2022

@tannergooding Thanks, addressed your feedback

EgorBo
EgorBo commented Feb 19, 2022
View reviewed changes
tannergooding
tannergooding reviewed Feb 19, 2022
View reviewed changes
SingleAccretion
SingleAccretion reviewed Feb 19, 2022
View reviewed changes
@EgorBo EgorBo mentioned this pull request Feb 19, 2022
Closed
@EgorBo EgorBo merged commit 7345109 into dotnet:main Feb 19, 2022
@SkiFoD SkiFoD mentioned this pull request Feb 22, 2022
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tannergooding tannergooding tannergooding approved these changes

+2 more reviewers

@am11 am11 am11 left review comments

@SingleAccretion SingleAccretion SingleAccretion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@EgorBo EgorBo

Labels

area-System.Numerics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@EgorBo @am11 @tannergooding @SingleAccretion