Skip to content

Commit

Permalink
Introduce HashableLiteral
Browse files Browse the repository at this point in the history
The only reason `Literal` had to implement `Eq` and `Hash` was due to the SPV backend having to cache those in a `HashMap`.

The `PartialEq` impl was error prone due to the match not being exhaustive.
  • Loading branch information
teoxoy authored and jimblandy committed Apr 18, 2024
1 parent d08a069 commit ca729cc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 52 deletions.
2 changes: 1 addition & 1 deletion naga/src/back/spv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ impl recyclable::Recyclable for CachedExpressions {

#[derive(Eq, Hash, PartialEq)]
enum CachedConstant {
Literal(crate::Literal),
Literal(crate::proc::HashableLiteral),
Composite {
ty: LookupType,
constituent_ids: Vec<Word>,
Expand Down
2 changes: 1 addition & 1 deletion naga/src/back/spv/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ impl Writer {
}

pub(super) fn get_constant_scalar(&mut self, value: crate::Literal) -> Word {
let scalar = CachedConstant::Literal(value);
let scalar = CachedConstant::Literal(value.into());
if let Some(&id) = self.cached_constants.get(&scalar) {
return id;
}
Expand Down
2 changes: 1 addition & 1 deletion naga/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ pub enum TypeInner {
BindingArray { base: Handle<Type>, size: ArraySize },
}

#[derive(Debug, Clone, Copy, PartialOrd)]
#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
Expand Down
73 changes: 24 additions & 49 deletions naga/src/proc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,56 +153,31 @@ impl super::Scalar {
}
}

impl PartialEq for crate::Literal {
fn eq(&self, other: &Self) -> bool {
match (*self, *other) {
(Self::F64(a), Self::F64(b)) => a.to_bits() == b.to_bits(),
(Self::F32(a), Self::F32(b)) => a.to_bits() == b.to_bits(),
(Self::U32(a), Self::U32(b)) => a == b,
(Self::I32(a), Self::I32(b)) => a == b,
(Self::U64(a), Self::U64(b)) => a == b,
(Self::I64(a), Self::I64(b)) => a == b,
(Self::Bool(a), Self::Bool(b)) => a == b,
_ => false,
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum HashableLiteral {
F64(u64),
F32(u32),
U32(u32),
I32(i32),
U64(u64),
I64(i64),
Bool(bool),
AbstractInt(i64),
AbstractFloat(u64),
}
impl Eq for crate::Literal {}
impl std::hash::Hash for crate::Literal {
fn hash<H: std::hash::Hasher>(&self, hasher: &mut H) {
match *self {
Self::F64(v) | Self::AbstractFloat(v) => {
hasher.write_u8(0);
v.to_bits().hash(hasher);
}
Self::F32(v) => {
hasher.write_u8(1);
v.to_bits().hash(hasher);
}
Self::U32(v) => {
hasher.write_u8(2);
v.hash(hasher);
}
Self::I32(v) => {
hasher.write_u8(3);
v.hash(hasher);
}
Self::Bool(v) => {
hasher.write_u8(4);
v.hash(hasher);
}
Self::I64(v) => {
hasher.write_u8(5);
v.hash(hasher);
}
Self::U64(v) => {
hasher.write_u8(6);
v.hash(hasher);
}
Self::AbstractInt(v) => {
hasher.write_u8(7);
v.hash(hasher);
}

impl From<crate::Literal> for HashableLiteral {
fn from(l: crate::Literal) -> Self {
match l {
crate::Literal::F64(v) => Self::F64(v.to_bits()),
crate::Literal::F32(v) => Self::F32(v.to_bits()),
crate::Literal::U32(v) => Self::U32(v),
crate::Literal::I32(v) => Self::I32(v),
crate::Literal::U64(v) => Self::U64(v),
crate::Literal::I64(v) => Self::I64(v),
crate::Literal::Bool(v) => Self::Bool(v),
crate::Literal::AbstractInt(v) => Self::AbstractInt(v),
crate::Literal::AbstractFloat(v) => Self::AbstractFloat(v.to_bits()),
}
}
}
Expand Down

0 comments on commit ca729cc

Please sign in to comment.