[Mesa-dev] [PATCH v3 1/2] spirv/nir: handle memory access qualifiers for SSBO loads/stores
Jason Ekstrand
jason at jlekstrand.net
Thu Oct 11 15:15:43 UTC 2018
Ok, I think there's a decent chance we've got them all now.
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
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. :/
On Wed, Oct 10, 2018 at 3:43 AM Samuel Pitoiset <samuel.pitoiset at gmail.com>
wrote:
> 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 <samuel.pitoiset at gmail.com>
> ---
> 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
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181011/61f97a63/attachment-0001.html>
More information about the mesa-dev
mailing list