Skip to content

Miri: handling of SNaN inputs in f*::pow operations#142514

Merged
bors merged 2 commits into
rust-lang:masterfrom
LorrensP-2158466:miri-float-nondet-pow
Jun 30, 2025
Merged

Miri: handling of SNaN inputs in f*::pow operations#142514
bors merged 2 commits into
rust-lang:masterfrom
LorrensP-2158466:miri-float-nondet-pow

Conversation

@LorrensP-2158466

@LorrensP-2158466 LorrensP-2158466 commented Jun 14, 2025

Copy link
Copy Markdown
Contributor

fixes miri/#4286 and related to #138062 and miri/#4208.

For the following cases of the powf or powi operations, Miri returns either 1.0 or an arbitrary NaN:

  • powf(SNaN, 0.0)
  • powf(1.0, SNaN)
  • powi(SNaN, 0)

Also added a macro in miri/tests/pass/float.rs which 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:

env MIRIFLAGS=-Zmiri-many-seeds ./x miri --no-fail-fast std core coretests -- f32 f64

This was successful. This does take a while, so I recommend using --no-doc and separate use of f32 or f64

The 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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 14, 2025
@rustbot

rustbot commented Jun 14, 2025

Copy link
Copy Markdown
Collaborator

The Miri subtree was changed

cc @rust-lang/miri

Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment on lines +506 to +512
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()))
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have extensive logic for random NaNs (generate_nan), please use that instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah 🤦

Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
("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) }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 { ...

Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/tests/pass/float.rs Outdated
}

// x^(SNaN) = (1 | NaN)
test_snan_nondet!(f32::powf(SNAN_F32, 0.0));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't invent new logic, but use existing logic. We already have ensure_nondet.

@RalfJung

Copy link
Copy Markdown
Member

Sorry for the slow review, I was traveling. And due to an upcoming deadline I'll be quite busy for another 2 weeks, unfortunately.

@RalfJung

Copy link
Copy Markdown
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 27, 2025
@rustbot

rustbot commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@RalfJung

Copy link
Copy Markdown
Member

@bors2 try jobs=x86_64-gnu-aux

@rust-bors

rust-bors Bot commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

⌛ Trying commit 550a13b with merge 1dc8c00

To cancel the try build, run the command @bors2 try cancel.

@rust-bors

rust-bors Bot commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 1dc8c00 (1dc8c007860b211c121063d9186c775b10a289d8, parent: fe5f3dedf7b4d6bea2cadb17343f747d70b4c66b)

@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

Sorry for the slow review, I was traveling. And due to an upcoming deadline I'll be quite busy for another 2 weeks, unfortunately.

No worries! I had exams, so I couldn't respond to the review if it had been done sooner.

@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 29, 2025
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/src/intrinsics/mod.rs Outdated
Comment thread src/tools/miri/tests/pass/float.rs Outdated
Comment thread src/tools/miri/tests/pass/float.rs Outdated
Comment thread src/tools/miri/tests/pass/float.rs Outdated
@RalfJung RalfJung force-pushed the miri-float-nondet-pow branch from 6718b82 to 06dd8bc Compare June 29, 2025 20:26
@RalfJung

Copy link
Copy Markdown
Member

Thanks! I added a commit with some more tests. Should be good to go now :)
@bors r+ rollup

@bors

bors commented Jun 29, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 06dd8bc has been approved by RalfJung

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 29, 2025
@RalfJung RalfJung force-pushed the miri-float-nondet-pow branch from 06dd8bc to e059457 Compare June 29, 2025 20:31
plus various minor tweaks
@RalfJung RalfJung force-pushed the miri-float-nondet-pow branch from e059457 to 67ab61e Compare June 29, 2025 20:32
@RalfJung

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jun 29, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 67ab61e has been approved by RalfJung

It is now in the queue for this repository.

@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

Thanks! I added a commit with some more tests. Should be good to go now :)

Oh wow, thanks!

@bors bors merged commit d2dc99c into rust-lang:master Jun 30, 2025
@rustbot rustbot added this to the 1.90.0 milestone Jun 30, 2025
@LorrensP-2158466 LorrensP-2158466 deleted the miri-float-nondet-pow branch July 13, 2025 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make powf/powi(sNaN, 0) over-approximate both glibc and musl

4 participants