-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
std: Add compatibility with android-9 #33211
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
// but hopefully that should be enough for now... | ||
|
||
pub fn log2f32(f: f32) -> f32 { | ||
f.ln() / 2.0f32.ln() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ln(2) can be precalculated and its value put into a constant to be as accurate as possible. I doubt the compiler does that for us (and certainly doesn’t in the debug builds).
Here’s a way more than enough digits of ln(2)
to cover f64
:
0.6931471805599453094172321214581765680755001343602552541206800094933936219696947156058633269964186875420014810205...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps std::f64::consts::LN_2
might help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better, instead of dividing by std::f64::consts::LN_2
it would be possible to multiply by std::f64::consts::LOG2_E
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt the compiler does that for us (and certainly doesn’t in the debug builds).
FWIW, it does in release builds (using one of the constants is nicer of course).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, it does in release builds (using one of the constants is nicer of course).
Might not do it in the cross-compiling case, since I think it actually uses the host's (!) libm
to do the calculation, which isn't necessarily valid in the cross-compiling case.
LGTM once the two comments are resolved. |
c38c213
to
0be64f3
Compare
@bors: r=nagisa 0be64f3ea384dfddbe5cec685d9cda13e3d2fd94 |
⌛ Testing commit 0be64f3 with merge 33ec2aa... |
💔 Test failed - auto-linux-64-x-android-t |
0be64f3
to
0e4ff0a
Compare
@bors: r=nagisa 0e4ff0a |
⌛ Testing commit 0e4ff0a with merge f25455d... |
💔 Test failed - auto-linux-64-x-android-t |
0e4ff0a
to
04b78c1
Compare
@bors: r=nagisa 04b78c1 |
⌛ Testing commit 04b78c1 with merge 3b9f856... |
💔 Test failed - auto-linux-64-x-android-t |
The Gecko folks currently use Android API level 9 for their builds, so they're requesting that we move back our minimum supported API level from 18 to 9. Turns out, ABI-wise at least, there's not that many changes we need to take care of. The `ftruncate64` API appeared in android-12 and the `log2` and `log2f` APIs appeared in android-18. We can have a simple shim for `ftruncate64` which falls back on `ftruncate` and the `log2` function can be approximated with just `ln(f) / ln(2)`. This should at least get the standard library building on API level 9, although the tests aren't quite happening there just yet. As we seem to be growing a number of Android compatibility shims, they're now centralized in a common `sys::android` module.
04b78c1
to
c31e2e7
Compare
std: Add compatibility with android-9 The Gecko folks currently use Android API level 9 for their builds, so they're requesting that we move back our minimum supported API level from 18 to 9. Turns out, ABI-wise at least, there's not that many changes we need to take care of. The `ftruncate64` API appeared in android-12 and the `log2` and `log2f` APIs appeared in android-18. We can have a simple shim for `ftruncate64` which falls back on `ftruncate` and the `log2` function can be approximated with just `ln(f) / ln(2)`. This should at least get the standard library building on API level 9, although the tests aren't quite happening there just yet. As we seem to be growing a number of Android compatibility shims, they're now centralized in a common `sys::android` module.
The Gecko folks currently use Android API level 9 for their builds, so they're
requesting that we move back our minimum supported API level from 18 to 9. Turns
out, ABI-wise at least, there's not that many changes we need to take care of.
The
ftruncate64
API appeared in android-12 and thelog2
andlog2f
APIsappeared in android-18. We can have a simple shim for
ftruncate64
which fallsback on
ftruncate
and thelog2
function can be approximated with justln(f) / ln(2)
.This should at least get the standard library building on API level 9, although
the tests aren't quite happening there just yet. As we seem to be growing a
number of Android compatibility shims, they're now centralized in a common
sys::android
module.