On Aug 30, 2015 3:44 AM, "Timothy Arceri" <<a href="mailto:t_arceri@yahoo.com.au">t_arceri@yahoo.com.au</a>> wrote: > > As a bonus we get indirect support for arrays of arrays for free. > --- > I've marked this as request for comment as although it works enough for to > pass all the piglit tests there are issues as I'm still trying to figure out > nir. > > The issues are around the second call to nir_instr_move_src() this call > expects the use_link list not to be empty just so it can then remove all > entries so I add the list_addtail() call above it to work around this. > While this is enough to pass the piglit tests on a non debug build it fails > validation for debug builds. > > I'm clearly missing a step and would be greatful to anyone who can point me > in the right direction. nir_instr_move_src expects the source to be already in an instruction. The reason its correct in the old code is because were stealing it from the indirect. For adding a "new" nir_src, use nir_instr_rewrite_src. --Jason > The validation error: > nir/nir_validate.c:873: postvalidate_ssa_def: Assertion `entry' failed. > > src/glsl/nir/nir_lower_samplers.cpp | 169 ++++++++++++++++++++---------------- > 1 file changed, 95 insertions(+), 74 deletions(-) > > diff --git a/src/glsl/nir/nir_lower_samplers.cpp b/src/glsl/nir/nir_lower_samplers.cpp > index 9583b45..dbe9e95 100644 > --- a/src/glsl/nir/nir_lower_samplers.cpp > +++ b/src/glsl/nir/nir_lower_samplers.cpp > @@ -24,6 +24,7 @@ > */ > > #include "nir.h" > +#include "nir_builder.h" > #include "../program.h" > #include "program/hash_table.h" > #include "ir_uniform.h" > @@ -35,89 +36,49 @@ extern "C" { > #include "program/program.h" > } > > -static unsigned > -get_sampler_index(const struct gl_shader_program *shader_program, > - gl_shader_stage stage, const char *name) > -{ > - unsigned location; > - if (!shader_program->UniformHash->get(location, name)) { > - assert(!"failed to find sampler"); > - return 0; > - } > - > - if (!shader_program->UniformStorage[location].sampler[stage].active) { > - assert(!"cannot return a sampler"); > - return 0; > - } > - > - return shader_program->UniformStorage[location].sampler[stage].index; > -} > - > +/* Calculate the sampler index based on array indicies and also > + * calculate the base uniform location for struct members. > + */ > static void > -lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_program, > - gl_shader_stage stage, void *mem_ctx) > +calc_sampler_offsets(nir_deref *tail, nir_tex_instr *instr, > + unsigned *array_elements, nir_src *indirect, > + nir_builder *b, unsigned *location, > + bool *found_indirect) > { > - if (instr->sampler == NULL) > - return; > - > - /* Get the name and the offset */ > - instr->sampler_index = 0; > - char *name = ralloc_strdup(mem_ctx, instr->sampler->var->name); > - > - for (nir_deref *deref = &instr->sampler->deref; > - deref->child; deref = deref->child) { > - switch (deref->child->deref_type) { > + if (tail->child != NULL) { > + switch (tail->child->deref_type) { > case nir_deref_type_array: { > - nir_deref_array *deref_array = nir_deref_as_array(deref->child); > + nir_deref_array *deref_array = nir_deref_as_array(tail->child); > > assert(deref_array->deref_array_type != nir_deref_array_type_wildcard); > > - if (deref_array->deref.child) { > - ralloc_asprintf_append(&name, "[%u]", > - deref_array->deref_array_type == nir_deref_array_type_direct ? > - deref_array->base_offset : 0); > - } else { > - assert(deref->child->type->base_type == GLSL_TYPE_SAMPLER); > - instr->sampler_index = deref_array->base_offset; > - } > + calc_sampler_offsets(tail->child, instr, array_elements, > + indirect, b, location, found_indirect); > + instr->sampler_index += deref_array->base_offset * *array_elements; > > - /* XXX: We're assuming here that the indirect is the last array > - * thing we have. This should be ok for now as we don't support > - * arrays_of_arrays yet. > - */ > if (deref_array->deref_array_type == nir_deref_array_type_indirect) { > - /* First, we have to resize the array of texture sources */ > - nir_tex_src *new_srcs = rzalloc_array(instr, nir_tex_src, > - instr->num_srcs + 1); > - > - for (unsigned i = 0; i < instr->num_srcs; i++) { > - new_srcs[i].src_type = instr->src[i].src_type; > - nir_instr_move_src(&instr->instr, &new_srcs[i].src, > - &instr->src[i].src); > + nir_ssa_def *mul = > + nir_imul(b, nir_imm_int(b, *array_elements), > + nir_ssa_for_src(b, deref_array->indirect, 1)); > + > + if (*found_indirect) { > + indirect->ssa = > + nir_iadd(b, nir_ssa_for_src(b, *indirect, 1), mul); > + } else { > + *indirect = nir_src_for_ssa(mul); > + *found_indirect = true; > } > - > - ralloc_free(instr->src); > - instr->src = new_srcs; > - > - /* Now we can go ahead and move the source over to being a > - * first-class texture source. > - */ > - instr->src[instr->num_srcs].src_type = nir_tex_src_sampler_offset; > - instr->num_srcs++; > - nir_instr_move_src(&instr->instr, > - &instr->src[instr->num_srcs - 1].src, > - &deref_array->indirect); > - > - instr->sampler_array_size = glsl_get_length(deref->type); > } > - break; > + > + *array_elements *= glsl_get_length(tail->type); > + break; > } > > case nir_deref_type_struct: { > - nir_deref_struct *deref_struct = nir_deref_as_struct(deref->child); > - const char *field = glsl_get_struct_elem_name(deref->type, > - deref_struct->index); > - ralloc_asprintf_append(&name, ".%s", field); > + nir_deref_struct *deref_struct = nir_deref_as_struct(tail->child); > + *location += deref_struct->index; > + calc_sampler_offsets(tail->child, instr, array_elements, > + indirect, b, location, found_indirect); > break; > } > > @@ -125,15 +86,75 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr > unreachable("Invalid deref type"); > break; > } > + } else { > + return; > + } > +} > + > +static void > +lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_program, > + gl_shader_stage stage, nir_builder *builder) > +{ > + if (instr->sampler == NULL) > + return; > + > + instr->sampler_index = 0; > + unsigned location = instr->sampler->var->data.location; > + > + if (instr->sampler->deref.child) { > + unsigned array_elements = 1; > + nir_src indirect; > + bool found_indirect = false; > + > + builder->cursor = nir_before_instr(&instr->instr); > + calc_sampler_offsets(&instr->sampler->deref, instr, > + &array_elements, &indirect, builder, > + &location, &found_indirect); > + > + if (found_indirect) { > + /* First, we have to resize the array of texture sources */ > + nir_tex_src *new_srcs = rzalloc_array(instr, nir_tex_src, > + instr->num_srcs + 1); > + > + for (unsigned i = 0; i < instr->num_srcs; i++) { > + new_srcs[i].src_type = instr->src[i].src_type; > + nir_instr_move_src(&instr->instr, &new_srcs[i].src, > + &instr->src[i].src); > + } > + > + ralloc_free(instr->src); > + instr->src = new_srcs; > + > + /* Now we can go ahead and move the source over to being a > + * first-class texture source. > + */ > + nir_src indirect_nir_src = nir_src_for_ssa(indirect.ssa); > + list_addtail(&indirect_nir_src.use_link, &indirect_nir_src.ssa->uses); > + > + instr->src[instr->num_srcs].src_type = nir_tex_src_sampler_offset; > + instr->num_srcs++; > + nir_instr_move_src(&instr->instr, > + &instr->src[instr->num_srcs - 1].src, > + &indirect_nir_src); > + > + instr->sampler_array_size = array_elements; > + } > + } > + > + if (location > shader_program->NumUniformStorage - 1 || > + !shader_program->UniformStorage[location].sampler[stage].active) { > + assert(!"cannot return a sampler"); > + return; > } > > - instr->sampler_index += get_sampler_index(shader_program, stage, name); > + instr->sampler_index += > + shader_program->UniformStorage[location].sampler[stage].index; > > instr->sampler = NULL; > } > > typedef struct { > - void *mem_ctx; > + nir_builder builder; > const struct gl_shader_program *shader_program; > gl_shader_stage stage; > } lower_state; > @@ -147,7 +168,7 @@ lower_block_cb(nir_block *block, void *_state) > if (instr->type == nir_instr_type_tex) { > nir_tex_instr *tex_instr = nir_instr_as_tex(instr); > lower_sampler(tex_instr, state->shader_program, state->stage, > - state->mem_ctx); > + &state->builder); > } > } > > @@ -160,7 +181,7 @@ lower_impl(nir_function_impl *impl, const struct gl_shader_program *shader_progr > { > lower_state state; > > - state.mem_ctx = ralloc_parent(impl); > + nir_builder_init(&state.builder, impl); > state.shader_program = shader_program; > state.stage = stage; > > -- > 2.4.3 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>