Miri: handling of SNaN inputs in f*::pow operations#142514
Conversation
|
The Miri subtree was changed cc @rust-lang/miri |
| fn random_nan<S: Semantics>(rng: &mut StdRng) -> IeeeFloat<S> { | ||
| if rng.random() { | ||
| IeeeFloat::<S>::snan(Some(rng.random())) | ||
| } else { | ||
| IeeeFloat::<S>::qnan(Some(rng.random())) | ||
| } | ||
| } |
There was a problem hiding this comment.
We already have extensive logic for random NaNs (generate_nan), please use that instead.
| ("powf32" | "powf64", [base, exp]) if *base == one => { | ||
| let rng = ecx.machine.rng.get_mut(); | ||
| // Handle both the musl and glibc cases non-deterministically. | ||
| if !exp.is_signaling() || rng.random() { one } else { random_nan(rng) } |
There was a problem hiding this comment.
This is too compact to be readable. Also, this needs to honor the (new) float_nondet flag.
let return_nan = exp.is_signaling() && ecx.machine.float_nondet && rng.random();
if return_nan { ...| } | ||
|
|
||
| // x^(SNaN) = (1 | NaN) | ||
| test_snan_nondet!(f32::powf(SNAN_F32, 0.0)); |
There was a problem hiding this comment.
Please don't invent new logic, but use existing logic. We already have ensure_nondet.
|
Sorry for the slow review, I was traveling. And due to an upcoming deadline I'll be quite busy for another 2 weeks, unfortunately. |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
@bors2 try jobs=x86_64-gnu-aux |
No worries! I had exams, so I couldn't respond to the review if it had been done sooner. |
550a13b to
f49980c
Compare
|
@rustbot ready |
6718b82 to
06dd8bc
Compare
|
Thanks! I added a commit with some more tests. Should be good to go now :) |
06dd8bc to
e059457
Compare
plus various minor tweaks
e059457 to
67ab61e
Compare
|
@bors r+ |
Oh wow, thanks! |
fixes miri/#4286 and related to #138062 and miri/#4208.
For the following cases of the powf or powi operations, Miri returns either
1.0or an arbitraryNaN:powf(SNaN, 0.0)powf(1.0, SNaN)powi(SNaN, 0)Also added a macro in
miri/tests/pass/float.rswhich conveniently checks if both are indeed returned from such an operation.Made these changes in the rust repo so I could test against stdlib, since these were impacted some time ago and were fixed in #138062. Tested with:
This was successful. This does take a while, so I recommend using
--no-docand separate use off32orf64The pr is somewhat split up into 3 main commits, which implement the cases described above. The first commit also introduces the macro, and the last commit is just a global refactor of some things.
r? @RalfJung