diff --git a/naga/src/back/spv/block.rs b/naga/src/back/spv/block.rs index 27a48d948a..355689d852 100644 --- a/naga/src/back/spv/block.rs +++ b/naga/src/back/spv/block.rs @@ -1773,6 +1773,7 @@ impl<'w> BlockContext<'w> { is_non_uniform_binding_array |= self.is_nonuniform_binding_array_access(base, index); + let index = crate::proc::index::GuardedIndex::Expression(index); let index_id = self.write_access_chain_index(base, index, &mut accumulated_checks, block)?; self.temp_list.push(index_id); @@ -1780,8 +1781,30 @@ impl<'w> BlockContext<'w> { base } crate::Expression::AccessIndex { base, index } => { - let const_id = self.get_index_constant(index); - self.temp_list.push(const_id); + // Decide whether we're indexing a struct (bounds checks + // forbidden) or anything else (bounds checks required). + let mut base_ty = self.fun_info[base].ty.inner_with(&self.ir_module.types); + if let crate::TypeInner::Pointer { base, .. } = *base_ty { + base_ty = &self.ir_module.types[base].inner; + } + let index_id = if let crate::TypeInner::Struct { .. } = *base_ty { + self.get_index_constant(index) + } else { + // `index` is constant, so this can't possibly require + // setting `is_nonuniform_binding_array_access`. + + // Even though the index value is statically known, `base` + // may be a runtime-sized array, so we still need to go + // through the bounds check process. + self.write_access_chain_index( + base, + crate::proc::index::GuardedIndex::Known(index), + &mut accumulated_checks, + block, + )? + }; + + self.temp_list.push(index_id); base } crate::Expression::GlobalVariable(handle) => { @@ -1868,7 +1891,7 @@ impl<'w> BlockContext<'w> { fn write_access_chain_index( &mut self, base: Handle, - index: Handle, + index: crate::proc::index::GuardedIndex, accumulated_checks: &mut Option, block: &mut Block, ) -> Result { diff --git a/naga/src/back/spv/index.rs b/naga/src/back/spv/index.rs index bc00a969d0..9f32b244a1 100644 --- a/naga/src/back/spv/index.rs +++ b/naga/src/back/spv/index.rs @@ -479,11 +479,10 @@ impl<'w> BlockContext<'w> { pub(super) fn write_bounds_check( &mut self, base: Handle, - index: Handle, + mut index: GuardedIndex, block: &mut Block, ) -> Result { // If the value of `index` is known at compile time, find it now. - let mut index = GuardedIndex::Expression(index); index.try_resolve_to_constant(self.ir_function, self.ir_module); let policy = self.writer.bounds_check_policies.choose_policy( @@ -517,6 +516,7 @@ impl<'w> BlockContext<'w> { let result_type_id = self.get_expression_type_id(&self.fun_info[expr_handle].ty); let base_id = self.cached[base]; + let index = GuardedIndex::Expression(index); let result_id = match self.write_bounds_check(base, index, block)? { BoundsCheckResult::KnownInBounds(known_index) => { diff --git a/naga/tests/out/spv/bounds-check-restrict.spvasm b/naga/tests/out/spv/bounds-check-restrict.spvasm index 3987ab5217..c4ba6dfee8 100644 --- a/naga/tests/out/spv/bounds-check-restrict.spvasm +++ b/naga/tests/out/spv/bounds-check-restrict.spvasm @@ -1,7 +1,7 @@ ; SPIR-V ; Version: 1.1 ; Generator: rspirv -; Bound: 174 +; Bound: 180 OpCapability Shader OpCapability Linkage OpExtension "SPV_KHR_storage_buffer_storage_class" @@ -52,7 +52,7 @@ OpDecorate %12 Binding 0 %129 = OpTypeFunction %2 %11 %7 %138 = OpTypeFunction %2 %11 %11 %3 %158 = OpTypeFunction %2 %3 -%166 = OpConstant %6 1000 +%168 = OpConstant %6 1000 %16 = OpFunction %3 None %17 %15 = OpFunctionParameter %11 %14 = OpLabel @@ -238,16 +238,22 @@ OpFunctionEnd %163 = OpLabel OpBranch %165 %165 = OpLabel -%167 = OpAccessChain %20 %12 %35 %166 -%168 = OpLoad %3 %167 -OpReturnValue %168 -OpFunctionEnd -%171 = OpFunction %2 None %158 -%170 = OpFunctionParameter %3 -%169 = OpLabel -OpBranch %172 +%166 = OpArrayLength %6 %12 3 +%167 = OpISub %6 %166 %32 +%169 = OpExtInst %6 %1 UMin %168 %167 +%170 = OpAccessChain %20 %12 %35 %169 +%171 = OpLoad %3 %170 +OpReturnValue %171 +OpFunctionEnd +%174 = OpFunction %2 None %158 +%173 = OpFunctionParameter %3 %172 = OpLabel -%173 = OpAccessChain %20 %12 %35 %166 -OpStore %173 %170 +OpBranch %175 +%175 = OpLabel +%176 = OpArrayLength %6 %12 3 +%177 = OpISub %6 %176 %32 +%178 = OpExtInst %6 %1 UMin %168 %177 +%179 = OpAccessChain %20 %12 %35 %178 +OpStore %179 %173 OpReturn OpFunctionEnd \ No newline at end of file diff --git a/naga/tests/out/spv/bounds-check-zero.spvasm b/naga/tests/out/spv/bounds-check-zero.spvasm index fb34f5d814..f1f1bedf3f 100644 --- a/naga/tests/out/spv/bounds-check-zero.spvasm +++ b/naga/tests/out/spv/bounds-check-zero.spvasm @@ -1,7 +1,7 @@ ; SPIR-V ; Version: 1.1 ; Generator: rspirv -; Bound: 211 +; Bound: 220 OpCapability Shader OpCapability Linkage OpExtension "SPV_KHR_storage_buffer_storage_class" @@ -56,7 +56,7 @@ OpDecorate %12 Binding 0 %159 = OpTypeFunction %2 %11 %7 %170 = OpTypeFunction %2 %11 %11 %3 %195 = OpTypeFunction %2 %3 -%203 = OpConstant %6 1000 +%204 = OpConstant %6 1000 %16 = OpFunction %3 None %17 %15 = OpFunctionParameter %11 %14 = OpLabel @@ -314,16 +314,31 @@ OpFunctionEnd %200 = OpLabel OpBranch %202 %202 = OpLabel -%204 = OpAccessChain %20 %12 %37 %203 -%205 = OpLoad %3 %204 -OpReturnValue %205 +%203 = OpArrayLength %6 %12 3 +%205 = OpULessThan %22 %204 %203 +OpSelectionMerge %207 None +OpBranchConditional %205 %208 %207 +%208 = OpLabel +%206 = OpAccessChain %20 %12 %37 %204 +%209 = OpLoad %3 %206 +OpBranch %207 +%207 = OpLabel +%210 = OpPhi %3 %25 %202 %209 %208 +OpReturnValue %210 OpFunctionEnd -%208 = OpFunction %2 None %195 -%207 = OpFunctionParameter %3 -%206 = OpLabel -OpBranch %209 -%209 = OpLabel -%210 = OpAccessChain %20 %12 %37 %203 -OpStore %210 %207 +%213 = OpFunction %2 None %195 +%212 = OpFunctionParameter %3 +%211 = OpLabel +OpBranch %214 +%214 = OpLabel +%215 = OpArrayLength %6 %12 3 +%216 = OpULessThan %22 %204 %215 +OpSelectionMerge %218 None +OpBranchConditional %216 %219 %218 +%219 = OpLabel +%217 = OpAccessChain %20 %12 %37 %204 +OpStore %217 %212 +OpBranch %218 +%218 = OpLabel OpReturn OpFunctionEnd \ No newline at end of file