diff --git a/cranelift/codegen/src/data_value.rs b/cranelift/codegen/src/data_value.rs index affc5b711a..86f1882c32 100644 --- a/cranelift/codegen/src/data_value.rs +++ b/cranelift/codegen/src/data_value.rs @@ -10,7 +10,7 @@ use core::fmt::{self, Display, Formatter}; /// /// [Value]: crate::ir::Value #[allow(missing_docs)] -#[derive(Clone, Debug, PartialEq, PartialOrd)] +#[derive(Clone, Debug, PartialOrd)] pub enum DataValue { B(bool), I8(i8), @@ -29,6 +29,44 @@ pub enum DataValue { V64([u8; 8]), } +impl PartialEq for DataValue { + fn eq(&self, other: &Self) -> bool { + use DataValue::*; + match (self, other) { + (B(l), B(r)) => l == r, + (B(_), _) => false, + (I8(l), I8(r)) => l == r, + (I8(_), _) => false, + (I16(l), I16(r)) => l == r, + (I16(_), _) => false, + (I32(l), I32(r)) => l == r, + (I32(_), _) => false, + (I64(l), I64(r)) => l == r, + (I64(_), _) => false, + (I128(l), I128(r)) => l == r, + (I128(_), _) => false, + (U8(l), U8(r)) => l == r, + (U8(_), _) => false, + (U16(l), U16(r)) => l == r, + (U16(_), _) => false, + (U32(l), U32(r)) => l == r, + (U32(_), _) => false, + (U64(l), U64(r)) => l == r, + (U64(_), _) => false, + (U128(l), U128(r)) => l == r, + (U128(_), _) => false, + (F32(l), F32(r)) => l.as_f32() == r.as_f32(), + (F32(_), _) => false, + (F64(l), F64(r)) => l.as_f64() == r.as_f64(), + (F64(_), _) => false, + (V128(l), V128(r)) => l == r, + (V128(_), _) => false, + (V64(l), V64(r)) => l == r, + (V64(_), _) => false, + } + } +} + impl DataValue { /// Try to cast an immediate integer (a wrapped `i64` on most Cranelift instructions) to the /// given Cranelift [Type]. diff --git a/cranelift/codegen/src/ir/immediates.rs b/cranelift/codegen/src/ir/immediates.rs index 445e3f9ae5..a98a9596f6 100644 --- a/cranelift/codegen/src/ir/immediates.rs +++ b/cranelift/codegen/src/ir/immediates.rs @@ -475,8 +475,11 @@ impl FromStr for Offset32 { /// We specifically avoid using a f32 here since some architectures may silently alter floats. /// See: https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646 /// +/// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but +/// [PartialOrd] respects IEEE754 semantics. +/// /// All bit patterns are allowed. -#[derive(Copy, Clone, Debug, Eq, Hash)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] #[repr(C)] pub struct Ieee32(u32); @@ -487,8 +490,11 @@ pub struct Ieee32(u32); /// We specifically avoid using a f64 here since some architectures may silently alter floats. /// See: https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646 /// +/// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but +/// [PartialOrd] respects IEEE754 semantics. +/// /// All bit patterns are allowed. -#[derive(Copy, Clone, Debug, Eq, Hash)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] #[repr(C)] pub struct Ieee64(u64); @@ -845,12 +851,6 @@ impl PartialOrd for Ieee32 { } } -impl PartialEq for Ieee32 { - fn eq(&self, other: &Ieee32) -> bool { - self.as_f32().eq(&other.as_f32()) - } -} - impl Display for Ieee32 { fn fmt(&self, f: &mut Formatter) -> fmt::Result { let bits: u32 = self.0; @@ -1037,12 +1037,6 @@ impl PartialOrd for Ieee64 { } } -impl PartialEq for Ieee64 { - fn eq(&self, other: &Ieee64) -> bool { - self.as_f64().eq(&other.as_f64()) - } -} - impl Display for Ieee64 { fn fmt(&self, f: &mut Formatter) -> fmt::Result { let bits: u64 = self.0; diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index 02a4d48e87..2f84525041 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -17,7 +17,6 @@ use core::str::FromStr; use serde::{Deserialize, Serialize}; use crate::bitset::BitSet; -use crate::data_value::DataValue; use crate::entity; use crate::ir::{ self, @@ -289,39 +288,6 @@ impl InstructionData { } } - /// Return the value of an immediate if the instruction has one or `None` otherwise. Only - /// immediate values are considered, not global values, constant handles, condition codes, etc. - pub fn imm_value(&self) -> Option { - match self { - &InstructionData::UnaryBool { imm, .. } => Some(DataValue::from(imm)), - // 8-bit. - &InstructionData::BinaryImm8 { imm, .. } - | &InstructionData::TernaryImm8 { imm, .. } => Some(DataValue::from(imm as i8)), // Note the switch from unsigned to signed. - // 32-bit - &InstructionData::UnaryIeee32 { imm, .. } => Some(DataValue::from(imm)), - &InstructionData::HeapAddr { imm, .. } => { - let imm: u32 = imm.into(); - Some(DataValue::from(imm as i32)) // Note the switch from unsigned to signed. - } - &InstructionData::Load { offset, .. } - | &InstructionData::Store { offset, .. } - | &InstructionData::StackLoad { offset, .. } - | &InstructionData::StackStore { offset, .. } - | &InstructionData::TableAddr { offset, .. } => Some(DataValue::from(offset)), - // 64-bit. - &InstructionData::UnaryImm { imm, .. } - | &InstructionData::BinaryImm64 { imm, .. } - | &InstructionData::IntCompareImm { imm, .. } => Some(DataValue::from(imm.bits())), - &InstructionData::UnaryIeee64 { imm, .. } => Some(DataValue::from(imm)), - // 128-bit; though these immediates are present logically in the IR they are not - // included in the `InstructionData` for memory-size reasons. This case, returning - // `None`, is left here to alert users of this method that they should retrieve the - // value using the `DataFlowGraph`. - &InstructionData::Shuffle { imm: _, .. } => None, - _ => None, - } - } - /// If this is a trapping instruction, get its trap code. Otherwise, return /// `None`. pub fn trap_code(&self) -> Option { diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index fdc0bbf1d6..cf73d0b43a 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -7,7 +7,6 @@ use std::cell::Cell; use target_lexicon::Triple; pub use super::MachLabel; -pub use crate::data_value::DataValue; pub use crate::ir::{ dynamic_to_fixed, ArgumentExtension, Constant, DynamicStackSlot, ExternalName, FuncRef, GlobalValue, Immediate, SigRef, StackSlot, diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 16347734f5..0642246117 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -114,7 +114,28 @@ where length => panic!("unexpected Shuffle mask length {}", length), } } - _ => inst.imm_value().unwrap(), + InstructionData::UnaryBool { imm, .. } => DataValue::from(imm), + // 8-bit. + InstructionData::BinaryImm8 { imm, .. } | InstructionData::TernaryImm8 { imm, .. } => { + DataValue::from(imm as i8) // Note the switch from unsigned to signed. + } + // 32-bit + InstructionData::UnaryIeee32 { imm, .. } => DataValue::from(imm), + InstructionData::HeapAddr { imm, .. } => { + let imm: u32 = imm.into(); + DataValue::from(imm as i32) // Note the switch from unsigned to signed. + } + InstructionData::Load { offset, .. } + | InstructionData::Store { offset, .. } + | InstructionData::StackLoad { offset, .. } + | InstructionData::StackStore { offset, .. } + | InstructionData::TableAddr { offset, .. } => DataValue::from(offset), + // 64-bit. + InstructionData::UnaryImm { imm, .. } + | InstructionData::BinaryImm64 { imm, .. } + | InstructionData::IntCompareImm { imm, .. } => DataValue::from(imm.bits()), + InstructionData::UnaryIeee64 { imm, .. } => DataValue::from(imm), + _ => unreachable!(), }) }; diff --git a/cranelift/interpreter/src/value.rs b/cranelift/interpreter/src/value.rs index 0e0f738e4a..2dccca4720 100644 --- a/cranelift/interpreter/src/value.rs +++ b/cranelift/interpreter/src/value.rs @@ -208,14 +208,6 @@ macro_rules! binary_match { } }; } -macro_rules! comparison_match { - ( $op:path[$arg1:expr, $arg2:expr]; [ $( $data_value_ty:ident ),* ] ) => { - match ($arg1, $arg2) { - $( (DataValue::$data_value_ty(a), DataValue::$data_value_ty(b)) => { Ok($op(a, b)) } )* - _ => unimplemented!("comparison: {:?}, {:?}", $arg1, $arg2) - } - }; -} impl Value for DataValue { fn ty(&self) -> Type { @@ -475,11 +467,11 @@ impl Value for DataValue { } fn eq(&self, other: &Self) -> ValueResult { - comparison_match!(PartialEq::eq[&self, &other]; [I8, I16, I32, I64, I128, U8, U16, U32, U64, U128, F32, F64]) + Ok(self == other) } fn gt(&self, other: &Self) -> ValueResult { - comparison_match!(PartialOrd::gt[&self, &other]; [I8, I16, I32, I64, I128, U8, U16, U32, U64, U128, F32, F64]) + Ok(self > other) } fn uno(&self, other: &Self) -> ValueResult { diff --git a/tests/misc_testsuite/issue4857.wast b/tests/misc_testsuite/issue4857.wast new file mode 100644 index 0000000000..c233b91258 --- /dev/null +++ b/tests/misc_testsuite/issue4857.wast @@ -0,0 +1,10 @@ +(module + (func + i32.const 0 + if + unreachable + end + f32.const nan + drop + ) +)