Switch DataValue to use Ieee32/Ieee64

As discussed in #2251, in order to be very confident that NaN signaling bits are correctly handled by the compiler, this switches `DataValue` to use Cranelift's `Ieee32` and `Ieee64` structures. This makes it a bit more inconvenient to interpreter Cranelift FP operations but this should change to something like `rustc_apfloat` in the future.

Author: abrown

cranelift/codegen/src/data_value.rs

@@ -18,8 +18,8 @@ pub enum DataValue {
     I16(i16),
     I32(i32),
     I64(i64),
-    F32(f32),
-    F64(f64),
+    F32(Ieee32),
+    F64(Ieee64),
     V128([u8; 16]),
 }
 
@@ -97,19 +97,9 @@ build_conversion_impl!(i8, I8, I8);
 build_conversion_impl!(i16, I16, I16);
 build_conversion_impl!(i32, I32, I32);
 build_conversion_impl!(i64, I64, I64);
-build_conversion_impl!(f32, F32, F32);
-build_conversion_impl!(f64, F64, F64);
+build_conversion_impl!(Ieee32, F32, F32);
+build_conversion_impl!(Ieee64, F64, F64);
 build_conversion_impl!([u8; 16], V128, I8X16);
-impl From<Ieee64> for DataValue {
-    fn from(f: Ieee64) -> Self {
-        DataValue::from(f64::from_bits(f.bits()))
-    }
-}
-impl From<Ieee32> for DataValue {
-    fn from(f: Ieee32) -> Self {
-        DataValue::from(f32::from_bits(f.bits()))
-    }
-}
 impl From<Offset32> for DataValue {
     fn from(o: Offset32) -> Self {
         DataValue::from(Into::<i32>::into(o))
@@ -124,9 +114,9 @@ impl Display for DataValue {
             DataValue::I16(dv) => write!(f, "{}", dv),
             DataValue::I32(dv) => write!(f, "{}", dv),
             DataValue::I64(dv) => write!(f, "{}", dv),
-            // Use the Ieee* wrappers here to maintain a consistent syntax.
-            DataValue::F32(dv) => write!(f, "{}", Ieee32::from(*dv)),
-            DataValue::F64(dv) => write!(f, "{}", Ieee64::from(*dv)),
+            // The Ieee* wrappers here print the expected syntax.
+            DataValue::F32(dv) => write!(f, "{}", dv),
+            DataValue::F64(dv) => write!(f, "{}", dv),
             // Again, for syntax consistency, use ConstantData, which in this case displays as hex.
             DataValue::V128(dv) => write!(f, "{}", ConstantData::from(&dv[..])),
         }

cranelift/codegen/src/ir/immediates.rs

@@ -450,13 +450,15 @@ impl FromStr for Offset32 {
 ///
 /// All bit patterns are allowed.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
+#[repr(C)]
 pub struct Ieee32(u32);
 
 /// An IEEE binary64 immediate floating point value, represented as a u64
 /// containing the bit pattern.
 ///
 /// All bit patterns are allowed.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
+#[repr(C)]
 pub struct Ieee64(u64);
 
 /// Format a floating point number in a way that is reasonably human-readable, and that can be

cranelift/filetests/src/function_runner.rs

@@ -2,6 +2,7 @@
 use core::{mem, ptr};
 use cranelift_codegen::binemit::{NullRelocSink, NullStackMapSink, NullTrapSink};
 use cranelift_codegen::data_value::DataValue;
+use cranelift_codegen::ir::immediates::{Ieee32, Ieee64};
 use cranelift_codegen::ir::{condcodes::IntCC, Function, InstBuilder, Signature, Type};
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::{ir, settings, CodegenError, Context};
@@ -233,8 +234,8 @@ impl UnboxedValues {
             DataValue::I16(i) => ptr::write(p as *mut i16, *i),
             DataValue::I32(i) => ptr::write(p as *mut i32, *i),
             DataValue::I64(i) => ptr::write(p as *mut i64, *i),
-            DataValue::F32(f) => ptr::write(p as *mut f32, *f),
-            DataValue::F64(f) => ptr::write(p as *mut f64, *f),
+            DataValue::F32(f) => ptr::write(p as *mut Ieee32, *f),
+            DataValue::F64(f) => ptr::write(p as *mut Ieee64, *f),
             DataValue::V128(b) => ptr::write(p as *mut [u8; 16], *b),
         }
     }
@@ -246,8 +247,8 @@ impl UnboxedValues {
             ir::types::I16 => DataValue::I16(ptr::read(p as *const i16)),
             ir::types::I32 => DataValue::I32(ptr::read(p as *const i32)),
             ir::types::I64 => DataValue::I64(ptr::read(p as *const i64)),
-            ir::types::F32 => DataValue::F32(ptr::read(p as *const f32)),
-            ir::types::F64 => DataValue::F64(ptr::read(p as *const f64)),
+            ir::types::F32 => DataValue::F32(ptr::read(p as *const Ieee32)),
+            ir::types::F64 => DataValue::F64(ptr::read(p as *const Ieee64)),
             _ if ty.is_bool() => DataValue::B(ptr::read(p as *const bool)),
             _ if ty.is_vector() && ty.bytes() == 16 => {
                 DataValue::V128(ptr::read(p as *const [u8; 16]))

cranelift/interpreter/src/interpreter.rs

@@ -12,7 +12,7 @@ use cranelift_codegen::ir::{
     Value as ValueRef, ValueList,
 };
 use log::trace;
-use std::ops::{Add, Div, Mul, Sub};
+use std::ops::{Add, Mul, Sub};
 use thiserror::Error;
 
 /// The valid control flow states.
@@ -153,10 +153,11 @@ impl Interpreter {
                     Iadd => binary_op!(Add::add[arg1, arg2]; [I8, I16, I32, I64]; inst),
                     Isub => binary_op!(Sub::sub[arg1, arg2]; [I8, I16, I32, I64]; inst),
                     Imul => binary_op!(Mul::mul[arg1, arg2]; [I8, I16, I32, I64]; inst),
-                    Fadd => binary_op!(Add::add[arg1, arg2]; [F32, F64]; inst),
-                    Fsub => binary_op!(Sub::sub[arg1, arg2]; [F32, F64]; inst),
-                    Fmul => binary_op!(Mul::mul[arg1, arg2]; [F32, F64]; inst),
-                    Fdiv => binary_op!(Div::div[arg1, arg2]; [F32, F64]; inst),
+                    // TODO re-enable by importing something like rustc_apfloat for correctness.
+                    // Fadd => binary_op!(Add::add[arg1, arg2]; [F32, F64]; inst),
+                    // Fsub => binary_op!(Sub::sub[arg1, arg2]; [F32, F64]; inst),
+                    // Fmul => binary_op!(Mul::mul[arg1, arg2]; [F32, F64]; inst),
+                    // Fdiv => binary_op!(Div::div[arg1, arg2]; [F32, F64]; inst),
                     _ => unimplemented!("interpreter does not support opcode yet: {}", opcode),
                 }?;
                 frame.set(first_result(frame.function, inst), result);

cranelift/reader/src/parser.rs

@@ -2700,8 +2700,8 @@ impl<'a> Parser<'a> {
             I16 => DataValue::from(self.match_imm16("expected an i16")?),
             I32 => DataValue::from(self.match_imm32("expected an i32")?),
             I64 => DataValue::from(Into::<i64>::into(self.match_imm64("expected an i64")?)),
-            F32 => DataValue::from(f32::from_bits(self.match_ieee32("expected an f32")?.bits())),
-            F64 => DataValue::from(f64::from_bits(self.match_ieee64("expected an f64")?.bits())),
+            F32 => DataValue::from(self.match_ieee32("expected an f32")?),
+            F64 => DataValue::from(self.match_ieee64("expected an f64")?),
             _ if ty.is_vector() => {
                 let as_vec = self.match_uimm128(ty)?.into_vec();
                 if as_vec.len() == 16 {