<div dir="ltr"><div>Ok, I think there's a decent chance we've got them all now.</div><div> </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div><div> </div><div>It'd be great if we had some sort of test which put the access qualifiers on the variable so we could be sure it worked. :/ </div></div> <div class="gmail_quote"><div dir="ltr">On Wed, Oct 10, 2018 at 3:43 AM Samuel Pitoiset <<a href="mailto:samuel.pitoiset@gmail.com">samuel.pitoiset@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">v2: - change how the access qualifiers are accumulated v3: - duplicate members in struct_member_decoration_cb() - handle access qualifiers on variables - remove access qualifiers handling in _vtn_variable_load_store() - fix setting access qualifiers on type->array_element Signed-off-by: Samuel Pitoiset <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank">samuel.pitoiset@gmail.com</a>> --- src/compiler/nir/nir_intrinsics.py | 4 +-- src/compiler/spirv/spirv_to_nir.c | 24 +++++++++++-- src/compiler/spirv/vtn_private.h | 9 +++++ src/compiler/spirv/vtn_variables.c | 54 +++++++++++++++++++++++++----- 4 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/compiler/nir/nir_intrinsics.py b/src/compiler/nir/nir_intrinsics.py index b06b38fc2c..ec3049ca06 100644 --- a/src/compiler/nir/nir_intrinsics.py +++ b/src/compiler/nir/nir_intrinsics.py @@ -565,7 +565,7 @@ intrinsic("load_interpolated_input", src_comp=[2, 1], dest_comp=0, indices=[BASE, COMPONENT], flags=[CAN_ELIMINATE, CAN_REORDER]) # src[] = { buffer_index, offset }. No const_index -load("ssbo", 2, flags=[CAN_ELIMINATE]) +load("ssbo", 2, flags=[CAN_ELIMINATE], indices=[ACCESS]) # src[] = { offset }. const_index[] = { base, component } load("output", 1, [BASE, COMPONENT], flags=[CAN_ELIMINATE]) # src[] = { vertex, offset }. const_index[] = { base } @@ -591,6 +591,6 @@ store("output", 2, [BASE, WRMASK, COMPONENT]) # const_index[] = { base, write_mask, component } store("per_vertex_output", 3, [BASE, WRMASK, COMPONENT]) # src[] = { value, block_index, offset }. const_index[] = { write_mask } -store("ssbo", 3, [WRMASK]) +store("ssbo", 3, [WRMASK, ACCESS]) # src[] = { value, offset }. const_index[] = { base, write_mask } store("shared", 2, [BASE, WRMASK]) diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c index 2ad83196e4..37a801037b 100644 --- a/src/compiler/spirv/spirv_to_nir.c +++ b/src/compiler/spirv/spirv_to_nir.c @@ -668,6 +668,16 @@ mutable_matrix_member(struct vtn_builder *b, struct vtn_type *type, int member) return type; } +static void +vtn_handle_access_qualifier(struct vtn_builder *b, struct vtn_type *type, + int member, enum gl_access_qualifier access) +{ + type->members[member] = vtn_type_copy(b, type->members[member]); + type = type->members[member]; + + type->access |= access; +} + static void struct_member_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member, @@ -681,13 +691,21 @@ struct_member_decoration_cb(struct vtn_builder *b, assert(member < ctx->num_fields); switch (dec->decoration) { + case SpvDecorationRelaxedPrecision: + case SpvDecorationUniform: + break; /* FIXME: Do nothing with this for now. */ case SpvDecorationNonWritable: + vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_NON_WRITEABLE); + break; case SpvDecorationNonReadable: - case SpvDecorationRelaxedPrecision: + vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_NON_READABLE); + break; case SpvDecorationVolatile: + vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_VOLATILE); + break; case SpvDecorationCoherent: - case SpvDecorationUniform: - break; /* FIXME: Do nothing with this for now. */ + vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_COHERENT); + break; case SpvDecorationNoPerspective: ctx->fields[member].interpolation = INTERP_MODE_NOPERSPECTIVE; break; diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index a31202d129..da7a04ce59 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -296,6 +296,9 @@ struct vtn_type { /* for arrays, matrices and pointers, the array stride */ unsigned stride; + /* Access qualifiers */ + enum gl_access_qualifier access; + union { /* Members for scalar, vector, and array-like types */ struct { @@ -457,6 +460,9 @@ struct vtn_pointer { /** A (block_index, offset) pair representing a UBO or SSBO position. */ struct nir_ssa_def *block_index; struct nir_ssa_def *offset; + + /* Access qualifiers */ + enum gl_access_qualifier access; }; struct vtn_variable { @@ -488,6 +494,9 @@ struct vtn_variable { * hack at some point in the future. */ struct vtn_pointer *copy_prop_sampler; + + /* Access qualifiers. */ + enum gl_access_qualifier access; }; struct vtn_image_pointer { diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 636fdb8689..cc3438bff2 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -89,6 +89,7 @@ vtn_access_chain_pointer_dereference(struct vtn_builder *b, struct vtn_access_chain *chain = vtn_access_chain_extend(b, base->chain, deref_chain->length); struct vtn_type *type = base->type; + enum gl_access_qualifier access = base->access; /* OpPtrAccessChain is only allowed on things which support variable * pointers. For everything else, the client is expected to just pass us @@ -106,6 +107,8 @@ vtn_access_chain_pointer_dereference(struct vtn_builder *b, } else { type = type->array_element; } + + access |= type->access; } struct vtn_pointer *ptr = rzalloc(b, struct vtn_pointer); @@ -114,6 +117,7 @@ vtn_access_chain_pointer_dereference(struct vtn_builder *b, ptr->var = base->var; ptr->deref = base->deref; ptr->chain = chain; + ptr->access = access; return ptr; } @@ -184,6 +188,7 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, nir_ssa_def *block_index = base->block_index; nir_ssa_def *offset = base->offset; struct vtn_type *type = base->type; + enum gl_access_qualifier access = base->access; unsigned idx = 0; if (base->mode == vtn_variable_mode_ubo || @@ -198,6 +203,7 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, idx++; /* This consumes a level of type */ type = type->array_element; + access |= type->access; } else { /* This is annoying. We've been asked for a pointer to the * array of UBOs/SSBOs and not a specifc buffer. Return a @@ -319,6 +325,7 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, vtn_access_link_as_ssa(b, deref_chain->link[idx], type->stride); offset = nir_iadd(&b->nb, offset, elem_offset); type = type->array_element; + access |= type->access; break; } @@ -328,6 +335,7 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, nir_ssa_def *mem_offset = nir_imm_int(&b->nb, type->offsets[member]); offset = nir_iadd(&b->nb, offset, mem_offset); type = type->members[member]; + access |= type->access; break; } @@ -341,6 +349,7 @@ vtn_ssa_offset_pointer_dereference(struct vtn_builder *b, ptr->type = type; ptr->block_index = block_index; ptr->offset = offset; + ptr->access = access; return ptr; } @@ -370,6 +379,7 @@ vtn_pointer_for_variable(struct vtn_builder *b, vtn_assert(ptr_type->deref->type == var->type->type); pointer->ptr_type = ptr_type; pointer->var = var; + pointer->access = var->access | var->type->access; return pointer; } @@ -608,7 +618,8 @@ static void _vtn_load_store_tail(struct vtn_builder *b, nir_intrinsic_op op, bool load, nir_ssa_def *index, nir_ssa_def *offset, unsigned access_offset, unsigned access_size, - struct vtn_ssa_value **inout, const struct glsl_type *type) + struct vtn_ssa_value **inout, const struct glsl_type *type, + enum gl_access_qualifier access) { nir_intrinsic_instr *instr = nir_intrinsic_instr_create(b->nb.shader, op); instr->num_components = glsl_get_vector_elements(type); @@ -624,6 +635,11 @@ _vtn_load_store_tail(struct vtn_builder *b, nir_intrinsic_op op, bool load, nir_intrinsic_set_range(instr, access_size); } + if (op == nir_intrinsic_load_ssbo || + op == nir_intrinsic_store_ssbo) { + nir_intrinsic_set_access(instr, access); + } + if (index) instr->src[src++] = nir_src_for_ssa(index); @@ -654,7 +670,8 @@ static void _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, nir_ssa_def *index, nir_ssa_def *offset, unsigned access_offset, unsigned access_size, - struct vtn_type *type, struct vtn_ssa_value **inout) + struct vtn_type *type, enum gl_access_qualifier access, + struct vtn_ssa_value **inout) { if (load && *inout == NULL) *inout = vtn_create_ssa_value(b, type->type); @@ -704,7 +721,8 @@ _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, _vtn_load_store_tail(b, op, load, index, elem_offset, access_offset, access_size, &(*inout)->elems[i], - glsl_vector_type(base_type, vec_width)); + glsl_vector_type(base_type, vec_width), + type->access | access); } if (load && type->row_major) @@ -717,7 +735,8 @@ _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, vtn_assert(glsl_type_is_vector_or_scalar(type->type)); _vtn_load_store_tail(b, op, load, index, offset, access_offset, access_size, - inout, type->type); + inout, type->type, + type->access | access); } else { /* This is a strided load. We have to load N things separately. * This is the single column of a row-major matrix case. @@ -738,7 +757,8 @@ _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, comp = &temp_val; _vtn_load_store_tail(b, op, load, index, elem_offset, access_offset, access_size, - &comp, glsl_scalar_type(base_type)); + &comp, glsl_scalar_type(base_type), + type->access | access); per_comp[i] = comp->def; } @@ -758,7 +778,9 @@ _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, nir_iadd(&b->nb, offset, nir_imm_int(&b->nb, i * type->stride)); _vtn_block_load_store(b, op, load, index, elem_off, access_offset, access_size, - type->array_element, &(*inout)->elems[i]); + type->array_element, + type->array_element->access | access, + &(*inout)->elems[i]); } return; } @@ -770,7 +792,9 @@ _vtn_block_load_store(struct vtn_builder *b, nir_intrinsic_op op, bool load, nir_iadd(&b->nb, offset, nir_imm_int(&b->nb, type->offsets[i])); _vtn_block_load_store(b, op, load, index, elem_off, access_offset, access_size, - type->members[i], &(*inout)->elems[i]); + type->members[i], + type->members[i]->access | access, + &(*inout)->elems[i]); } return; } @@ -809,7 +833,7 @@ vtn_block_load(struct vtn_builder *b, struct vtn_pointer *src) struct vtn_ssa_value *value = NULL; _vtn_block_load_store(b, op, true, index, offset, access_offset, access_size, - src->type, &value); + src->type, src->access, &value); return value; } @@ -833,7 +857,7 @@ vtn_block_store(struct vtn_builder *b, struct vtn_ssa_value *src, offset = vtn_pointer_to_offset(b, dst, &index); _vtn_block_load_store(b, op, false, index, offset, - 0, 0, dst->type, &src); + 0, 0, dst->type, dst->access, &src); } static void @@ -1389,6 +1413,18 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member, case SpvDecorationOffset: vtn_var->offset = dec->literals[0]; break; + case SpvDecorationNonWritable: + vtn_var->access |= ACCESS_NON_WRITEABLE; + break; + case SpvDecorationNonReadable: + vtn_var->access |= ACCESS_NON_READABLE; + break; + case SpvDecorationVolatile: + vtn_var->access |= ACCESS_VOLATILE; + break; + case SpvDecorationCoherent: + vtn_var->access |= ACCESS_COHERENT; + break; default: break; } -- 2.19.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">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>