diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0b617a21..acf45202 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -123,6 +123,8 @@ jobs: run: RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps - name: cargo clippy run: cargo clippy --all-targets -- -D warnings + - name: grammar-parser without codegen feature + run: cargo check -p rspirv2-grammar-parser defaults: run: diff --git a/crates/grammar-parser/src/lib.rs b/crates/grammar-parser/src/lib.rs index 113f2a19..60e4aa44 100644 --- a/crates/grammar-parser/src/lib.rs +++ b/crates/grammar-parser/src/lib.rs @@ -6,5 +6,7 @@ pub mod timer; #[cfg(feature = "codegen")] pub mod codegen; +#[cfg(feature = "codegen")] pub use proc_macro2; +#[cfg(feature = "codegen")] pub use quote; diff --git a/crates/rspirv2-types/src/operand/id.rs b/crates/rspirv2-types/src/operand/id.rs index 1a351526..024ac76f 100644 --- a/crates/rspirv2-types/src/operand/id.rs +++ b/crates/rspirv2-types/src/operand/id.rs @@ -103,7 +103,7 @@ impl Display for IdResultWriter<'_> { let style = ctx.color(ID_RESULT_COLOR); write!(f, "{}{style}%{}{style:#} = ", &ctx.padding[..pad_len], name) } else { - write!(f, "{}", &ctx.padding) + write!(f, "{}", ctx.padding) } } } diff --git a/crates/rspirv2-types/src/operand/literal_string.rs b/crates/rspirv2-types/src/operand/literal_string.rs index 2a450a82..e6a4d2b8 100644 --- a/crates/rspirv2-types/src/operand/literal_string.rs +++ b/crates/rspirv2-types/src/operand/literal_string.rs @@ -142,21 +142,11 @@ mod tests { #[test] fn test_str() -> anyhow::Result<()> { - roundtrip("abc", &[[b'a', b'b', b'c', 0]])?; - roundtrip("123", &[[b'1', b'2', b'3', 0]])?; - roundtrip("abcd", &[[b'a', b'b', b'c', b'd'], [0, 0, 0, 0]])?; - roundtrip( - "abcdefg", - &[[b'a', b'b', b'c', b'd'], [b'e', b'f', b'g', 0]], - )?; - roundtrip( - "abcdefgh", - &[ - [b'a', b'b', b'c', b'd'], - [b'e', b'f', b'g', b'h'], - [0, 0, 0, 0], - ], - )?; + roundtrip("abc", &[*b"abc\0"])?; + roundtrip("123", &[*b"123\0"])?; + roundtrip("abcd", &[*b"abcd", *b"\0\0\0\0"])?; + roundtrip("abcdefg", &[*b"abcd", *b"efg\0"])?; + roundtrip("abcdefgh", &[*b"abcd", *b"efgh", *b"\0\0\0\0"])?; Ok(()) } @@ -172,25 +162,19 @@ mod tests { assert_eq!(read.as_ref().map(|s| s.as_str()), str); }; - test(&[[b'a', 0, 0, 0]], Some("a")); - test(&[[b'a', b'b', 0, 0]], Some("ab")); - test(&[[b'a', b'b', b'c', 0]], Some("abc")); - test(&[[b'a', b'b', b'c', b'd'], [0, 0, 0, 0]], Some("abcd")); - test(&[[b'a', b'b', b'c', b'd'], [b'e', 0, 0, 0]], Some("abcde")); - test( - &[[b'a', b'b', b'c', b'd'], [b'e', b'f', b'g', 0]], - Some("abcdefg"), - ); - test( - &[[b'a', b'b', b'c', b'd'], [b'e', b'f', b'g', b'h'], [0; 4]], - Some("abcdefgh"), - ); + test(&[*b"a\0\0\0"], Some("a")); + test(&[*b"ab\0\0"], Some("ab")); + test(&[*b"abc\0"], Some("abc")); + test(&[*b"abcd", *b"\0\0\0\0"], Some("abcd")); + test(&[*b"abcd", *b"e\0\0\0"], Some("abcde")); + test(&[*b"abcd", *b"efg\0"], Some("abcdefg")); + test(&[*b"abcd", *b"efgh", *b"\0\0\0\0"], Some("abcdefgh")); // missing null terminator test(&[], None); - test(&[[b'a', b'b', b'c', b'd']], None); - test(&[[b'a', b'b', b'c', b'd'], [b'e', b'f', b'g', b'h']], None); + test(&[*b"abcd"], None); + test(&[*b"abcd", *b"efgh"], None); - test(&[[0, 0, 0, 0]], Some("")); + test(&[*b"\0\0\0\0"], Some("")); } } diff --git a/crates/rspirv2-types/src/slice.rs b/crates/rspirv2-types/src/slice.rs index 194b2c4b..e6f90745 100644 --- a/crates/rspirv2-types/src/slice.rs +++ b/crates/rspirv2-types/src/slice.rs @@ -4,7 +4,7 @@ use crate::dis::{DisInstSlice, InstSetDisCtx, IntoDisContext}; use crate::inst::{InstEncoding, InstRef}; use std::fmt::{Debug, Formatter}; use std::marker::PhantomData; -use std::ops::Deref; +use std::ops::{Bound, Deref, Index, RangeBounds}; pub fn decode_failed(e: DecodeError) -> ! { panic!("Decode failed: {e}") @@ -75,6 +75,12 @@ impl InstSlice { unsafe { core::mem::transmute(raw) } } + /// An [`InstSlice`] with 0 instructions + #[inline] + pub const fn empty() -> &'static Self { + Self::from_words_unchecked(&[]) + } + /// View self as a [`RawInstSlice`] #[inline] pub const fn as_raw(&self) -> &RawInstSlice { @@ -111,6 +117,55 @@ impl Debug for InstSlice { } } +impl InstSlice { + /// Get instruction at `offset` as an [`InstRef`], return `None` when index is invalid + pub fn get_ref(&self, offset: InstOffset) -> Option> { + self.iter_ref().with_offsets().advance_to(offset) + } + + /// Get instruction at `offset`, return `None` when index is invalid + pub fn get(&self, offset: InstOffset) -> Option { + Some(self.get_ref(offset)?.get()) + } + + /// Get instruction at `offset` as an [`InstRef`], panic when index is invalid + pub fn index_ref(&self, offset: InstOffset) -> InstRef<'_, ISA> { + self.get_ref(offset) + .unwrap_or_else(|| panic!("Offset {offset} invalid for this InstSlice")) + } + + /// Get instruction at `offset`, panic when index is invalid + pub fn index(&self, offset: InstOffset) -> ISA { + self.index_ref(offset).get() + } + + /// Slice this [`InstSlice`] + pub fn slice>(&self, index: R) -> Option<&Self> { + let mut iter = self.iter_ref().with_offsets(); + let start = iter.advance_to_bound(index.start_bound(), false)?; + // reusing the same iter to not have to advance it twice over `..start` insts + // if end < start, the advance may fail due to already having skipped over the end inst, but that's fine since + // indexing a slice leads to failure anyway. + // Important detail: When you hit the offset, do NOT advance the iterator, otherwise `0..=0` would fail + let end = iter.advance_to_bound(index.end_bound(), true)?; + let slice = match (start, end) { + (Some(start), Some(end)) => &self.0[start..end], + (Some(start), None) => &self.0[start..], + (None, Some(end)) => &self.0[..end], + (None, None) => &self.0[..], + }; + Some(InstSlice::from_words_unchecked(slice)) + } +} + +impl> Index for InstSlice { + type Output = InstSlice; + + fn index(&self, index: R) -> &Self::Output { + self.slice(index).expect("Index out of bounds") + } +} + impl InstSlice { /// disassemble #[inline] @@ -133,6 +188,67 @@ impl<'a, ISA: InstEncoding> InstOffsetRefIter<'a, ISA> { _phantom: PhantomData, } } + + pub fn offset(&self) -> InstOffset { + self.inner.offset() + } + + pub fn peek(&self) -> Option<(InstOffset, InstRef<'a, ISA>)> { + Some(reader_to_inst_ref_offset(self.inner.peek()?)) + } + + /// Advance the iterator to this offset and return an [`InstRef`] to the instruction at this offset. + /// + /// May return `None` if offset is out of bounds, offset is within an instruction and not at the start of one, or + /// this Iterator has advanced beyond the requested offset already. + pub fn advance_to(&mut self, to: InstOffset) -> Option> { + while let Some((off, inst)) = self.peek() { + if off == to { + return Some(inst); + } else if off > to { + // jumped over offset -> offset within an inst or iter has advanced too far before calling this + return None; + } + self.next(); + } + // eof + None + } + + /// outer Option: failure due to eof or in the middle of insts + /// inner Option: Bound or Unbounded + #[expect(clippy::option_option)] + fn advance_to_bound(&mut self, bound: Bound<&InstOffset>, end: bool) -> Option> { + let to = match bound.cloned() { + Bound::Included(to) | Bound::Excluded(to) => to, + Bound::Unbounded => { + return Some(None); + } + }; + let one_further = matches!(bound, Bound::Included(_)) == end; + + // handle "one past end" + let total_len = self.inner.raw.as_words().len(); + if to.0 == total_len { + return if !one_further { + Some(Some(total_len)) + } else { + None + }; + } + + // handle degenerate RangeInclusive + // `6..=5` needs to return an empty slice, but `6..=4` should fail as normal. Just that the offset is one + // dynamically sized instruction. The best we can do is reset to offset 0 and start iterating again. + // Expensive, but only happens in this degenerate case or when it's oob anyway. + if self.inner.offset > to { + self.inner.offset = InstOffset(0); + } + + let inst = self.advance_to(to)?; + let extra = if one_further { inst.len() } else { 0 }; + Some(Some(to.0 + extra)) + } } impl<'a, ISA: InstEncoding> Iterator for InstOffsetRefIter<'a, ISA> { @@ -140,16 +256,22 @@ impl<'a, ISA: InstEncoding> Iterator for InstOffsetRefIter<'a, ISA> { #[inline] fn next(&mut self) -> Option { - match self.inner.next()? { - Ok((offset, reader)) => Some(( - offset, - match InstRef::from_words_unchecked(reader.to_words()) { - Ok(e) => e, - Err(e) => decode_failed(e), - }, - )), - Err(e) => decode_failed(e), + Some(reader_to_inst_ref_offset(self.inner.next()?)) + } +} + +fn reader_to_inst_ref_offset( + reader: Result<(InstOffset, InstReader<'_>), DecodeError>, +) -> (InstOffset, InstRef<'_, ISA>) { + match reader { + Ok((offset, reader)) => { + let inst_ref = match InstRef::from_words_unchecked(reader.to_words()) { + Ok(e) => e, + Err(e) => decode_failed(e), + }; + (offset, inst_ref) } + Err(e) => decode_failed(e), } } @@ -179,6 +301,16 @@ impl<'a, ISA: InstEncoding> InstRefIter<'a, ISA> { pub const fn new(slice: &'a InstSlice) -> Self { Self(InstOffsetRefIter::new(slice)) } + + /// Add [`InstOffset`]s to this iterator, akin to `enumerate` + #[inline] + pub const fn with_offsets(self) -> InstOffsetRefIter<'a, ISA> { + self.0 + } + + pub fn peek(&self) -> Option> { + Some(self.0.peek()?.1) + } } impl<'a, ISA: InstEncoding> Iterator for InstRefIter<'a, ISA> { @@ -293,6 +425,12 @@ impl RawInstSlice { unsafe { core::mem::transmute(words) } } + /// An [`InstSlice`] with 0 instructions + #[inline] + pub const fn empty() -> &'static Self { + Self::from_words(&[]) + } + /// Returns the underlying slice of words pub const fn as_words(&self) -> &[Word] { &self.0 @@ -342,28 +480,20 @@ impl<'a> RawInstOffsetRefIter<'a> { offset: InstOffset(0), } } -} -impl<'a> Iterator for RawInstOffsetRefIter<'a> { - type Item = Result<(InstOffset, InstReader<'a>), DecodeError>; + pub fn offset(&self) -> InstOffset { + self.offset + } - #[inline] - fn next(&mut self) -> Option { - let old_offset = self.offset; - if let Some(words) = self.raw.0.get(*old_offset..) { + pub fn peek(&self) -> Option), DecodeError>> { + if let Some(words) = self.raw.0.get(*self.offset..) { match InstReader::from_words(words) { - Ok(inst_reader) => { - *self.offset += inst_reader.len(); - Some(Ok((old_offset, inst_reader))) - } + Ok(inst_reader) => Some(Ok((self.offset, inst_reader))), Err(DecodeError { kind: DecodeErrorKind::OutOfInstructions, .. }) => None, - Err(e) => { - self.offset = InstOffset(!0); - Some(Err(e.with_inst_offset(old_offset))) - } + Err(e) => Some(Err(e.with_inst_offset(self.offset))), } } else { None @@ -371,6 +501,25 @@ impl<'a> Iterator for RawInstOffsetRefIter<'a> { } } +impl<'a> Iterator for RawInstOffsetRefIter<'a> { + type Item = Result<(InstOffset, InstReader<'a>), DecodeError>; + + #[inline] + fn next(&mut self) -> Option { + let out = self.peek(); + match out { + Some(Ok((_, inst_reader))) => { + *self.offset += inst_reader.len(); + } + Some(Err(_)) => { + self.offset = InstOffset(!0); + } + None => (), + } + out + } +} + /// An [`Iterator`] of [`Result`] yielding either an [`InstReader`] or a [`DecodeError`]. /// /// Use [`Self::with_offsets`] to also get [`InstOffset`] of the instruction. @@ -388,6 +537,10 @@ impl<'a> RawInstRefIter<'a> { pub const fn with_offsets(self) -> RawInstOffsetRefIter<'a> { self.0 } + + pub fn peek(&self) -> Option, DecodeError>> { + remove_offset_raw(self.0.peek()) + } } impl<'a> Iterator for RawInstRefIter<'a> { @@ -395,11 +548,17 @@ impl<'a> Iterator for RawInstRefIter<'a> { #[inline] fn next(&mut self) -> Option { - match self.0.next() { - Some(Ok((_, inst))) => Some(Ok(inst)), - Some(Err(e)) => Some(Err(e)), - None => None, - } + remove_offset_raw(self.0.next()) + } +} + +fn remove_offset_raw( + value: Option>, +) -> Option> { + match value { + Some(Ok((_, inst))) => Some(Ok(inst)), + Some(Err(e)) => Some(Err(e)), + None => None, } } diff --git a/tests/tests/dis_invalid_inst.rs b/tests/tests/dis_invalid_inst.rs index 3ec58499..e769c70f 100644 --- a/tests/tests/dis_invalid_inst.rs +++ b/tests/tests/dis_invalid_inst.rs @@ -39,7 +39,7 @@ fn op_name_str_offset(words: &[Word]) -> usize { words .iter() .enumerate() - .find(|(_, w)| **w == Word::from_le_bytes([b'a', b'b', b'c', b'd'])) + .find(|(_, w)| **w == Word::from_le_bytes(*b"abcd")) .unwrap() .0 } @@ -61,7 +61,7 @@ fn test_valid_dis() { fn test_no_null_term() { let mut words = record_inst().into_vec(); let str_offset = op_name_str_offset(&words); - words[str_offset + 1] = Word::from_le_bytes([b'e', b'f', b'g', b'h']); + words[str_offset + 1] = Word::from_le_bytes(*b"efgh"); expect![[r#" %42 = OpTypeInt 32 0 Error: String is not null-terminated. diff --git a/tests/tests/slicing.rs b/tests/tests/slicing.rs new file mode 100644 index 00000000..4506d935 --- /dev/null +++ b/tests/tests/slicing.rs @@ -0,0 +1,197 @@ +use rspirv2::core::inst::{ + OpAccessChain, OpConstant, OpConvertUToF, OpFAdd, OpStore, OpTypeFloat, OpTypeInt, + OpTypePointer, OpVariable, +}; +use rspirv2::core::inst_set::CoreInstSet; +use rspirv2::core::operands::StorageClass; +use rspirv2::core::preamble::MemoryAccess; +use rspirv2_types::Word; +use rspirv2_types::binary::{IdResultAlloc, InstOffset}; +use rspirv2_types::operand::{IdRef, IdResultType, LiteralConst, LiteralInteger}; +use rspirv2_types::vec::InstVec; +use smallvec::SmallVec; +use std::assert_matches; +use std::fmt::Debug; +use std::ops::{Range, RangeBounds}; + +fn demo_inst() -> InstVec { + let mut alloc = IdResultAlloc::new(); + let mut vec = InstVec::new(); + + // types + let u32 = vec.push_inst(OpTypeInt { + id_result: alloc.alloc_id(), + width: LiteralInteger::new(32), + signedness: LiteralInteger::new(0), + }); + let f32 = vec.push_inst(OpTypeFloat { + id_result: alloc.alloc_id(), + width: LiteralInteger::new(32), + floating_point_encoding: None, + }); + let f32_ptr_output = vec.push_inst(OpTypePointer { + id_result: alloc.alloc_id(), + storage_class: StorageClass::Output, + ty: IdRef(f32), + }); + + // const + let u32_0 = vec.push_inst(OpConstant { + id_result_type: IdResultType(u32), + id_result: alloc.alloc_id(), + value: LiteralConst::from(0u32), + }); + let u32_42 = vec.push_inst(OpConstant { + id_result_type: IdResultType(u32), + id_result: alloc.alloc_id(), + value: LiteralConst::from(42u32), + }); + let f32_123_45 = vec.push_inst(OpConstant { + id_result_type: IdResultType(u32), + id_result: alloc.alloc_id(), + value: LiteralConst::from(123.45f32), + }); + + // computation + // a = 42u32 as f32; + // b = 123.45f32 + a; + let a = vec.push_inst(OpConvertUToF { + id_result_type: IdResultType(f32), + id_result: alloc.alloc_id(), + unsigned_value: IdRef(u32_42), + }); + let b = vec.push_inst(OpFAdd { + id_result_type: IdResultType(f32), + id_result: alloc.alloc_id(), + operand_1: IdRef(f32_123_45), + operand_2: IdRef(a), + }); + + // output + let var_out = vec.push_inst(OpVariable { + id_result_type: IdResultType(f32), + id_result: alloc.alloc_id(), + storage_class: StorageClass::Output, + initializer: None, + }); + let ptr_var_out = vec.push_inst(OpAccessChain { + id_result_type: IdResultType(f32_ptr_output), + id_result: alloc.alloc_id(), + base: IdRef(var_out), + indexes: SmallVec::from_iter([IdRef(u32_0)]), + }); + vec.push_inst(OpStore { + pointer: IdRef(ptr_var_out), + object: IdRef(b), + memory_access: Some({ + let mut access = MemoryAccess::new(); + access.set_aligned(Some(LiteralInteger::new(4))); + access + }), + }); + vec +} + +#[test] +fn test_inst_offset_indexing() { + let vec = demo_inst(); + for (off, inst) in vec.iter_ref().with_offsets() { + let inst2 = vec.index_ref(off); + assert_eq!(inst.get(), inst2.get()); + } +} + +#[test] +#[expect(clippy::reversed_empty_ranges)] +fn test_slicing() { + fn test + Debug>(range: R, expected: Option>) { + println!("{range:?} - {expected:?}"); + let vec = demo_inst(); + let slice = vec.slice(( + range.start_bound().map(|a| InstOffset(*a)), + range.end_bound().map(|a| InstOffset(*a)), + )); + let actual = slice.map(|slice| { + let start = (slice.as_words().as_ptr() as usize - vec.as_words().as_ptr() as usize) + / size_of::(); + let len = slice.as_words().len(); + start..(start + len) + }); + assert_eq!(actual, expected); + } + + assert_eq!(demo_inst().as_words().len(), 46); + test(.., Some(0..46)); + + // RangeFrom + test(0.., Some(0..46)); + test(1.., None); + test(2.., None); + test(3.., None); + test(4.., Some(4..46)); + test(5.., None); + test(6.., None); + test(7.., Some(7..46)); + test(11.., Some(11..46)); + + // RangeTo + test(..46, Some(0..46)); + test(..45, None); + // Note: 0..44 is *almost* a valid instruction stream. The last 2 words of the last instruction are optional, + // but the op word encodes an instruction of length 5 instead of 3, so it is rejected when sliced manually. + // We must reject that sort of slicing here too. + test(..44, None); + test(..43, None); + test(..42, None); + test(..41, Some(0..41)); + + // Range + test(0..46, Some(0..46)); + test(0..1, None); + test(0..2, None); + test(0..3, None); + test(0..4, Some(0..4)); + test(1..4, None); + test(2..4, None); + test(3..4, None); + test(0..7, Some(0..7)); + test(4..7, Some(4..7)); + test(41..46, Some(41..46)); + + // RangeInclusive & RangeToInclusive + test(..=0, Some(0..4)); + test(0..=0, Some(0..4)); + test(0..=1, None); + test(..=4, Some(0..7)); + test(0..=4, Some(0..7)); + test(4..=4, Some(4..7)); + test(..=7, Some(0..11)); + test(0..=7, Some(0..11)); + test(4..=7, Some(4..11)); + test(7..=7, Some(7..11)); + test(..=41, Some(0..46)); + test(41..=41, Some(41..46)); + test(..=46, None); + test(41..=46, None); + test(46..=46, None); + + // zero length but valid + test(0..0, Some(0..0)); + test(1..1, None); + test(4..4, Some(4..4)); + test(7..7, Some(7..7)); + test(41..41, Some(41..41)); + assert_matches!(demo_inst().as_words().get(46..46), Some(_)); + test(46..46, Some(46..46)); + + // oob + assert_matches!(demo_inst().as_words().get(47..47), None); + test(47..47, None); + + // zero-sized degenerate RangeInclusive + assert_matches!(demo_inst().as_words().get(6..=5), Some(_)); + assert_matches!(demo_inst().as_words().get(6..=4), None); + test(1..=0, None); + test(7..=4, Some(7..7)); + test(11..=7, Some(11..11)); +}