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

Make Math/MathF.Sign inlineable for floating point data #57413

Closed
Tracked by #47244
MichalPetryka opened this issue Aug 14, 2021 · 4 comments
Closed
Tracked by #47244

Make Math/MathF.Sign inlineable for floating point data #57413

MichalPetryka opened this issue Aug 14, 2021 · 4 comments
Labels

Comments

@MichalPetryka
Copy link
Contributor

Description

Math/MathF.Sign for floating point data currently isn't inlined, probably due to the throw there. Since it's a simple math operation, it should probably be inlined.

Configuration

Sharplab Core CLR 5.0.721.25508 on amd64

Regression?

No idea.

Data

Sharplab

Analysis

Moving the throw there to a throw helper should probably make it inlineable.

@MichalPetryka MichalPetryka added the tenet-performance Performance related issue label Aug 14, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 14, 2021
@tannergooding tannergooding added area-System.Numerics and removed untriaged New issue has not been triaged by the area owner labels Aug 14, 2021
@ghost
Copy link

ghost commented Aug 14, 2021

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

Issue Details

Description

Math/MathF.Sign for floating point data currently isn't inlined, probably due to the throw there. Since it's a simple math operation, it should probably be inlined.

Configuration

Sharplab Core CLR 5.0.721.25508 on amd64

Regression?

No idea.

Data

Sharplab

Analysis

Moving the throw there to a throw helper should probably make it inlineable.

Author: MichalPetryka
Assignees: -
Labels:

area-System.Numerics, tenet-performance

Milestone: -

@tannergooding
Copy link
Member

It's probably not worthwhile to make this inlineable. If perf is a consideration (and even if it isn't) then double/float.IsNegative or Math/MathF.CopySign is probably better.

The only consideration really is that these correctly report or propagate -0 and -NaN (that is, IsNegative reports true for negative zero and negative NaN; and CopySign will do the same, copying the sign to or from such inputs).

@dakersnar dakersnar modified the milestones: 7.0.0, 8.0.0 Aug 8, 2022
@dakersnar
Copy link
Contributor

Closing this for reasons explained above.

@dakersnar dakersnar closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2022
@dakersnar dakersnar removed this from the 8.0.0 milestone Nov 29, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants