On 09/07/2017 07:15 PM, Jason Ekstrand wrote: > On Tue, Aug 29, 2017 at 3:04 AM, Samuel Iglesias Gonsálvez > <sigles...@igalia.com <mailto:sigles...@igalia.com>> wrote: > > 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. > > > 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. >
For example, one of the tests failing is:
dEQP-VK.spirv_assembly.instruction.compute.indexing.opptraccesschain_u32
A snippet of its SPIR-V is:
[...]
OpDecorate %_ptr_buffer_Input ArrayStride 65536
[...]
%Input = OpTypeStruct %_arr__arr_mat4v4float_uint_32_uint_32
%_ptr_buffer_Input = OpTypePointer StorageBuffer %Input
%dataInput = OpVariable %_ptr_buffer_Input StorageBuffer
[...]
%54 = OpPtrAccessChain %_ptr_buffer_float %dataInput %idx_1 %idx_0 %i0
%i1 %i2 %i3
%dataInput points to a struct of an AoA of matrices. However, according
to SPV_KHR_variable_pointers [0]:
*
Each *OpPtrAccessChain* must have a /Base/ whose type is decorated
with *ArrayStride*.
So I think this is fine, or am I understanding it wrongly?
Sam
[0]
https://www.khronos.org/registry/spir-v/extensions/KHR/SPV_KHR_variable_pointers.html
> Reordering that code fixes crashes in:
>
> dEQP-VK.spirv_assembly.instruction.*.indexing.*
>
> Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com
> <mailto:sigles...@igalia.com>>
> ---
> 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
> mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
