-
Notifications
You must be signed in to change notification settings - Fork 385
Missing cmath function: ldexp #821
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
Comments
I am taking care of this one |
Awesome! Do you need mentoring? The file to edit is this one. |
Thank you Ralf. It seems my gut feeling was right this time :D that's the file I modified. I just added a new match under "ldexp" => {
let x = f32::from_bits(this.read_scalar(args[0])?.to_u32()?);
let exp = f32::from_bits(this.read_scalar(args[1])?.to_u32()?);
this.write_scalar(Scalar::from_u32((x * 2.0f32.powf(exp)).to_bits()), dest)?;
} Should that be enough? Or should I use the crate you mentioned? I also suppose we should remove https://github.com/rust-lang/rust/blob/0f11354a9c1bf0c5ac250c7fa2bafc289a662f42/src/libcore/tests/num/flt2dec/mod.rs#L1, is that right? |
Hm. I guess it is a reasonable start. Maybe add a FIXME saying that if we see imprecise results, we should try to use the C math lib Your code should also, like the other math functions, have a FIXME because it uses host floats. And finally, find an appropriate test case to extend and call the function there.
Once this lands in Miri and Miri has been updated in rustc, that can be removed, yes. One step after the other. :) |
I can use the C math library directly if you think that's a more definitive solution.
Ok, then I will do the miri PR first and wait :) |
That is more in line with what we do with the other functions. I just don't know if it will work on all platforms. |
Seems like |
I am seeing test failures when turning on the libcore tests that use |
I'll fix this ASAP :) |
The libcore test suites calls
ldexp
via FFI. Unfortunately, libstd does not expose this function in any way. What would be the best way for us to implement this shim?I found this crate. Alternatively we could try to just define the
extern
symbol ourselves and see if that works on all our host platforms?^^The text was updated successfully, but these errors were encountered: