From 75e793d468eebf8b732e366ee214ad4bb9b6a45c Mon Sep 17 00:00:00 2001 From: atlas dostal Date: Wed, 23 Oct 2024 18:08:59 -0400 Subject: [PATCH 1/3] Add naga variants and stubs for r64uint --- naga/src/back/dot/mod.rs | 13 ++++++++ naga/src/back/glsl/mod.rs | 12 ++++++++ naga/src/back/hlsl/writer.rs | 9 ++++++ naga/src/back/msl/writer.rs | 11 +++++++ naga/src/back/pipeline_constants.rs | 12 ++++++++ naga/src/back/spv/block.rs | 11 +++++++ naga/src/back/wgsl/writer.rs | 9 ++++++ naga/src/compact/statements.rs | 24 +++++++++++++++ naga/src/front/spv/mod.rs | 1 + naga/src/lib.rs | 48 +++++++++++++++++++++++++++++ naga/src/proc/terminator.rs | 1 + naga/src/valid/analyzer.rs | 13 ++++++++ naga/src/valid/expression.rs | 4 +++ naga/src/valid/function.rs | 16 ++++++++++ naga/src/valid/handles.rs | 13 ++++++++ 15 files changed, 197 insertions(+) diff --git a/naga/src/back/dot/mod.rs b/naga/src/back/dot/mod.rs index 2780879657..6733ac115c 100644 --- a/naga/src/back/dot/mod.rs +++ b/naga/src/back/dot/mod.rs @@ -254,6 +254,19 @@ impl StatementGraph { } "Atomic" } + S::ImageAtomic { + image, + coordinate, + sample, + fun: _, + value, + } => { + self.dependencies.push((id, image, "image")); + self.dependencies.push((id, coordinate, "coordinate")); + self.dependencies.push((id, sample, "sample")); + self.dependencies.push((id, value, "value")); + "ImageAtomic" + } S::WorkGroupUniformLoad { pointer, result } => { self.emits.push((id, result)); self.dependencies.push((id, pointer, "pointer")); diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index de4a31b74c..ea0a556588 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -2469,6 +2469,18 @@ impl<'a, W: Write> Writer<'a, W> { self.write_expr(value, ctx)?; writeln!(self.out, ");")?; } + // Stores a value into an image. + Statement::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + return Err(Error::Custom( + "Image atomics are not implemented".to_string(), + )); + } Statement::RayQuery { .. } => unreachable!(), Statement::SubgroupBallot { result, predicate } => { write!(self.out, "{level}")?; diff --git a/naga/src/back/hlsl/writer.rs b/naga/src/back/hlsl/writer.rs index 3f2755878a..897aa90cac 100644 --- a/naga/src/back/hlsl/writer.rs +++ b/naga/src/back/hlsl/writer.rs @@ -2209,6 +2209,15 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> { writeln!(self.out, ");")?; } + crate::Statement::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + return Err(Error::Unimplemented("image atomics".to_string())); + } Statement::WorkGroupUniformLoad { pointer, result } => { self.write_barrier(crate::Barrier::WORK_GROUP, level)?; write!(self.out, "{level}")?; diff --git a/naga/src/back/msl/writer.rs b/naga/src/back/msl/writer.rs index 83d937d486..b127a63ae3 100644 --- a/naga/src/back/msl/writer.rs +++ b/naga/src/back/msl/writer.rs @@ -3219,6 +3219,17 @@ impl Writer { // Done writeln!(self.out, ";")?; } + crate::Statement::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + return Err(Error::UnsupportedCall( + "Image atomics are not supported".into(), + )); + } crate::Statement::WorkGroupUniformLoad { pointer, result } => { self.write_barrier(crate::Barrier::WORK_GROUP, level)?; diff --git a/naga/src/back/pipeline_constants.rs b/naga/src/back/pipeline_constants.rs index 0005cbcb0e..85499919b8 100644 --- a/naga/src/back/pipeline_constants.rs +++ b/naga/src/back/pipeline_constants.rs @@ -638,6 +638,18 @@ fn adjust_stmt(new_pos: &HandleVec>, stmt: &mut S | crate::AtomicFunction::Exchange { compare: None } => {} } } + Statement::ImageAtomic { + ref mut image, + ref mut coordinate, + ref mut sample, + fun: _, + ref mut value, + } => { + adjust(image); + adjust(coordinate); + adjust(sample); + adjust(value); + } Statement::WorkGroupUniformLoad { ref mut pointer, ref mut result, diff --git a/naga/src/back/spv/block.rs b/naga/src/back/spv/block.rs index 65c48d9e73..e827aaa946 100644 --- a/naga/src/back/spv/block.rs +++ b/naga/src/back/spv/block.rs @@ -2870,6 +2870,17 @@ impl<'w> BlockContext<'w> { block.body.push(instruction); } + Statement::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + return Err(Error::FeatureNotImplemented( + "Image atomics are not supported", + )); + } Statement::WorkGroupUniformLoad { pointer, result } => { self.writer .write_barrier(crate::Barrier::WORK_GROUP, &mut block); diff --git a/naga/src/back/wgsl/writer.rs b/naga/src/back/wgsl/writer.rs index 171102f241..5799739890 100644 --- a/naga/src/back/wgsl/writer.rs +++ b/naga/src/back/wgsl/writer.rs @@ -784,6 +784,15 @@ impl Writer { self.write_expr(module, value, func_ctx)?; writeln!(self.out, ");")? } + Statement::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + unimplemented!("Image atomics are not yet supported"); + } Statement::WorkGroupUniformLoad { pointer, result } => { write!(self.out, "{level}")?; // TODO: Obey named expressions here. diff --git a/naga/src/compact/statements.rs b/naga/src/compact/statements.rs index 759dcc2eda..07ab70d456 100644 --- a/naga/src/compact/statements.rs +++ b/naga/src/compact/statements.rs @@ -79,6 +79,18 @@ impl FunctionTracer<'_> { self.expressions_used.insert(result); } } + St::ImageAtomic { + image, + coordinate, + sample, + fun: _, + value, + } => { + self.expressions_used.insert(image); + self.expressions_used.insert(coordinate); + self.expressions_used.insert(sample); + self.expressions_used.insert(value); + } St::WorkGroupUniformLoad { pointer, result } => { self.expressions_used.insert(pointer); self.expressions_used.insert(result); @@ -261,6 +273,18 @@ impl FunctionMap { adjust(result); } } + St::ImageAtomic { + ref mut image, + ref mut coordinate, + ref mut sample, + fun: _, + ref mut value, + } => { + adjust(image); + adjust(coordinate); + adjust(sample); + adjust(value); + } St::WorkGroupUniformLoad { ref mut pointer, ref mut result, diff --git a/naga/src/front/spv/mod.rs b/naga/src/front/spv/mod.rs index 5ad063a6b6..2e5c918d30 100644 --- a/naga/src/front/spv/mod.rs +++ b/naga/src/front/spv/mod.rs @@ -4377,6 +4377,7 @@ impl> Frontend { | S::Store { .. } | S::ImageStore { .. } | S::Atomic { .. } + | S::ImageAtomic { .. } | S::RayQuery { .. } | S::SubgroupBallot { .. } | S::SubgroupCollectiveOperation { .. } diff --git a/naga/src/lib.rs b/naga/src/lib.rs index 145b95f667..51b198945d 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -1988,6 +1988,54 @@ pub enum Statement { /// [`SHADER_INT64_ATOMIC_ALL_OPS`]: crate::valid::Capabilities::SHADER_INT64_ATOMIC_ALL_OPS result: Option>, }, + /// Performs an atomic operation on a texel value of an image. + /// + /// Doing atomics on images with mipmaps is not supported, so there is no + /// `level` operand. + /// + /// This statement is a barrier for any operations on the corresponding + /// [`Expression::GlobalVariable`] for this image. + ImageAtomic { + /// The image to perform an atomic operation on. This must have type + /// [`Image`]. (This will necessarily be a [`GlobalVariable`] or + /// [`FunctionArgument`] expression, since no other expressions are + /// allowed to have that type.) + /// + /// [`Image`]: TypeInner::Image + /// [`GlobalVariable`]: Expression::GlobalVariable + /// [`FunctionArgument`]: Expression::FunctionArgument + image: Handle, + + /// The coordinate of the texel we wish to load. This must be a scalar + /// for [`D1`] images, a [`Bi`] vector for [`D2`] images, and a [`Tri`] + /// vector for [`D3`] images. (sample indices are supplied separately.) + /// Its component type must be [`Sint`]. + /// + /// If this image is arrayed, [`D1`] images require a [`Bi`] vector and + /// [`D2`] images require a [`Tri`] vector. + /// + /// Explicit level-of-detail values are unsupported. + /// + /// [`D1`]: ImageDimension::D1 + /// [`D2`]: ImageDimension::D2 + /// [`D3`]: ImageDimension::D3 + /// [`Bi`]: VectorSize::Bi + /// [`Tri`]: VectorSize::Tri + /// [`Sint`]: ScalarKind::Sint + coordinate: Handle, + + /// A sample index, for multisampled [`Sampled`] and [`Depth`] images. + /// + /// [`Sampled`]: ImageClass::Sampled + /// [`Depth`]: ImageClass::Depth + sample: Handle, + + /// The kind of atomic operation to perform on the texel. + fun: AtomicFunction, + + // The value with which to perform the atomic operation. + value: Handle, + }, /// Load uniformly from a uniform pointer in the workgroup address space. /// /// Corresponds to the [`workgroupUniformLoad`](https://www.w3.org/TR/WGSL/#workgroupUniformLoad-builtin) diff --git a/naga/src/proc/terminator.rs b/naga/src/proc/terminator.rs index 5edf55cb73..19c37294ec 100644 --- a/naga/src/proc/terminator.rs +++ b/naga/src/proc/terminator.rs @@ -36,6 +36,7 @@ pub fn ensure_block_returns(block: &mut crate::Block) { | S::Call { .. } | S::RayQuery { .. } | S::Atomic { .. } + | S::ImageAtomic { .. } | S::WorkGroupUniformLoad { .. } | S::SubgroupBallot { .. } | S::SubgroupCollectiveOperation { .. } diff --git a/naga/src/valid/analyzer.rs b/naga/src/valid/analyzer.rs index af95fd098f..b2d0ead8cf 100644 --- a/naga/src/valid/analyzer.rs +++ b/naga/src/valid/analyzer.rs @@ -1022,6 +1022,19 @@ impl FunctionInfo { } FunctionUniformity::new() } + S::ImageAtomic { + image, + coordinate, + sample, + fun: _, + value, + } => { + let _ = self.add_ref_impl(image, GlobalUse::WRITE); + let _ = self.add_ref(coordinate); + let _ = self.add_ref(sample); + let _ = self.add_ref(value); + FunctionUniformity::new() + } S::RayQuery { query, ref fun } => { let _ = self.add_ref(query); if let crate::RayQueryFunction::Initialize { diff --git a/naga/src/valid/expression.rs b/naga/src/valid/expression.rs index ccdc501a5c..672e279ced 100644 --- a/naga/src/valid/expression.rs +++ b/naga/src/valid/expression.rs @@ -65,6 +65,10 @@ pub enum ExpressionError { ExpectedSamplerType(Handle), #[error("Unable to operate on image class {0:?}")] InvalidImageClass(crate::ImageClass), + #[error("Image atomics are not supported for storage format {0:?}")] + InvalidImageFormat(crate::StorageFormat), + #[error("Image atomics require read/write storage access, {0:?} is insufficient")] + InvalidImageStorageAccess(crate::StorageAccess), #[error("Derivatives can only be taken from scalar and vector floats")] InvalidDerivative, #[error("Image array index parameter is misplaced")] diff --git a/naga/src/valid/function.rs b/naga/src/valid/function.rs index c695d65144..4184495db0 100644 --- a/naga/src/valid/function.rs +++ b/naga/src/valid/function.rs @@ -146,6 +146,10 @@ pub enum FunctionError { }, #[error("Image store parameters are invalid")] InvalidImageStore(#[source] ExpressionError), + #[error("Image atomic parameters are invalid")] + InvalidImageAtomic(#[source] ExpressionError), + #[error("Image atomic value is invalid")] + InvalidAtomicValue(Handle), #[error("Call to {function:?} is invalid")] InvalidCall { function: Handle, @@ -1136,6 +1140,18 @@ impl super::Validator { } => { self.validate_atomic(pointer, fun, value, result, span, context)?; } + S::ImageAtomic { + image: _, + coordinate: _, + sample: _, + fun: _, + value: _, + } => { + return Err(FunctionError::InvalidAtomic(AtomicError::MissingCapability( + super::Capabilities::SHADER_INT64_ATOMIC_MIN_MAX, + )) + .with_span_static(span, "Image atomics are not implemented yet")) + } S::WorkGroupUniformLoad { pointer, result } => { stages &= super::ShaderStages::COMPUTE; let pointer_inner = diff --git a/naga/src/valid/handles.rs b/naga/src/valid/handles.rs index f8be76d026..6420d36057 100644 --- a/naga/src/valid/handles.rs +++ b/naga/src/valid/handles.rs @@ -536,6 +536,19 @@ impl super::Validator { } Ok(()) } + crate::Statement::ImageAtomic { + image, + coordinate, + sample, + fun: _, + value, + } => { + validate_expr(image)?; + validate_expr(coordinate)?; + validate_expr(sample)?; + validate_expr(value)?; + Ok(()) + } crate::Statement::WorkGroupUniformLoad { pointer, result } => { validate_expr(pointer)?; validate_expr(result)?; From 00af7af89cf701659f0d98a2f14a626cce519b3b Mon Sep 17 00:00:00 2001 From: atlas dostal Date: Wed, 23 Oct 2024 18:15:25 -0400 Subject: [PATCH 2/3] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abf7044c23..04ec8ff7c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216). - Add base support for parsing `requires`, `enable`, and `diagnostic` directives. No extensions or diagnostic filters are yet supported, but diagnostics have improved dramatically. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437). - Include error chain information as a message and notes in shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436). - Unify Naga CLI error output with the format of shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436). +- Add ImageAtomic enum variants and stubs. By @atlv24 in [#6459](https://github.com/gfx-rs/wgpu/pull/6459). #### General From a70577fe9f7c34974f68274f785ae465e76a11c9 Mon Sep 17 00:00:00 2001 From: atlas dostal Date: Thu, 24 Oct 2024 12:40:47 -0400 Subject: [PATCH 3/3] remove panic --- naga/src/back/wgsl/writer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/naga/src/back/wgsl/writer.rs b/naga/src/back/wgsl/writer.rs index 5799739890..27e2d92784 100644 --- a/naga/src/back/wgsl/writer.rs +++ b/naga/src/back/wgsl/writer.rs @@ -791,7 +791,9 @@ impl Writer { fun: _, value: _, } => { - unimplemented!("Image atomics are not yet supported"); + return Err(Error::Unimplemented( + "Image atomics are not yet supported".into(), + )); } Statement::WorkGroupUniformLoad { pointer, result } => { write!(self.out, "{level}")?;