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

Discussion: float no_std unsupported methods #50

Open
nerodesu017 opened this issue Jul 23, 2024 · 8 comments
Open

Discussion: float no_std unsupported methods #50

nerodesu017 opened this issue Jul 23, 2024 · 8 comments

Comments

@nerodesu017
Copy link
Collaborator

As the title says, there are a few methods for f32 (and I assume for f64, as well), that are only in the std library: abs, ceil, floor, trunc, round, sqrt and copysign.

The only alternative I could find, until now, was the libm library. Sadly, it has a major number of 0, which means breaking changes might appear on any new update.
We could lock it at a version or maybe just use from its source what's needed, since licenses are Apache 2 and MIT

@florianhartung
Copy link
Collaborator

Including libm as a dependency is not an option IIRC. Although not mentioned in the requirements, we want to minimize the use of external dependencies. The closest matching requirement would be:

[REQUIREMENT]
UID: REQ-3
TITLE: Baremetal
STATEMENT: >>>
The interpreter shall be executable on bare-metal environments
<<<
RATIONALE: >>>
No reliance on any specific functionality from the provided execution environment is acceptable, as the interpreter shall be ready for embedding to any environment that Rust can compile for.
<<<

@florianhartung
Copy link
Collaborator

@wucke13

@nerodesu017
Copy link
Collaborator Author

So what solution would you guys propose?

@wucke13
Copy link
Collaborator

wucke13 commented Jul 24, 2024

I' immediately aware of https://docs.rs/libm/latest/libm/ and https://crates.io/crates/num-traits . Num internally relies on libm, but it uses the same names like std::f64, thus it is more familiar. Both have a minimal dependency tree, i.e. libm depends on nothing, and num-traits depends only on libm. Other than that, we could implement these by hand. here would be the most basic example for abs:

pub fn abs64(x: f64) -> f64 {
    f64::from_bits(x.to_bits() & (i64::MAX as u64))
}
pub fn abs32(x: f32) -> f32 {
    f32::from_bits(x.to_bits() & (i32::MAX as u32))
}

ceil and floor require some comparison with the exponent and the mantissa, should be easy enough. Really the only non-trivial issue is sqrt. We could write a newton iteration for that, but it is not guaranteed to complete in a fixed time, i.e. it would be a loop with an unstable termination condition. Here I'd much prefer to use the respective hardware instruction.

In short, I think it is mostly feasible to implement this by hand, but I believe it would be wise to tap into num-traits and be done with it.

wucke13 added a commit that referenced this issue Jul 24, 2024
A recent discussion in issue #50 sparked a need for clarification w/r/t
the introduction of dependencies. This refines the requirements to
answer the questions w/r/t dependencies.

Signed-off-by: wucke13 <[email protected]>
wucke13 added a commit that referenced this issue Jul 24, 2024
A recent discussion in issue #50 sparked a need for clarification w/r/t
the introduction of dependencies. This commit adds requirements to
answer the questions w/r/t dependencies.

Signed-off-by: wucke13 <[email protected]>
@nerodesu017
Copy link
Collaborator Author

We agreed to use libm, but not num-crates

@nerodesu017
Copy link
Collaborator Author

As requested by @wucke13 here is the link that talks about some architectures silently altering floats:

bytecodealliance/wasmtime#2251 (comment)

wucke13 added a commit that referenced this issue Jul 24, 2024
A recent discussion in issue #50 sparked a need for clarification w/r/t
the introduction of dependencies. This commit adds requirements to
answer the questions w/r/t dependencies.

Signed-off-by: wucke13 <[email protected]>
@wucke13
Copy link
Collaborator

wucke13 commented Jul 31, 2024

That is an interesting read. IIUC, the TL;DR of it is: bit representation (and especially the case distinctions) for NaN are not handled identical on all architectures. Hence wasmtime chose to implement these themselves. I'm still in favor of just using libm for now, and revisit this issue some-time later, as I do not expect immediate issue with this in-determinism w/r/t NaN representations. However, I do not have a strong opinion on this.

github-merge-queue bot pushed a commit that referenced this issue Jul 31, 2024
A recent discussion in issue #50 sparked a need for clarification w/r/t
the introduction of dependencies. This commit adds requirements to
answer the questions w/r/t dependencies.

Signed-off-by: wucke13 <[email protected]>
@valexandru
Copy link
Collaborator

Specifically talking about NaNs, I think it is not a critical problem, but I would keep in mind to fix it later in a similar manner to wasmtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants