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

[SR-6960] Floating-point nextUp and nextDown are unnecessarily slow for concrete types #49508

Closed
xwu opened this issue Feb 9, 2018 · 8 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. good first issue Good for newcomers performance standard library Area: Standard library umbrella

Comments

@xwu
Copy link
Collaborator

xwu commented Feb 9, 2018

Previous ID SR-6960
Radar None
Original Reporter @xwu
Type Bug
Status Closed
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug, Performance, StarterBug
Assignee @xwu
Priority Medium

md5: cc10aebb523597a05ab15d027507566e

Issue Description:

As discussed on the Swift forums, implementations of nextUp and nextDown are unnecessarily slow for concrete types.

For example, a specialized version of Float.nextUp that would be significantly faster is:

extension Float {
  var nextUp: Float {
    //  Map -0 to +0, silence signaling NaNs.
    let x = self + 0.0
    if x < .infinity {
      let increment = Int32(bitPattern: x.bitPattern) >> 31 | 1
      return Float(bitPattern: x.bitPattern &+ UInt32(bitPattern: increment))
    }
    return x
  }
}

We should have efficient, specialized implementations such as this. Similar issues may apply to binade and ulp.

@xwu
Copy link
Collaborator Author

xwu commented Feb 9, 2018

cc @stephentyrone

@jepers
Copy link

jepers commented Feb 9, 2018

So is it OK for a future implementation to differ from the current implementation by silencing signaling NaNs?

I assume it is, but perhaps it's still worth to clarify?

Ie, let's rename the example implementation in the description to nextUp2, then:

let x = Float.signalingNaN
let currentImpl = x.nextUp
let exampleImpl = x.nextUp2
print(currentImpl.bitPattern == exampleImpl.bitPattern) // false
print(currentImpl.bitPattern == x.bitPattern) // true
print(exampleImpl.bitPattern == x.bitPattern) // false
print(currentImpl.isSignalingNaN) // true
print(exampleImpl.isSignalingNaN) // false
print(String(currentImpl.bitPattern, radix: 2))
print(String(exampleImpl.bitPattern, radix: 2))
// 1111111101000000000000000000000
// 1111111111000000000000000000000

This goes against (a perhaps too strict interpretation of) the documentation of .nextUp since x.nextUp2 is not x itself:
"For any finite value x, x.nextUp is greater than x. For nan or infinity, x.nextUp is x itself."

@xwu
Copy link
Collaborator Author

xwu commented Feb 9, 2018

No, not too strict an interpretation. The IEEE standard specifically will point out where it is OK and not OK to silence NaN and we will need to check.

@stephentyrone
Copy link
Member

nextUp should silence signaling nans (and canonicalize results in formats that have non-canonical values), so this also fixes a bug.

More broadly, any operation that isn’t explicitly defined to preserve sNaN in 754 is expected to signal and return qNaN.

@stephentyrone
Copy link
Member

The documentation should really be read as “is x itself (canonicalized and silenced as appropriate).” This is a detail that I would say is out of scope for the documentation of an individual function, though perhaps we can come up with some standard terminology.

@xwu
Copy link
Collaborator Author

xwu commented Mar 7, 2018

Faster versions of nextUp, ulp, and binade are included for Float and Double in PR #15021.

@stephentyrone
Copy link
Member

Thanks Xiaodi!

@milseman
Copy link
Mannequin

milseman mannequin commented Mar 7, 2018

Convenience link: #15021

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. good first issue Good for newcomers performance standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

3 participants