From 40351e4e811699fbed5d0a99c55d9b6fe024f01b Mon Sep 17 00:00:00 2001 From: Firestar99 <4696087-firestar99@users.noreply.gitlab.com> Date: Mon, 23 Sep 2024 13:37:15 +0200 Subject: [PATCH 1/7] ByteAddressableBuffer: split up into read-only and mutable variants --- .../spirv-std/src/byte_addressable_buffer.rs | 114 +++++++++++++----- crates/spirv-std/src/lib.rs | 5 +- 2 files changed, 87 insertions(+), 32 deletions(-) diff --git a/crates/spirv-std/src/byte_addressable_buffer.rs b/crates/spirv-std/src/byte_addressable_buffer.rs index f22cfe7ff9..f9442d475b 100644 --- a/crates/spirv-std/src/byte_addressable_buffer.rs +++ b/crates/spirv-std/src/byte_addressable_buffer.rs @@ -43,59 +43,114 @@ unsafe fn buffer_store_intrinsic( .write(value); } -/// `ByteAddressableBuffer` is an untyped blob of data, allowing loads and stores of arbitrary -/// basic data types at arbitrary indices. However, all data must be aligned to size 4, each -/// element within the data (e.g. struct fields) must have a size and alignment of a multiple of 4, -/// and the `byte_index` passed to load and store must be a multiple of 4 (`byte_index` will be -/// rounded down to the nearest multiple of 4). So, it's not technically a *byte* addressable -/// buffer, but rather a *word* buffer, but this naming and behavior was inherited from HLSL (where -/// it's UB to pass in an index not a multiple of 4). +/// `ByteAddressableBuffer` is a read-only untyped blob of data, allowing loads +/// of arbitrary basic data types at arbitrary indices. See +/// [`MutByteAddressableBuffer`] for a writeable variant. +/// +/// # Alignment +/// All data must be aligned to size 4, each element within the data (e.g. +/// struct fields) must have a size and alignment of a multiple of 4, and the +/// `byte_index` passed to load and store must be a multiple of 4. Technically +/// it's not a *byte* addressable buffer, but rather a *word* buffer, but this +/// naming and behavior was inherited from HLSL (where it's UB to pass in an +/// index not a multiple of 4). +/// +/// # Safety +/// Using these functions allows reading a different type from the buffer than +/// was originally written (by [`MutByteAddressableBuffer`] or the host API), +/// allowing all sorts of safety guarantees to be bypassed (effectively a +/// transmute). #[repr(transparent)] pub struct ByteAddressableBuffer<'a> { /// The underlying array of bytes, able to be directly accessed. - pub data: &'a mut [u32], + pub data: &'a [u32], } impl<'a> ByteAddressableBuffer<'a> { + /// Creates a `ByteAddressableBuffer` from the untyped blob of data. + #[inline] + pub fn new(data: &'a [u32]) -> Self { + Self { data } + } + + /// Loads an arbitrary type from the buffer. `byte_index` must be a + /// multiple of 4. + /// + /// # Safety + /// See [`Self`]. + pub unsafe fn load(&self, byte_index: u32) -> T { + if byte_index % 4 != 0 { + panic!("`byte_index` should be a multiple of 4"); + } + if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { + panic!( + "index out of bounds: the len is {} but the `byte_index` is {}", + self.data.len(), + byte_index + ); + } + buffer_load_intrinsic(self.data, byte_index) + } + + /// Loads an arbitrary type from the buffer. `byte_index` must be a + /// multiple of 4. + /// + /// # Safety + /// See [`Self`]. Additionally, bounds or alignment checking is not performed. + pub unsafe fn load_unchecked(&self, byte_index: u32) -> T { + buffer_load_intrinsic(self.data, byte_index) + } +} + +/// `MutByteAddressableBuffer` is a mutable untyped blob of data, allowing +/// loads and stores of arbitrary basic data types at arbitrary indices. See +/// [`ByteAddressableBuffer`] for details. +#[repr(transparent)] +pub struct MutByteAddressableBuffer<'a> { + /// The underlying array of bytes, able to be directly accessed. + pub data: &'a mut [u32], +} + +impl<'a> MutByteAddressableBuffer<'a> { /// Creates a `ByteAddressableBuffer` from the untyped blob of data. #[inline] pub fn new(data: &'a mut [u32]) -> Self { Self { data } } - /// Loads an arbitrary type from the buffer. `byte_index` must be a multiple of 4, otherwise, - /// it will get silently rounded down to the nearest multiple of 4. + /// Loads an arbitrary type from the buffer. `byte_index` must be a + /// multiple of 4. /// /// # Safety - /// This function allows writing a type to an untyped buffer, then reading a different type - /// from the same buffer, allowing all sorts of safety guarantees to be bypassed (effectively a - /// transmute) + /// See [`Self`]. pub unsafe fn load(&self, byte_index: u32) -> T { + if byte_index % 4 != 0 { + panic!("`byte_index` should be a multiple of 4"); + } if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { - panic!("Index out of range"); + panic!( + "index out of bounds: the len is {} but the `byte_index` is {}", + self.data.len(), + byte_index + ); } buffer_load_intrinsic(self.data, byte_index) } - /// Loads an arbitrary type from the buffer. `byte_index` must be a multiple of 4, otherwise, - /// it will get silently rounded down to the nearest multiple of 4. Bounds checking is not - /// performed. + /// Loads an arbitrary type from the buffer. `byte_index` must be a + /// multiple of 4. /// /// # Safety - /// This function allows writing a type to an untyped buffer, then reading a different type - /// from the same buffer, allowing all sorts of safety guarantees to be bypassed (effectively a - /// transmute). Additionally, bounds checking is not performed. + /// See [`Self`]. Additionally, bounds or alignment checking is not performed. pub unsafe fn load_unchecked(&self, byte_index: u32) -> T { buffer_load_intrinsic(self.data, byte_index) } - /// Stores an arbitrary type int the buffer. `byte_index` must be a multiple of 4, otherwise, - /// it will get silently rounded down to the nearest multiple of 4. + /// Stores an arbitrary type into the buffer. `byte_index` must be a + /// multiple of 4. /// /// # Safety - /// This function allows writing a type to an untyped buffer, then reading a different type - /// from the same buffer, allowing all sorts of safety guarantees to be bypassed (effectively a - /// transmute) + /// See [`Self`]. pub unsafe fn store(&mut self, byte_index: u32, value: T) { if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { panic!("Index out of range"); @@ -103,14 +158,11 @@ impl<'a> ByteAddressableBuffer<'a> { buffer_store_intrinsic(self.data, byte_index, value); } - /// Stores an arbitrary type int the buffer. `byte_index` must be a multiple of 4, otherwise, - /// it will get silently rounded down to the nearest multiple of 4. Bounds checking is not - /// performed. + /// Stores an arbitrary type into the buffer. `byte_index` must be a + /// multiple of 4. /// /// # Safety - /// This function allows writing a type to an untyped buffer, then reading a different type - /// from the same buffer, allowing all sorts of safety guarantees to be bypassed (effectively a - /// transmute). Additionally, bounds checking is not performed. + /// See [`Self`]. Additionally, bounds or alignment checking is not performed. pub unsafe fn store_unchecked(&mut self, byte_index: u32, value: T) { buffer_store_intrinsic(self.data, byte_index, value); } diff --git a/crates/spirv-std/src/lib.rs b/crates/spirv-std/src/lib.rs index 175099537a..c3b1106295 100644 --- a/crates/spirv-std/src/lib.rs +++ b/crates/spirv-std/src/lib.rs @@ -112,10 +112,13 @@ pub mod vector; pub use self::sampler::Sampler; pub use crate::macros::Image; -pub use byte_addressable_buffer::ByteAddressableBuffer; pub use num_traits; pub use runtime_array::*; pub use typed_buffer::*; +pub use { + byte_addressable_buffer::ByteAddressableBuffer, + byte_addressable_buffer::MutByteAddressableBuffer, +}; pub use glam; From f78287a1a447467ad8d5270330e2a21ec5b4c807 Mon Sep 17 00:00:00 2001 From: Firestar99 <4696087-firestar99@users.noreply.gitlab.com> Date: Mon, 23 Sep 2024 13:48:08 +0200 Subject: [PATCH 2/7] ByteAddressableBuffer: fixup tests --- .../ui/arch/debug_printf_type_checking.stderr | 20 +++++++++---------- tests/ui/byte_addressable_buffer/arr.rs | 4 ++-- .../ui/byte_addressable_buffer/big_struct.rs | 4 ++-- tests/ui/byte_addressable_buffer/complex.rs | 4 ++-- .../byte_addressable_buffer/empty_struct.rs | 4 ++-- tests/ui/byte_addressable_buffer/f32.rs | 4 ++-- .../byte_addressable_buffer/small_struct.rs | 4 ++-- tests/ui/byte_addressable_buffer/u32.rs | 4 ++-- tests/ui/byte_addressable_buffer/vec.rs | 4 ++-- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/ui/arch/debug_printf_type_checking.stderr b/tests/ui/arch/debug_printf_type_checking.stderr index b027cc7b34..dae06105ea 100644 --- a/tests/ui/arch/debug_printf_type_checking.stderr +++ b/tests/ui/arch/debug_printf_type_checking.stderr @@ -75,9 +75,9 @@ help: the return type of this call is `u32` due to the type of the argument pass | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:138:8 + --> $SPIRV_STD_SRC/lib.rs:141:8 | -138 | pub fn debug_printf_assert_is_type(ty: T) -> T { +141 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) help: change the type of the numeric literal from `u32` to `f32` @@ -102,9 +102,9 @@ help: the return type of this call is `f32` due to the type of the argument pass | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:138:8 + --> $SPIRV_STD_SRC/lib.rs:141:8 | -138 | pub fn debug_printf_assert_is_type(ty: T) -> T { +141 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) help: change the type of the numeric literal from `f32` to `u32` @@ -129,12 +129,12 @@ error[E0277]: the trait bound `{float}: Vector` is not satisfied > and 5 others note: required by a bound in `debug_printf_assert_is_vector` - --> $SPIRV_STD_SRC/lib.rs:145:8 + --> $SPIRV_STD_SRC/lib.rs:148:8 | -143 | pub fn debug_printf_assert_is_vector< +146 | pub fn debug_printf_assert_is_vector< | ----------------------------- required by a bound in this function -144 | TY: crate::scalar::Scalar, -145 | V: crate::vector::Vector, +147 | TY: crate::scalar::Scalar, +148 | V: crate::vector::Vector, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `debug_printf_assert_is_vector` = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -155,9 +155,9 @@ help: the return type of this call is `Vec2` due to the type of the argument pas | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:138:8 + --> $SPIRV_STD_SRC/lib.rs:141:8 | -138 | pub fn debug_printf_assert_is_type(ty: T) -> T { +141 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/byte_addressable_buffer/arr.rs b/tests/ui/byte_addressable_buffer/arr.rs index 4cacbcf906..c90f058c14 100644 --- a/tests/ui/byte_addressable_buffer/arr.rs +++ b/tests/ui/byte_addressable_buffer/arr.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec4, ByteAddressableBuffer}; +use spirv_std::{glam::Vec4, ByteAddressableBuffer, MutByteAddressableBuffer}; #[spirv(fragment)] pub fn load( @@ -20,7 +20,7 @@ pub fn store( #[spirv(flat)] val: [i32; 4], ) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/big_struct.rs b/tests/ui/byte_addressable_buffer/big_struct.rs index 27907afe3c..c2988c911d 100644 --- a/tests/ui/byte_addressable_buffer/big_struct.rs +++ b/tests/ui/byte_addressable_buffer/big_struct.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::ByteAddressableBuffer; +use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; pub struct BigStruct { a: u32, @@ -29,7 +29,7 @@ pub fn store( #[spirv(flat)] val: BigStruct, ) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/complex.rs b/tests/ui/byte_addressable_buffer/complex.rs index 1ec3267a2e..dd6e16331f 100644 --- a/tests/ui/byte_addressable_buffer/complex.rs +++ b/tests/ui/byte_addressable_buffer/complex.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec2, ByteAddressableBuffer}; +use spirv_std::{glam::Vec2, ByteAddressableBuffer, MutByteAddressableBuffer}; pub struct Complex { x: u32, @@ -35,7 +35,7 @@ pub fn store( val: Nesty, ) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/empty_struct.rs b/tests/ui/byte_addressable_buffer/empty_struct.rs index b586139d3a..d6d13e5300 100644 --- a/tests/ui/byte_addressable_buffer/empty_struct.rs +++ b/tests/ui/byte_addressable_buffer/empty_struct.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::ByteAddressableBuffer; +use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; pub struct EmptyStruct {} @@ -20,7 +20,7 @@ pub fn load( pub fn store(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32]) { let val = EmptyStruct {}; unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/f32.rs b/tests/ui/byte_addressable_buffer/f32.rs index 3dc5d3ffe3..a6cd7fe2e5 100644 --- a/tests/ui/byte_addressable_buffer/f32.rs +++ b/tests/ui/byte_addressable_buffer/f32.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::ByteAddressableBuffer; +use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; #[spirv(fragment)] pub fn load( @@ -17,7 +17,7 @@ pub fn load( #[spirv(fragment)] pub fn store(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], val: f32) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/small_struct.rs b/tests/ui/byte_addressable_buffer/small_struct.rs index d6959db768..96a43a80f4 100644 --- a/tests/ui/byte_addressable_buffer/small_struct.rs +++ b/tests/ui/byte_addressable_buffer/small_struct.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::ByteAddressableBuffer; +use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; pub struct SmallStruct { a: u32, @@ -27,7 +27,7 @@ pub fn store( ) { let val = SmallStruct { a, b }; unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/u32.rs b/tests/ui/byte_addressable_buffer/u32.rs index 8d5d03ad65..7bbd48d4ba 100644 --- a/tests/ui/byte_addressable_buffer/u32.rs +++ b/tests/ui/byte_addressable_buffer/u32.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::ByteAddressableBuffer; +use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; #[spirv(fragment)] pub fn load( @@ -20,7 +20,7 @@ pub fn store( #[spirv(flat)] val: u32, ) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/vec.rs b/tests/ui/byte_addressable_buffer/vec.rs index 49ecbc15c2..060c3aad63 100644 --- a/tests/ui/byte_addressable_buffer/vec.rs +++ b/tests/ui/byte_addressable_buffer/vec.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec4, ByteAddressableBuffer}; +use spirv_std::{glam::Vec4, ByteAddressableBuffer, MutByteAddressableBuffer}; #[spirv(matrix)] pub struct Mat4 { @@ -31,7 +31,7 @@ pub fn store( valmat: Mat4, ) { unsafe { - let mut buf = ByteAddressableBuffer::new(buf); + let mut buf = MutByteAddressableBuffer::new(buf); buf.store(5, val); buf.store(5, valmat); } From fa01a797ecb0d0b652827533bceac2ed4fe619ea Mon Sep 17 00:00:00 2001 From: Firestar99 <4696087-firestar99@users.noreply.gitlab.com> Date: Fri, 27 Sep 2024 14:05:17 +0200 Subject: [PATCH 3/7] ByteAddressableBuffer: remove mut variant, make ByteAddressableBuffer generic on mutability --- .../spirv-std/src/byte_addressable_buffer.rs | 30 +++++++------------ crates/spirv-std/src/lib.rs | 5 +--- .../ui/arch/debug_printf_type_checking.stderr | 20 ++++++------- tests/ui/byte_addressable_buffer/arr.rs | 17 +++++++++-- .../ui/byte_addressable_buffer/big_struct.rs | 17 +++++++++-- tests/ui/byte_addressable_buffer/complex.rs | 17 +++++++++-- .../byte_addressable_buffer/empty_struct.rs | 17 +++++++++-- tests/ui/byte_addressable_buffer/f32.rs | 16 +++++++--- .../byte_addressable_buffer/small_struct.rs | 17 +++++++++-- tests/ui/byte_addressable_buffer/u32.rs | 16 +++++++--- tests/ui/byte_addressable_buffer/vec.rs | 19 ++++++++++-- 11 files changed, 132 insertions(+), 59 deletions(-) diff --git a/crates/spirv-std/src/byte_addressable_buffer.rs b/crates/spirv-std/src/byte_addressable_buffer.rs index f9442d475b..c164e0fc40 100644 --- a/crates/spirv-std/src/byte_addressable_buffer.rs +++ b/crates/spirv-std/src/byte_addressable_buffer.rs @@ -43,15 +43,16 @@ unsafe fn buffer_store_intrinsic( .write(value); } -/// `ByteAddressableBuffer` is a read-only untyped blob of data, allowing loads -/// of arbitrary basic data types at arbitrary indices. See -/// [`MutByteAddressableBuffer`] for a writeable variant. +/// `ByteAddressableBuffer` is a view to an untyped blob of data, allowing +/// loads and stores of arbitrary basic data types at arbitrary indices. Use +/// `from_slice()` or `from_mut_slice()` to create the `ByteAddressableBuffer`, +/// with only the mutable slice allowing stores. /// /// # Alignment /// All data must be aligned to size 4, each element within the data (e.g. /// struct fields) must have a size and alignment of a multiple of 4, and the /// `byte_index` passed to load and store must be a multiple of 4. Technically -/// it's not a *byte* addressable buffer, but rather a *word* buffer, but this +/// it is not a *byte* addressable buffer, but rather a *word* buffer, but this /// naming and behavior was inherited from HLSL (where it's UB to pass in an /// index not a multiple of 4). /// @@ -61,15 +62,15 @@ unsafe fn buffer_store_intrinsic( /// allowing all sorts of safety guarantees to be bypassed (effectively a /// transmute). #[repr(transparent)] -pub struct ByteAddressableBuffer<'a> { +pub struct ByteAddressableBuffer { /// The underlying array of bytes, able to be directly accessed. - pub data: &'a [u32], + pub data: T, } -impl<'a> ByteAddressableBuffer<'a> { +impl<'a> ByteAddressableBuffer<&'a [u32]> { /// Creates a `ByteAddressableBuffer` from the untyped blob of data. #[inline] - pub fn new(data: &'a [u32]) -> Self { + pub fn from_slice(data: &'a [u32]) -> Self { Self { data } } @@ -102,19 +103,10 @@ impl<'a> ByteAddressableBuffer<'a> { } } -/// `MutByteAddressableBuffer` is a mutable untyped blob of data, allowing -/// loads and stores of arbitrary basic data types at arbitrary indices. See -/// [`ByteAddressableBuffer`] for details. -#[repr(transparent)] -pub struct MutByteAddressableBuffer<'a> { - /// The underlying array of bytes, able to be directly accessed. - pub data: &'a mut [u32], -} - -impl<'a> MutByteAddressableBuffer<'a> { +impl<'a> ByteAddressableBuffer<&'a mut [u32]> { /// Creates a `ByteAddressableBuffer` from the untyped blob of data. #[inline] - pub fn new(data: &'a mut [u32]) -> Self { + pub fn from_mut_slice(data: &'a mut [u32]) -> Self { Self { data } } diff --git a/crates/spirv-std/src/lib.rs b/crates/spirv-std/src/lib.rs index c3b1106295..175099537a 100644 --- a/crates/spirv-std/src/lib.rs +++ b/crates/spirv-std/src/lib.rs @@ -112,13 +112,10 @@ pub mod vector; pub use self::sampler::Sampler; pub use crate::macros::Image; +pub use byte_addressable_buffer::ByteAddressableBuffer; pub use num_traits; pub use runtime_array::*; pub use typed_buffer::*; -pub use { - byte_addressable_buffer::ByteAddressableBuffer, - byte_addressable_buffer::MutByteAddressableBuffer, -}; pub use glam; diff --git a/tests/ui/arch/debug_printf_type_checking.stderr b/tests/ui/arch/debug_printf_type_checking.stderr index dae06105ea..b027cc7b34 100644 --- a/tests/ui/arch/debug_printf_type_checking.stderr +++ b/tests/ui/arch/debug_printf_type_checking.stderr @@ -75,9 +75,9 @@ help: the return type of this call is `u32` due to the type of the argument pass | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:141:8 + --> $SPIRV_STD_SRC/lib.rs:138:8 | -141 | pub fn debug_printf_assert_is_type(ty: T) -> T { +138 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) help: change the type of the numeric literal from `u32` to `f32` @@ -102,9 +102,9 @@ help: the return type of this call is `f32` due to the type of the argument pass | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:141:8 + --> $SPIRV_STD_SRC/lib.rs:138:8 | -141 | pub fn debug_printf_assert_is_type(ty: T) -> T { +138 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) help: change the type of the numeric literal from `f32` to `u32` @@ -129,12 +129,12 @@ error[E0277]: the trait bound `{float}: Vector` is not satisfied > and 5 others note: required by a bound in `debug_printf_assert_is_vector` - --> $SPIRV_STD_SRC/lib.rs:148:8 + --> $SPIRV_STD_SRC/lib.rs:145:8 | -146 | pub fn debug_printf_assert_is_vector< +143 | pub fn debug_printf_assert_is_vector< | ----------------------------- required by a bound in this function -147 | TY: crate::scalar::Scalar, -148 | V: crate::vector::Vector, +144 | TY: crate::scalar::Scalar, +145 | V: crate::vector::Vector, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `debug_printf_assert_is_vector` = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -155,9 +155,9 @@ help: the return type of this call is `Vec2` due to the type of the argument pas | | | this argument influences the return type of `spirv_std` note: function defined here - --> $SPIRV_STD_SRC/lib.rs:141:8 + --> $SPIRV_STD_SRC/lib.rs:138:8 | -141 | pub fn debug_printf_assert_is_type(ty: T) -> T { +138 | pub fn debug_printf_assert_is_type(ty: T) -> T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `debug_printf` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/byte_addressable_buffer/arr.rs b/tests/ui/byte_addressable_buffer/arr.rs index c90f058c14..798c3cd515 100644 --- a/tests/ui/byte_addressable_buffer/arr.rs +++ b/tests/ui/byte_addressable_buffer/arr.rs @@ -1,15 +1,26 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec4, ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::{glam::Vec4, ByteAddressableBuffer}; #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut [i32; 4], +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut [i32; 4], ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -20,7 +31,7 @@ pub fn store( #[spirv(flat)] val: [i32; 4], ) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/big_struct.rs b/tests/ui/byte_addressable_buffer/big_struct.rs index c2988c911d..233cb87fae 100644 --- a/tests/ui/byte_addressable_buffer/big_struct.rs +++ b/tests/ui/byte_addressable_buffer/big_struct.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::ByteAddressableBuffer; pub struct BigStruct { a: u32, @@ -14,11 +14,22 @@ pub struct BigStruct { #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut BigStruct, +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut BigStruct, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -29,7 +40,7 @@ pub fn store( #[spirv(flat)] val: BigStruct, ) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/complex.rs b/tests/ui/byte_addressable_buffer/complex.rs index dd6e16331f..b9e3edf128 100644 --- a/tests/ui/byte_addressable_buffer/complex.rs +++ b/tests/ui/byte_addressable_buffer/complex.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec2, ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::{glam::Vec2, ByteAddressableBuffer}; pub struct Complex { x: u32, @@ -20,11 +20,22 @@ pub struct Nesty { #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut Nesty, +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut Nesty, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -35,7 +46,7 @@ pub fn store( val: Nesty, ) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/empty_struct.rs b/tests/ui/byte_addressable_buffer/empty_struct.rs index d6d13e5300..1425526112 100644 --- a/tests/ui/byte_addressable_buffer/empty_struct.rs +++ b/tests/ui/byte_addressable_buffer/empty_struct.rs @@ -1,17 +1,28 @@ // build-pass use spirv_std::spirv; -use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::ByteAddressableBuffer; pub struct EmptyStruct {} #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut EmptyStruct, +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut EmptyStruct, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -20,7 +31,7 @@ pub fn load( pub fn store(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32]) { let val = EmptyStruct {}; unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/f32.rs b/tests/ui/byte_addressable_buffer/f32.rs index a6cd7fe2e5..2b82f89d16 100644 --- a/tests/ui/byte_addressable_buffer/f32.rs +++ b/tests/ui/byte_addressable_buffer/f32.rs @@ -1,15 +1,23 @@ // build-pass use spirv_std::spirv; -use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::ByteAddressableBuffer; #[spirv(fragment)] -pub fn load( +pub fn load(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], out: &mut f32) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut f32, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -17,7 +25,7 @@ pub fn load( #[spirv(fragment)] pub fn store(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], val: f32) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/small_struct.rs b/tests/ui/byte_addressable_buffer/small_struct.rs index 96a43a80f4..948af2dd92 100644 --- a/tests/ui/byte_addressable_buffer/small_struct.rs +++ b/tests/ui/byte_addressable_buffer/small_struct.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::ByteAddressableBuffer; pub struct SmallStruct { a: u32, @@ -10,11 +10,22 @@ pub struct SmallStruct { #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut SmallStruct, +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut SmallStruct, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -27,7 +38,7 @@ pub fn store( ) { let val = SmallStruct { a, b }; unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/u32.rs b/tests/ui/byte_addressable_buffer/u32.rs index 7bbd48d4ba..d0e1e44624 100644 --- a/tests/ui/byte_addressable_buffer/u32.rs +++ b/tests/ui/byte_addressable_buffer/u32.rs @@ -1,15 +1,23 @@ // build-pass use spirv_std::spirv; -use spirv_std::{ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::ByteAddressableBuffer; #[spirv(fragment)] -pub fn load( +pub fn load(#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], out: &mut u32) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut u32, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); } } @@ -20,7 +28,7 @@ pub fn store( #[spirv(flat)] val: u32, ) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); } } diff --git a/tests/ui/byte_addressable_buffer/vec.rs b/tests/ui/byte_addressable_buffer/vec.rs index 060c3aad63..e934071b12 100644 --- a/tests/ui/byte_addressable_buffer/vec.rs +++ b/tests/ui/byte_addressable_buffer/vec.rs @@ -1,7 +1,7 @@ // build-pass use spirv_std::spirv; -use spirv_std::{glam::Vec4, ByteAddressableBuffer, MutByteAddressableBuffer}; +use spirv_std::{glam::Vec4, ByteAddressableBuffer}; #[spirv(matrix)] pub struct Mat4 { @@ -13,12 +13,25 @@ pub struct Mat4 { #[spirv(fragment)] pub fn load( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &[u32], + out: &mut Vec4, + outmat: &mut Mat4, +) { + unsafe { + let buf = ByteAddressableBuffer::from_slice(buf); + *out = buf.load(5); + *outmat = buf.load(5); + } +} + +#[spirv(fragment)] +pub fn load_mut( #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] buf: &mut [u32], out: &mut Vec4, outmat: &mut Mat4, ) { unsafe { - let buf = ByteAddressableBuffer::new(buf); + let buf = ByteAddressableBuffer::from_mut_slice(buf); *out = buf.load(5); *outmat = buf.load(5); } @@ -31,7 +44,7 @@ pub fn store( valmat: Mat4, ) { unsafe { - let mut buf = MutByteAddressableBuffer::new(buf); + let mut buf = ByteAddressableBuffer::from_mut_slice(buf); buf.store(5, val); buf.store(5, valmat); } From 6545b87db6b320018d1afd28b5671905b7ae6b6f Mon Sep 17 00:00:00 2001 From: Firestar99 <4696087-firestar99@users.noreply.gitlab.com> Date: Fri, 27 Sep 2024 15:21:31 +0200 Subject: [PATCH 4/7] ByteAddressableBuffer: unify bounds check in separate method --- .../spirv-std/src/byte_addressable_buffer.rs | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/crates/spirv-std/src/byte_addressable_buffer.rs b/crates/spirv-std/src/byte_addressable_buffer.rs index c164e0fc40..b9b613f5bd 100644 --- a/crates/spirv-std/src/byte_addressable_buffer.rs +++ b/crates/spirv-std/src/byte_addressable_buffer.rs @@ -67,6 +67,23 @@ pub struct ByteAddressableBuffer { pub data: T, } +fn bounds_check(data: &[u32], byte_index: u32) { + let sizeof = mem::size_of::() as u32; + if byte_index % 4 != 0 { + panic!("`byte_index` should be a multiple of 4"); + } + if byte_index + sizeof > data.len() as u32 { + let last_byte = byte_index + sizeof; + panic!( + "index out of bounds: the len is {} but loading {} bytes at `byte_index` {} reads until {} (exclusive)", + data.len(), + sizeof, + byte_index, + last_byte, + ); + } +} + impl<'a> ByteAddressableBuffer<&'a [u32]> { /// Creates a `ByteAddressableBuffer` from the untyped blob of data. #[inline] @@ -80,16 +97,7 @@ impl<'a> ByteAddressableBuffer<&'a [u32]> { /// # Safety /// See [`Self`]. pub unsafe fn load(&self, byte_index: u32) -> T { - if byte_index % 4 != 0 { - panic!("`byte_index` should be a multiple of 4"); - } - if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { - panic!( - "index out of bounds: the len is {} but the `byte_index` is {}", - self.data.len(), - byte_index - ); - } + bounds_check::(self.data, byte_index); buffer_load_intrinsic(self.data, byte_index) } @@ -116,16 +124,7 @@ impl<'a> ByteAddressableBuffer<&'a mut [u32]> { /// # Safety /// See [`Self`]. pub unsafe fn load(&self, byte_index: u32) -> T { - if byte_index % 4 != 0 { - panic!("`byte_index` should be a multiple of 4"); - } - if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { - panic!( - "index out of bounds: the len is {} but the `byte_index` is {}", - self.data.len(), - byte_index - ); - } + bounds_check::(self.data, byte_index); buffer_load_intrinsic(self.data, byte_index) } @@ -144,9 +143,7 @@ impl<'a> ByteAddressableBuffer<&'a mut [u32]> { /// # Safety /// See [`Self`]. pub unsafe fn store(&mut self, byte_index: u32, value: T) { - if byte_index + mem::size_of::() as u32 > self.data.len() as u32 { - panic!("Index out of range"); - } + bounds_check::(self.data, byte_index); buffer_store_intrinsic(self.data, byte_index, value); } From e2d0fdff29512aae8f49bd2092dfa9d7fc627faa Mon Sep 17 00:00:00 2001 From: Firestar99 <4696087-firestar99@users.noreply.gitlab.com> Date: Fri, 27 Sep 2024 15:31:28 +0200 Subject: [PATCH 5/7] ByteAddressableBuffer: fixup docs --- crates/spirv-std/src/byte_addressable_buffer.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/spirv-std/src/byte_addressable_buffer.rs b/crates/spirv-std/src/byte_addressable_buffer.rs index b9b613f5bd..4749b2b1b8 100644 --- a/crates/spirv-std/src/byte_addressable_buffer.rs +++ b/crates/spirv-std/src/byte_addressable_buffer.rs @@ -44,9 +44,7 @@ unsafe fn buffer_store_intrinsic( } /// `ByteAddressableBuffer` is a view to an untyped blob of data, allowing -/// loads and stores of arbitrary basic data types at arbitrary indices. Use -/// `from_slice()` or `from_mut_slice()` to create the `ByteAddressableBuffer`, -/// with only the mutable slice allowing stores. +/// loads and stores of arbitrary basic data types at arbitrary indices. /// /// # Alignment /// All data must be aligned to size 4, each element within the data (e.g. @@ -58,9 +56,9 @@ unsafe fn buffer_store_intrinsic( /// /// # Safety /// Using these functions allows reading a different type from the buffer than -/// was originally written (by [`MutByteAddressableBuffer`] or the host API), -/// allowing all sorts of safety guarantees to be bypassed (effectively a -/// transmute). +/// was originally written (by a previous `store()` or the host API), allowing +/// all sorts of safety guarantees to be bypassed, making it effectively a +/// transmute. #[repr(transparent)] pub struct ByteAddressableBuffer { /// The underlying array of bytes, able to be directly accessed. From 1e8b9afb9e6eb6e95c8ebd314d618a3b6750e844 Mon Sep 17 00:00:00 2001 From: Firestar99 <31222740+Firestar99@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:18:22 +0200 Subject: [PATCH 6/7] ByteAddressableBuffer: add as_ref() to convert mutable to read-only --- crates/spirv-std/src/byte_addressable_buffer.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/spirv-std/src/byte_addressable_buffer.rs b/crates/spirv-std/src/byte_addressable_buffer.rs index 4749b2b1b8..0640e54bda 100644 --- a/crates/spirv-std/src/byte_addressable_buffer.rs +++ b/crates/spirv-std/src/byte_addressable_buffer.rs @@ -116,14 +116,20 @@ impl<'a> ByteAddressableBuffer<&'a mut [u32]> { Self { data } } + /// Create a non-mutable `ByteAddressableBuffer` from this mutable one. + #[inline] + pub fn as_ref(&self) -> ByteAddressableBuffer<&[u32]> { + ByteAddressableBuffer { data: self.data } + } + /// Loads an arbitrary type from the buffer. `byte_index` must be a /// multiple of 4. /// /// # Safety /// See [`Self`]. + #[inline] pub unsafe fn load(&self, byte_index: u32) -> T { - bounds_check::(self.data, byte_index); - buffer_load_intrinsic(self.data, byte_index) + self.as_ref().load(byte_index) } /// Loads an arbitrary type from the buffer. `byte_index` must be a @@ -131,8 +137,9 @@ impl<'a> ByteAddressableBuffer<&'a mut [u32]> { /// /// # Safety /// See [`Self`]. Additionally, bounds or alignment checking is not performed. + #[inline] pub unsafe fn load_unchecked(&self, byte_index: u32) -> T { - buffer_load_intrinsic(self.data, byte_index) + self.as_ref().load_unchecked(byte_index) } /// Stores an arbitrary type into the buffer. `byte_index` must be a From 5bbcb316c93c33c1f14d15c3def67b376c763e59 Mon Sep 17 00:00:00 2001 From: Firestar99 <31222740+Firestar99@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:21:14 +0200 Subject: [PATCH 7/7] changelog: add ByteAddressableBuffer PR --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18fcf8e4d2..ffe2021988 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,10 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### 🚨BREAKING🚨 -- Signed for loops like `for _ in 0..4i32 {}` no longer compile. We recommend switching to unsigned for loops and casting back to signed integers in the meanwhile. - ### Changed 🛠 +- [PR#17](https://github.com/Rust-GPU/rust-gpu/pull/17) refactor ByteAddressableBuffer to allow reading from read-only buffers - [PR#14](https://github.com/Rust-GPU/rust-gpu/pull/14) add subgroup intrinsics matching glsl's [`GL_KHR_shader_subgroup`](https://github.com/KhronosGroup/GLSL/blob/main/extensions/khr/GL_KHR_shader_subgroup.txt) - [PR#13](https://github.com/Rust-GPU/rust-gpu/pull/13) allow cargo features to be passed to the shader crate - [PR#12](https://github.com/rust-gpu/rust-gpu/pull/12) updated toolchain to `nightly-2024-04-24`