[Mesa-dev] [RFC] nir: Add a deref instruction type
Bas Nieuwenhuizen
bas at basnieuwenhuizen.nl
Thu Mar 15 15:22:26 UTC 2018
On Thu, Mar 15, 2018 at 6:33 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> This commit adds a new instruction type to NIR for handling derefs.
> Nothing uses it yet but this adds the data structure as well as all of
> the code to validate, print, clone, and [de]serialize them.
>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Connor Abbott <cwabbott0 at gmail.com>
>
> ---
>
> This is not tested beyond compile testing. I'm sending it out ahead so
> that people can comment on the instruction data structure. I think this
> should handle all the SPIR-V use-cases fairly nicely as well as the
> use-cases we have today.
>
> src/compiler/nir/nir.c | 49 +++++++++++++++++++++++
> src/compiler/nir/nir.h | 47 +++++++++++++++++++++-
> src/compiler/nir/nir_clone.c | 45 +++++++++++++++++++++
> src/compiler/nir/nir_print.c | 46 ++++++++++++++++++++++
> src/compiler/nir/nir_serialize.c | 85 ++++++++++++++++++++++++++++++++++++++++
> src/compiler/nir/nir_validate.c | 67 +++++++++++++++++++++++++++++++
> 6 files changed, 338 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index a97b119..1023eb9 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -469,6 +469,26 @@ nir_alu_instr_create(nir_shader *shader, nir_op op)
> return instr;
> }
>
> +nir_deref_instr *
> +nir_deref_instr_create(nir_shader *shader, nir_deref_type deref_type)
> +{
> + nir_deref_instr *instr =
> + rzalloc_size(shader, sizeof(nir_deref_instr));
> +
> + instr_init(&instr->instr, nir_instr_type_deref);
> +
> + instr->deref_type = deref_type;
> + if (deref_type != nir_deref_type_var)
> + src_init(&instr->parent);
> +
> + if (deref_type == nir_deref_type_array_indirect)
> + src_init(&instr->arr.indirect);
> +
> + dest_init(&instr->dest);
> +
> + return instr;
> +}
> +
> nir_jump_instr *
> nir_jump_instr_create(nir_shader *shader, nir_jump_type type)
> {
> @@ -1198,6 +1218,12 @@ visit_alu_dest(nir_alu_instr *instr, nir_foreach_dest_cb cb, void *state)
> }
>
> static bool
> +visit_deref_dest(nir_deref_instr *instr, nir_foreach_dest_cb cb, void *state)
> +{
> + return cb(&instr->dest, state);
> +}
> +
> +static bool
> visit_intrinsic_dest(nir_intrinsic_instr *instr, nir_foreach_dest_cb cb,
> void *state)
> {
> @@ -1238,6 +1264,8 @@ nir_foreach_dest(nir_instr *instr, nir_foreach_dest_cb cb, void *state)
> switch (instr->type) {
> case nir_instr_type_alu:
> return visit_alu_dest(nir_instr_as_alu(instr), cb, state);
> + case nir_instr_type_deref:
> + return visit_deref_dest(nir_instr_as_deref(instr), cb, state);
> case nir_instr_type_intrinsic:
> return visit_intrinsic_dest(nir_instr_as_intrinsic(instr), cb, state);
> case nir_instr_type_tex:
> @@ -1349,6 +1377,23 @@ visit_alu_src(nir_alu_instr *instr, nir_foreach_src_cb cb, void *state)
> }
>
> static bool
> +visit_deref_instr_src(nir_deref_instr *instr,
> + nir_foreach_src_cb cb, void *state)
> +{
> + if (instr->deref_type != nir_deref_type_var) {
> + if (!visit_src(&instr->parent, cb, state))
> + return false;
> + }
> +
> + if (instr->deref_type == nir_deref_type_array_indirect) {
> + if (!visit_src(&instr->arr.indirect, cb, state))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool
> visit_tex_src(nir_tex_instr *instr, nir_foreach_src_cb cb, void *state)
> {
> for (unsigned i = 0; i < instr->num_srcs; i++) {
> @@ -1436,6 +1481,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb cb, void *state)
> if (!visit_alu_src(nir_instr_as_alu(instr), cb, state))
> return false;
> break;
> + case nir_instr_type_deref:
> + if (!visit_deref_instr_src(nir_instr_as_deref(instr), cb, state))
> + return false;
> + break;
> case nir_instr_type_intrinsic:
> if (!visit_intrinsic_src(nir_instr_as_intrinsic(instr), cb, state))
> return false;
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 839d403..a40a3a0 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -421,6 +421,7 @@ typedef struct nir_register {
>
> typedef enum {
> nir_instr_type_alu,
> + nir_instr_type_deref,
> nir_instr_type_call,
> nir_instr_type_tex,
> nir_instr_type_intrinsic,
> @@ -888,7 +889,10 @@ bool nir_alu_srcs_equal(const nir_alu_instr *alu1, const nir_alu_instr *alu2,
> typedef enum {
> nir_deref_type_var,
> nir_deref_type_array,
> - nir_deref_type_struct
> + nir_deref_type_struct,
> + nir_deref_type_array_direct,
> + nir_deref_type_array_indirect,
> + nir_deref_type_array_wildcard,
> } nir_deref_type;
>
> typedef struct nir_deref {
> @@ -950,6 +954,42 @@ nir_deref_tail(nir_deref *deref)
> typedef struct {
> nir_instr instr;
>
> + /** The type of this deref instruction */
> + nir_deref_type deref_type;
> +
> + /** The mode of the underlying variable */
> + nir_variable_mode mode;
> +
> + /** The dereferenced type of the resulting pointer value */
> + const struct glsl_type *type;
> +
> + union {
> + /** Variable being dereferenced if deref_type is a deref_var */
> + nir_variable *var;
> +
> + /** Parent deref if deref_type is not deref_var */
> + nir_src parent;
> + };
> +
> + /** Additional deref parameters */
> + union {
> + struct {
> + unsigned base_offset;
> + nir_src indirect;
> + } arr;
> +
> + struct {
> + unsigned index;
> + } strct;
> + };
> +
> + /** Destination to store the resulting "pointer" */
> + nir_dest dest;
> +} nir_deref_instr;
> +
> +typedef struct {
> + nir_instr instr;
> +
> unsigned num_params;
> nir_deref_var **params;
> nir_deref_var *return_deref;
> @@ -1521,6 +1561,8 @@ typedef struct {
>
> NIR_DEFINE_CAST(nir_instr_as_alu, nir_instr, nir_alu_instr, instr,
> type, nir_instr_type_alu)
> +NIR_DEFINE_CAST(nir_instr_as_deref, nir_instr, nir_deref_instr, instr,
> + type, nir_instr_type_deref)
> NIR_DEFINE_CAST(nir_instr_as_call, nir_instr, nir_call_instr, instr,
> type, nir_instr_type_call)
> NIR_DEFINE_CAST(nir_instr_as_jump, nir_instr, nir_jump_instr, instr,
> @@ -2047,6 +2089,9 @@ void nir_metadata_preserve(nir_function_impl *impl, nir_metadata preserved);
> /** creates an instruction with default swizzle/writemask/etc. with NULL registers */
> nir_alu_instr *nir_alu_instr_create(nir_shader *shader, nir_op op);
>
> +nir_deref_instr *nir_deref_instr_create(nir_shader *shader,
> + nir_deref_type deref_type);
> +
> nir_jump_instr *nir_jump_instr_create(nir_shader *shader, nir_jump_type type);
>
> nir_load_const_instr *nir_load_const_instr_create(nir_shader *shader,
> diff --git a/src/compiler/nir/nir_clone.c b/src/compiler/nir/nir_clone.c
> index bcfdaa7..98b3f98 100644
> --- a/src/compiler/nir/nir_clone.c
> +++ b/src/compiler/nir/nir_clone.c
> @@ -346,6 +346,49 @@ clone_alu(clone_state *state, const nir_alu_instr *alu)
> return nalu;
> }
>
> +static nir_deref_instr *
> +clone_deref_instr(clone_state *state, const nir_deref_instr *deref)
> +{
> + nir_deref_instr *nderef =
> + nir_deref_instr_create(state->ns, deref->deref_type);
> +
> + __clone_dst(state, &nderef->instr, &nderef->dest, &deref->dest);
> +
> + nderef->mode = deref->mode;
> + nderef->type = deref->type;
> +
> + if (deref->deref_type == nir_deref_type_var) {
> + nderef->var = remap_var(state, deref->var);
> + } else {
> + __clone_src(state, &nderef->instr, &nderef->parent, &deref->parent);
> + }
> +
> + switch (deref->deref_type) {
> + case nir_deref_type_struct:
> + nderef->strct.index = deref->strct.index;
> + break;
> +
> + case nir_deref_type_array_indirect:
> + __clone_src(state, &nderef->instr,
> + &nderef->arr.indirect, &deref->arr.indirect);
> + /* fall through */
> +
> + case nir_deref_type_array_direct:
> + nderef->arr.base_offset = deref->arr.base_offset;
> + break;
> +
> + case nir_deref_type_var:
> + case nir_deref_type_array_wildcard:
> + /* Nothing to do */
> + break;
> +
> + default:
> + unreachable("Invalid instruction deref type");
> + }
> +
> + return nderef;
> +}
> +
> static nir_intrinsic_instr *
> clone_intrinsic(clone_state *state, const nir_intrinsic_instr *itr)
> {
> @@ -502,6 +545,8 @@ clone_instr(clone_state *state, const nir_instr *instr)
> switch (instr->type) {
> case nir_instr_type_alu:
> return &clone_alu(state, nir_instr_as_alu(instr))->instr;
> + case nir_instr_type_deref:
> + return &clone_deref_instr(state, nir_instr_as_deref(instr))->instr;
> case nir_instr_type_intrinsic:
> return &clone_intrinsic(state, nir_instr_as_intrinsic(instr))->instr;
> case nir_instr_type_load_const:
> diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c
> index 7888dbd..32aa3fc 100644
> --- a/src/compiler/nir/nir_print.c
> +++ b/src/compiler/nir/nir_print.c
> @@ -486,6 +486,48 @@ print_var_decl(nir_variable *var, print_state *state)
> }
>
> static void
> +print_deref_instr(nir_deref_instr *instr, print_state *state)
> +{
> + FILE *fp = state->fp;
> +
> + print_dest(&instr->dest, state);
> +
> + if (instr->deref_type == nir_deref_type_var) {
> + fprintf(fp, " = %s", get_var_name(instr->var, state));
> + return;
> + }
> +
> + fprintf(fp, " = &");
> + print_src(&instr->parent, state);
> +
> + assert(instr->parent.is_ssa);
> + nir_deref_instr *parent =
> + nir_instr_as_deref(instr->parent.ssa->parent_instr);
> +
> + switch (instr->deref_type) {
> + case nir_deref_type_struct:
> + fprintf(fp, "->%s",
> + glsl_get_struct_elem_name(parent->type, instr->strct.index));
> + break;
> +
> + case nir_deref_type_array_direct:
> + fprintf(fp, "[%u]", instr->arr.base_offset);
> + break;
> +
> + case nir_deref_type_array_indirect:
> + fprintf(fp, "[");
> + if (instr->arr.base_offset)
> + fprintf(fp, "%u + ", instr->arr.base_offset);
> + print_src(&instr->arr.indirect, state);
> + fprintf(fp, "]");
> + break;
> +
> + default:
> + unreachable("Invalid deref instruction type");
> + }
> +}
> +
> +static void
> print_var(nir_variable *var, print_state *state)
> {
> FILE *fp = state->fp;
> @@ -922,6 +964,10 @@ print_instr(const nir_instr *instr, print_state *state, unsigned tabs)
> print_alu_instr(nir_instr_as_alu(instr), state);
> break;
>
> + case nir_instr_type_deref:
> + print_deref_instr(nir_instr_as_deref(instr), state);
> + break;
> +
> case nir_instr_type_call:
> print_call_instr(nir_instr_as_call(instr), state);
> break;
> diff --git a/src/compiler/nir/nir_serialize.c b/src/compiler/nir/nir_serialize.c
> index 00df49c..87e40d9 100644
> --- a/src/compiler/nir/nir_serialize.c
> +++ b/src/compiler/nir/nir_serialize.c
> @@ -479,6 +479,85 @@ read_alu(read_ctx *ctx)
> }
>
> static void
> +write_deref(write_ctx *ctx, const nir_deref_instr *deref)
> +{
> + blob_write_uint32(ctx->blob, deref->deref_type);
> +
> + blob_write_uint32(ctx->blob, deref->mode);
> + encode_type_to_blob(ctx->blob, deref->type);
> +
> + write_dest(ctx, &deref->dest);
> +
> + if (deref->deref_type == nir_deref_type_var) {
> + write_object(ctx, deref->var);
> + return;
> + }
> +
> + write_src(ctx, &deref->parent);
> +
> + switch (deref->deref_type) {
> + case nir_deref_type_struct:
> + blob_write_uint32(ctx->blob, deref->strct.index);
> + break;
> +
> + case nir_deref_type_array_indirect:
> + write_src(ctx, &deref->arr.indirect);
> + /* fall through */
> + case nir_deref_type_array_direct:
> + blob_write_uint32(ctx->blob, deref->arr.base_offset);
> + break;
> +
> + case nir_deref_type_array_wildcard:
> + /* Nothing to do */
> + break;
> +
> + default:
> + unreachable("Invalid deref type");
> + }
> +}
> +
> +static nir_deref_instr *
> +read_deref(read_ctx *ctx)
> +{
> + nir_deref_type deref_type = blob_read_uint32(ctx->blob);
> + nir_deref_instr *deref = nir_deref_instr_create(ctx->nir, deref_type);
> +
> + deref->mode = blob_read_uint32(ctx->blob);
> + deref->type = decode_type_from_blob(ctx->blob);
> +
> + read_dest(ctx, &deref->dest, &deref->instr);
> +
> + if (deref_type == nir_deref_type_var) {
> + deref->var = read_object(ctx);
> + return deref;
> + }
> +
> + read_src(ctx, &deref->parent, &deref->instr);
> +
> + switch (deref->deref_type) {
> + case nir_deref_type_struct:
> + deref->strct.index = blob_read_uint32(ctx->blob);
> + break;
> +
> + case nir_deref_type_array_indirect:
> + read_src(ctx, &deref->arr.indirect, &deref->instr);
> + /* fall through */
> + case nir_deref_type_array_direct:
> + deref->arr.base_offset = blob_read_uint32(ctx->blob);
> + break;
> +
> + case nir_deref_type_array_wildcard:
> + /* Nothing to do */
> + break;
> +
> + default:
> + unreachable("Invalid deref type");
> + }
> +
> + return deref;
> +}
> +
> +static void
> write_intrinsic(write_ctx *ctx, const nir_intrinsic_instr *intrin)
> {
> blob_write_uint32(ctx->blob, intrin->intrinsic);
> @@ -803,6 +882,9 @@ write_instr(write_ctx *ctx, const nir_instr *instr)
> case nir_instr_type_alu:
> write_alu(ctx, nir_instr_as_alu(instr));
> break;
> + case nir_instr_type_deref:
> + write_deref(ctx, nir_instr_as_deref(instr));
> + break;
> case nir_instr_type_intrinsic:
> write_intrinsic(ctx, nir_instr_as_intrinsic(instr));
> break;
> @@ -840,6 +922,9 @@ read_instr(read_ctx *ctx, nir_block *block)
> case nir_instr_type_alu:
> instr = &read_alu(ctx)->instr;
> break;
> + case nir_instr_type_deref:
> + instr = &read_deref(ctx)->instr;
> + break;
> case nir_instr_type_intrinsic:
> instr = &read_intrinsic(ctx)->instr;
> break;
> diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
> index a49948f..354b32b 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -397,6 +397,69 @@ validate_alu_instr(nir_alu_instr *instr, validate_state *state)
> }
>
> static void
> +validate_deref_instr(nir_deref_instr *instr, validate_state *state)
> +{
> + if (instr->deref_type == nir_deref_type_var) {
> + /* Variable dereferences are stupid simple. */
> + validate_assert(state, instr->mode == instr->var->data.mode);
> + validate_assert(state, instr->type == instr->var->type);
> + } else {
> + /* We require the parent to be SSA. This may be lifted in the future */
> + validate_assert(state, instr->parent.is_ssa);
> +
> + /* The parent pointer value must have the same number of components
> + * as the destination.
> + */
> + validate_src(&instr->parent, state, nir_dest_bit_size(instr->dest),
> + nir_dest_num_components(instr->dest));
> +
> + nir_instr *parent_instr = instr->parent.ssa->parent_instr;
> +
> + /* The parent must come from another deref instruction */
> + validate_assert(state, parent_instr->type == nir_instr_type_deref);
I thought a lot of the value was that we could pipe this through
things like phis eventually? I'm fine with not starting that way
though.
> +
> + nir_deref_instr *parent = nir_instr_as_deref(parent_instr);
> +
> + validate_assert(state, instr->mode == parent->mode);
> +
> + switch (instr->deref_type) {
> + case nir_deref_type_struct:
> + validate_assert(state, glsl_type_is_struct(parent->type));
> + validate_assert(state,
> + instr->strct.index < glsl_get_length(parent->type));
> + validate_assert(state, instr->type ==
> + glsl_get_struct_field(parent->type, instr->strct.index));
> + break;
> +
> + case nir_deref_type_array_direct:
> + case nir_deref_type_array_indirect:
> + case nir_deref_type_array_wildcard:
> + validate_assert(state, glsl_type_is_array(parent->type));
> + validate_assert(state,
> + instr->type == glsl_get_array_element(parent->type));
> +
> + if (instr->deref_type != nir_deref_type_array_wildcard) {
> + validate_assert(state,
> + instr->arr.base_offset < glsl_get_length(parent->type));
> + }
> +
> + if (instr->deref_type == nir_deref_type_array_indirect)
> + validate_src(&instr->arr.indirect, state, 32, 1);
> + break;
> +
> + default:
> + unreachable("Invalid deref instruction type");
> + }
> + }
> +
> + /* We intentionally don't validate the size of the destination because we
> + * want to let other compiler components such as SPIR-V decide how big
> + * pointers should be.
> + */
> + validate_dest(&instr->dest, state, 0, 0);
> +}
> +
> +static void
> validate_deref_chain(nir_deref *deref, nir_variable_mode mode,
> validate_state *state)
> {
> @@ -622,6 +685,10 @@ validate_instr(nir_instr *instr, validate_state *state)
> validate_alu_instr(nir_instr_as_alu(instr), state);
> break;
>
> + case nir_instr_type_deref:
> + validate_deref_instr(nir_instr_as_deref(instr), state);
> + break;
> +
> case nir_instr_type_call:
> validate_call_instr(nir_instr_as_call(instr), state);
> break;
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list