From d07a97837df119f380c88b7eaa3c910596c7ec6c Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Wed, 2 Aug 2023 18:55:01 -0400 Subject: [PATCH] [spv-out] Fix non-uniformity attributes --- src/back/spv/block.rs | 136 +++++++++++++++------ src/back/spv/index.rs | 12 +- tests/out/spv/binding-arrays.spvasm | 44 +++---- tests/out/spv/binding-buffer-arrays.spvasm | 2 +- 4 files changed, 131 insertions(+), 63 deletions(-) diff --git a/src/back/spv/block.rs b/src/back/spv/block.rs index 74d8cdfef1..85bba495c0 100644 --- a/src/back/spv/block.rs +++ b/src/back/spv/block.rs @@ -25,7 +25,7 @@ fn get_dimension(type_inner: &crate::TypeInner) -> Dimension { enum ExpressionPointer { /// The pointer to the expression's value is available, as the value of the /// expression with the given id. - Ready { pointer_id: Word }, + Ready { pointer_id: Word, non_uniform: bool }, /// The access expression must be conditional on the value of `condition`, a boolean /// expression that is true if all indices are in bounds. If `condition` is true, then @@ -35,6 +35,7 @@ enum ExpressionPointer { Conditional { condition: Word, access: Instruction, + non_uniform: bool, }, } @@ -285,12 +286,15 @@ impl<'w> BlockContext<'w> { class: helpers::map_storage_class(space), }); - let result_id = match self.write_expression_pointer( + let (result_id, non_uniform) = match self.write_expression_pointer( expr_handle, block, Some(binding_array_false_pointer), )? { - ExpressionPointer::Ready { pointer_id } => pointer_id, + ExpressionPointer::Ready { + pointer_id, + non_uniform, + } => (pointer_id, non_uniform), ExpressionPointer::Conditional { .. } => { return Err(Error::FeatureNotImplemented( "Texture array out-of-bounds handling", @@ -308,6 +312,11 @@ impl<'w> BlockContext<'w> { None, )); + if non_uniform { + self.writer + .decorate_non_uniform_binding_array_access(load_id)?; + } + load_id } ref other => { @@ -361,12 +370,15 @@ impl<'w> BlockContext<'w> { class: helpers::map_storage_class(space), }); - let result_id = match self.write_expression_pointer( + let (result_id, non_uniform) = match self.write_expression_pointer( expr_handle, block, Some(binding_array_false_pointer), )? { - ExpressionPointer::Ready { pointer_id } => pointer_id, + ExpressionPointer::Ready { + pointer_id, + non_uniform, + } => (pointer_id, non_uniform), ExpressionPointer::Conditional { .. } => { return Err(Error::FeatureNotImplemented( "Texture array out-of-bounds handling", @@ -384,6 +396,11 @@ impl<'w> BlockContext<'w> { None, )); + if non_uniform { + self.writer + .decorate_non_uniform_binding_array_access(load_id)?; + } + load_id } ref other => { @@ -1052,7 +1069,10 @@ impl<'w> BlockContext<'w> { crate::Expression::LocalVariable(variable) => self.function.variables[&variable].id, crate::Expression::Load { pointer } => { match self.write_expression_pointer(pointer, block, None)? { - ExpressionPointer::Ready { pointer_id } => { + ExpressionPointer::Ready { + pointer_id, + non_uniform, + } => { let id = self.gen_id(); let atomic_space = match *self.fun_info[pointer].ty.inner_with(&self.ir_module.types) { @@ -1079,15 +1099,23 @@ impl<'w> BlockContext<'w> { Instruction::load(result_type_id, id, pointer_id, None) }; block.body.push(instruction); + if non_uniform { + self.writer.decorate_non_uniform_binding_array_access(id)?; + } id } - ExpressionPointer::Conditional { condition, access } => { + ExpressionPointer::Conditional { + condition, + access, + non_uniform, + } => { //TODO: support atomics? - self.write_conditional_indexed_load( + let mut non_uniform_instruction = None; + let res = self.write_conditional_indexed_load( result_type_id, condition, block, - move |id_gen, block| { + |id_gen, block| { // The in-bounds path. Perform the access and the load. let pointer_id = access.result_id.unwrap(); let value_id = id_gen.next(); @@ -1098,9 +1126,17 @@ impl<'w> BlockContext<'w> { pointer_id, None, )); - value_id + if non_uniform { + non_uniform_instruction = Some(value_id); + } + Ok(value_id) }, - ) + )?; + if let Some(value_id) = non_uniform_instruction { + self.writer + .decorate_non_uniform_binding_array_access(value_id)?; + } + res } } } @@ -1517,13 +1553,11 @@ impl<'w> BlockContext<'w> { } }; - let (pointer_id, expr_pointer) = if self.temp_list.is_empty() { - ( - root_id, - ExpressionPointer::Ready { - pointer_id: root_id, - }, - ) + let expr_pointer = if self.temp_list.is_empty() { + ExpressionPointer::Ready { + pointer_id: root_id, + non_uniform: is_non_uniform_binding_array, + } } else { self.temp_list.reverse(); let pointer_id = self.gen_id(); @@ -1534,19 +1568,21 @@ impl<'w> BlockContext<'w> { // caller to generate the branch, the access, the load or store, and // the zero value (for loads). Otherwise, we can emit the access // ourselves, and just hand them the id of the pointer. - let expr_pointer = match accumulated_checks { - Some(condition) => ExpressionPointer::Conditional { condition, access }, + match accumulated_checks { + Some(condition) => ExpressionPointer::Conditional { + condition, + access, + non_uniform: is_non_uniform_binding_array, + }, None => { block.body.push(access); - ExpressionPointer::Ready { pointer_id } + ExpressionPointer::Ready { + pointer_id, + non_uniform: is_non_uniform_binding_array, + } } - }; - (pointer_id, expr_pointer) + } }; - if is_non_uniform_binding_array { - self.writer - .decorate_non_uniform_binding_array_access(pointer_id)?; - } Ok(expr_pointer) } @@ -2003,7 +2039,10 @@ impl<'w> BlockContext<'w> { crate::Statement::Store { pointer, value } => { let value_id = self.cached[value]; match self.write_expression_pointer(pointer, &mut block, None)? { - ExpressionPointer::Ready { pointer_id } => { + ExpressionPointer::Ready { + pointer_id, + non_uniform, + } => { let atomic_space = match *self.fun_info[pointer] .ty .inner_with(&self.ir_module.types) @@ -2029,9 +2068,17 @@ impl<'w> BlockContext<'w> { } else { Instruction::store(pointer_id, value_id, None) }; + if non_uniform { + self.writer + .decorate_non_uniform_binding_array_access(pointer_id)?; + } block.body.push(instruction); } - ExpressionPointer::Conditional { condition, access } => { + ExpressionPointer::Conditional { + condition, + access, + non_uniform, + } => { let mut selection = Selection::start(&mut block, ()); selection.if_true(self, condition, ()); @@ -2042,6 +2089,10 @@ impl<'w> BlockContext<'w> { .block() .body .push(Instruction::store(pointer_id, value_id, None)); + if non_uniform { + self.writer + .decorate_non_uniform_binding_array_access(pointer_id)?; + } // Finish the in-bounds block and start the merge block. This // is the block we'll leave current on return. @@ -2094,7 +2145,16 @@ impl<'w> BlockContext<'w> { let pointer_id = match self.write_expression_pointer(pointer, &mut block, None)? { - ExpressionPointer::Ready { pointer_id } => pointer_id, + ExpressionPointer::Ready { + pointer_id, + non_uniform, + } => { + if non_uniform { + self.writer + .decorate_non_uniform_binding_array_access(pointer_id)?; + } + pointer_id + } ExpressionPointer::Conditional { .. } => { return Err(Error::FeatureNotImplemented( "Atomics out-of-bounds handling", @@ -2269,7 +2329,11 @@ impl<'w> BlockContext<'w> { let result_type_id = self.get_expression_type_id(&self.fun_info[result].ty); // Embed the body of match self.write_expression_pointer(pointer, &mut block, None)? { - ExpressionPointer::Ready { pointer_id } => { + // workgroup uniform loads can't ever be non-uniform + ExpressionPointer::Ready { + pointer_id, + non_uniform: _, + } => { let id = self.gen_id(); block.body.push(Instruction::load( result_type_id, @@ -2279,7 +2343,11 @@ impl<'w> BlockContext<'w> { )); self.cached[result] = id; } - ExpressionPointer::Conditional { condition, access } => { + ExpressionPointer::Conditional { + condition, + access, + non_uniform: _, + } => { self.cached[result] = self.write_conditional_indexed_load( result_type_id, condition, @@ -2295,9 +2363,9 @@ impl<'w> BlockContext<'w> { pointer_id, None, )); - value_id + Ok(value_id) }, - ) + )? } } self.writer diff --git a/src/back/spv/index.rs b/src/back/spv/index.rs index 92e0f88d9a..5a91a90098 100644 --- a/src/back/spv/index.rs +++ b/src/back/spv/index.rs @@ -299,9 +299,9 @@ impl<'w> BlockContext<'w> { condition: Word, block: &mut Block, emit_load: F, - ) -> Word + ) -> Result where - F: FnOnce(&mut IdGenerator, &mut Block) -> Word, + F: FnOnce(&mut IdGenerator, &mut Block) -> Result, { // For the out-of-bounds case, we produce a zero value. let null_id = self.writer.get_constant_null(result_type); @@ -324,9 +324,9 @@ impl<'w> BlockContext<'w> { selection.if_true(self, condition, null_id); // The in-bounds path. Perform the access and the load. - let loaded_value = emit_load(&mut self.writer.id_gen, selection.block()); + let loaded_value = emit_load(&mut self.writer.id_gen, selection.block())?; - selection.finish(self, loaded_value) + Ok(selection.finish(self, loaded_value)) } /// Emit code for bounds checks for an array, vector, or matrix access. @@ -410,9 +410,9 @@ impl<'w> BlockContext<'w> { base_id, index_id, )); - element_id + Ok(element_id) }, - ) + )? } }; diff --git a/tests/out/spv/binding-arrays.spvasm b/tests/out/spv/binding-arrays.spvasm index cd56fb4965..0901731005 100644 --- a/tests/out/spv/binding-arrays.spvasm +++ b/tests/out/spv/binding-arrays.spvasm @@ -35,28 +35,28 @@ OpMemberDecorate %47 0 Offset 0 OpDecorate %63 Location 0 OpDecorate %63 Flat OpDecorate %66 Location 0 -OpDecorate %95 NonUniform -OpDecorate %118 NonUniform -OpDecorate %120 NonUniform -OpDecorate %145 NonUniform -OpDecorate %147 NonUniform -OpDecorate %183 NonUniform -OpDecorate %211 NonUniform -OpDecorate %227 NonUniform -OpDecorate %243 NonUniform -OpDecorate %264 NonUniform -OpDecorate %266 NonUniform -OpDecorate %288 NonUniform -OpDecorate %290 NonUniform -OpDecorate %312 NonUniform -OpDecorate %314 NonUniform -OpDecorate %336 NonUniform -OpDecorate %338 NonUniform -OpDecorate %360 NonUniform -OpDecorate %362 NonUniform -OpDecorate %384 NonUniform -OpDecorate %386 NonUniform -OpDecorate %409 NonUniform +OpDecorate %96 NonUniform +OpDecorate %119 NonUniform +OpDecorate %121 NonUniform +OpDecorate %146 NonUniform +OpDecorate %148 NonUniform +OpDecorate %184 NonUniform +OpDecorate %212 NonUniform +OpDecorate %228 NonUniform +OpDecorate %244 NonUniform +OpDecorate %265 NonUniform +OpDecorate %267 NonUniform +OpDecorate %289 NonUniform +OpDecorate %291 NonUniform +OpDecorate %313 NonUniform +OpDecorate %315 NonUniform +OpDecorate %337 NonUniform +OpDecorate %339 NonUniform +OpDecorate %361 NonUniform +OpDecorate %363 NonUniform +OpDecorate %385 NonUniform +OpDecorate %387 NonUniform +OpDecorate %410 NonUniform %2 = OpTypeVoid %3 = OpTypeInt 32 0 %4 = OpTypeStruct %3 diff --git a/tests/out/spv/binding-buffer-arrays.spvasm b/tests/out/spv/binding-buffer-arrays.spvasm index 00d5b0fcfb..5d92b7e803 100644 --- a/tests/out/spv/binding-buffer-arrays.spvasm +++ b/tests/out/spv/binding-buffer-arrays.spvasm @@ -24,7 +24,7 @@ OpMemberDecorate %16 0 Offset 0 OpDecorate %23 Location 0 OpDecorate %23 Flat OpDecorate %26 Location 0 -OpDecorate %55 NonUniform +OpDecorate %58 NonUniform %2 = OpTypeVoid %3 = OpTypeInt 32 0 %4 = OpTypeStruct %3