Skip to content

Commit

Permalink
Make DataValue, not Ieee32/64, respect IEEE754
Browse files Browse the repository at this point in the history
This fixes bytecodealliance#4857 by partially reverting bytecodealliance#4849.

It turns out that Ieee32 and Ieee64 need bitwise equality semantics so
they can be used as hash-table keys.

Moving the IEEE754 semantics up a layer to DataValue makes sense in
conjunction with bytecodealliance#4855, where we introduced a DataValue::bitwise_eq
alternative implementation of equality for those cases where users of
DataValue still want the bitwise equality semantics.
  • Loading branch information
jameysharp committed Sep 2, 2022
1 parent cbfab3f commit ae76e65
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 15 deletions.
40 changes: 39 additions & 1 deletion cranelift/codegen/src/data_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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].
Expand Down
22 changes: 8 additions & 14 deletions cranelift/codegen/src/ir/immediates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -845,12 +851,6 @@ impl PartialOrd for Ieee32 {
}
}

impl PartialEq<Ieee32> 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;
Expand Down Expand Up @@ -1037,12 +1037,6 @@ impl PartialOrd for Ieee64 {
}
}

impl PartialEq<Ieee64> 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;
Expand Down

0 comments on commit ae76e65

Please sign in to comment.