Skip to content

Commit

Permalink
miri: make float min/max non-deterministic
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jan 31, 2025
1 parent 25a1657 commit d98270b
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 6 deletions.
16 changes: 14 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
{
let a: F = self.read_scalar(&args[0])?.to_float()?;
let b: F = self.read_scalar(&args[1])?.to_float()?;
let res = self.adjust_nan(a.min(b), &[a, b]);
let res = if a == b {
// They are definitely not NaN (those are never equal), but they could be `+0` and `-0`.
// Let the machine decide which one to return.
M::equal_float_min_max(self, a, b)
} else {
self.adjust_nan(a.min(b), &[a, b])
};
self.write_scalar(res, dest)?;
interp_ok(())
}
Expand All @@ -762,7 +768,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
{
let a: F = self.read_scalar(&args[0])?.to_float()?;
let b: F = self.read_scalar(&args[1])?.to_float()?;
let res = self.adjust_nan(a.max(b), &[a, b]);
let res = if a == b {
// They are definitely not NaN (those are never equal), but they could be `+0` and `-0`.
// Let the machine decide which one to return.
M::equal_float_min_max(self, a, b)
} else {
self.adjust_nan(a.max(b), &[a, b])
};
self.write_scalar(res, dest)?;
interp_ok(())
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ pub trait Machine<'tcx>: Sized {
F2::NAN
}

/// Determines the result of `min`/`max` on floats when the arguments are equal.
fn equal_float_min_max<F: Float>(_ecx: &InterpCx<'tcx, Self>, a: F, _b: F) -> F {
// By default, we pick the left argument.
a
}

/// Called before a basic block terminator is executed.
#[inline]
fn before_terminator(_ecx: &mut InterpCx<'tcx, Self>) -> InterpResult<'tcx> {
Expand Down
13 changes: 9 additions & 4 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::{fmt, process};
use rand::rngs::StdRng;
use rand::{Rng, SeedableRng};
use rustc_abi::{Align, ExternAbi, Size};
use rustc_apfloat::{Float, FloatConvert};
use rustc_attr_parsing::InlineAttr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
#[allow(unused)]
Expand Down Expand Up @@ -1129,20 +1130,24 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
}

#[inline(always)]
fn generate_nan<
F1: rustc_apfloat::Float + rustc_apfloat::FloatConvert<F2>,
F2: rustc_apfloat::Float,
>(
fn generate_nan<F1: Float + FloatConvert<F2>, F2: Float>(
ecx: &InterpCx<'tcx, Self>,
inputs: &[F1],
) -> F2 {
ecx.generate_nan(inputs)
}

#[inline(always)]
fn equal_float_min_max<F: Float>(ecx: &MiriInterpCx<'tcx>, a: F, b: F) -> F {
ecx.equal_float_min_max(a, b)
}

#[inline(always)]
fn ub_checks(ecx: &InterpCx<'tcx, Self>) -> InterpResult<'tcx, bool> {
interp_ok(ecx.tcx.sess.ub_checks())
}

#[inline(always)]
fn thread_local_static_pointer(
ecx: &mut MiriInterpCx<'tcx>,
def_id: DefId,
Expand Down
7 changes: 7 additions & 0 deletions src/tools/miri/src/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
nan
}
}

fn equal_float_min_max<F: Float>(&self, a: F, b: F) -> F {
let this = self.eval_context_ref();
// Return one side non-deterministically.
let mut rand = this.machine.rng.borrow_mut();
if rand.gen() { a } else { b }
}
}
28 changes: 28 additions & 0 deletions src/tools/miri/tests/pass/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fn main() {
test_fast();
test_algebraic();
test_fmuladd();
test_min_max_nondet();
}

trait Float: Copy + PartialEq + Debug {
Expand Down Expand Up @@ -1211,3 +1212,30 @@ fn test_fmuladd() {
test_operations_f32(0.1, 0.2, 0.3);
test_operations_f64(1.1, 1.2, 1.3);
}

/// `min` and `max` on equal arguments are non-deterministic.
fn test_min_max_nondet() {
/// Ensure that if we call the closure often enough, we see both `true` and `false.`
#[track_caller]
fn ensure_both(f: impl Fn() -> bool) {
let rounds = 16;
let first = f();
for _ in 1..rounds {
if f() != first {
// We saw two different values!
return;
}
}
// We saw the same thing N times.
panic!("expected non-determinism, got {rounds} times the same result: {first:?}");
}

ensure_both(|| f16::min(0.0, -0.0).is_sign_positive());
ensure_both(|| f16::max(0.0, -0.0).is_sign_positive());
ensure_both(|| f32::min(0.0, -0.0).is_sign_positive());
ensure_both(|| f32::max(0.0, -0.0).is_sign_positive());
ensure_both(|| f64::min(0.0, -0.0).is_sign_positive());
ensure_both(|| f64::max(0.0, -0.0).is_sign_positive());
ensure_both(|| f128::min(0.0, -0.0).is_sign_positive());
ensure_both(|| f128::max(0.0, -0.0).is_sign_positive());
}

0 comments on commit d98270b

Please sign in to comment.