<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 29, 2017 at 3:04 AM, Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com" target="_blank">siglesias@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">When creating a vtn_pointer for an OpVariable, the block_index and offsets fields are null because there is not ssa to take the data from. However, we can dereference that pointer when processing an SpvOp*AccessChain opcodes through vtn_ssa_offset_pointer_dereference() when the OpVariable when the StorageClass is Uniform or StorageBuffer. Inside vtn_ssa_offset_pointer_dereference() we have the code to initialize block_index and offset if they are null, but it is called after checking if the pointer has then non-null. </blockquote><div> </div><div>This seems fishy. The code you're moving only triggers for OpPtrAccessChain. In order to run into an issue, they would have to be doing an OpPtrAccessChain an an array of resources. This shouldn't be allowed by the spec. I'll have to look at the actual SPIR-V to be sure, but I think this is probably a CTS bug. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Reordering that code fixes crashes in: dEQP-VK.spirv_assembly.instruction.*.indexing.* Signed-off-by: Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> --- src/compiler/spirv/vtn_variables.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 4f6acd2e07..baf1edde4c 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -146,20 +146,6 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, struct vtn_type *type = base->type; unsigned idx = 0; - if (deref_chain->ptr_as_array) { - /* We need ptr_type for the stride */ - assert(base->ptr_type); - /* This must be a pointer to an actual element somewhere */ - assert(block_index && offset); - /* We need at least one element in the chain */ - assert(deref_chain->length >= 1); - - nir_ssa_def *elem_offset = - vtn_access_link_as_ssa(b, deref_chain->link[idx], - base->ptr_type->stride); - offset = nir_iadd(&b->nb, offset, elem_offset); - idx++; - } if (!block_index) { assert(base->var); @@ -182,6 +168,21 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, } assert(offset); + if (deref_chain->ptr_as_array) { + /* We need ptr_type for the stride */ + assert(base->ptr_type); + /* This must be a pointer to an actual element somewhere */ + assert(block_index && offset); + /* We need at least one element in the chain */ + assert(deref_chain->length >= 1); + + nir_ssa_def *elem_offset = + vtn_access_link_as_ssa(b, deref_chain->link[idx], + base->ptr_type->stride); + offset = nir_iadd(&b->nb, offset, elem_offset); + idx++; + } + for (; idx < deref_chain->length; idx++) { switch (glsl_get_base_type(type->type)) { case GLSL_TYPE_UINT: -- 2.14.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>