[Mesa-dev] [PATCH V4 6/6] nir: support indirect indexing samplers in struct arrays
Jason Ekstrand
jason at jlekstrand.net
Tue Sep 15 11:52:43 PDT 2015
On Tue, Sep 15, 2015 at 12:51 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> As a bonus we get indirect support for arrays of arrays for free.
>
> V4: fix struct member location caclulation, use nir_ssa_def rather than
> nir_src for the indirect as suggested by Jason
>
> V3: Use nir_instr_rewrite_src() with empty src rather then clearing
> the use_link list directly for the old indirects as suggested by Jason
>
> V2: Fixed validation error in debug build
>
> Cc: Jason Ekstrand <jason.ekstrand at intel.com>
> ---
> src/glsl/nir/nir_lower_samplers.cpp | 160 ++++++++++++++++++++----------------
> 1 file changed, 88 insertions(+), 72 deletions(-)
>
> diff --git a/src/glsl/nir/nir_lower_samplers.cpp b/src/glsl/nir/nir_lower_samplers.cpp
> index 9583b45..623a07c 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,105 +36,120 @@ 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)
> +/* Calculate the sampler index based on array indicies and also
> + * calculate the base uniform location for struct members.
> + */
> +static void
> +calc_sampler_offsets(nir_deref *tail, nir_tex_instr *instr,
> + unsigned *array_elements, nir_ssa_def **indirect,
> + nir_builder *b, unsigned *location)
> {
> - unsigned location;
> - if (!shader_program->UniformHash->get(location, name)) {
> - assert(!"failed to find sampler");
> - return 0;
> - }
> + if (tail->child != NULL) {
> + switch (tail->child->deref_type) {
> + case nir_deref_type_array: {
> + nir_deref_array *deref_array = nir_deref_as_array(tail->child);
>
> - if (!shader_program->UniformStorage[location].sampler[stage].active) {
> - assert(!"cannot return a sampler");
> - return 0;
> - }
> + assert(deref_array->deref_array_type != nir_deref_array_type_wildcard);
> +
> + calc_sampler_offsets(tail->child, instr, array_elements,
> + indirect, b, location);
> + instr->sampler_index += deref_array->base_offset * *array_elements;
> +
> + if (deref_array->deref_array_type == nir_deref_array_type_indirect) {
> + nir_ssa_def *mul =
> + nir_imul(b, nir_imm_int(b, *array_elements),
> + nir_ssa_for_src(b, deref_array->indirect, 1));
> +
> + nir_instr_rewrite_src(&instr->instr, &deref_array->indirect,
> + NIR_SRC_INIT);
> +
> + if (*indirect) {
> + *indirect = nir_iadd(b, *indirect, mul);
> + } else {
> + *indirect = mul;
> + }
> + }
> +
> + *array_elements *= glsl_get_length(tail->type);
> + break;
> + }
> +
> + case nir_deref_type_struct: {
> + nir_deref_struct *deref_struct = nir_deref_as_struct(tail->child);
> + *location += tail->type->record_location_offset(deref_struct->index);
> + calc_sampler_offsets(tail->child, instr, array_elements,
> + indirect, b, location);
> + break;
> + }
>
> - return shader_program->UniformStorage[location].sampler[stage].index;
> + default:
> + unreachable("Invalid deref type");
> + break;
> + }
> + } else {
> + return;
Why don't we just do "if (tail->child == NULL) return;" at the top?
That way you don't have to go all the way down here to figure out what
we do in the else case.
> + }
> }
>
> static void
> lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_program,
> - gl_shader_stage stage, void *mem_ctx)
> + gl_shader_stage stage, nir_builder *builder)
> {
> 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);
> + unsigned location = instr->sampler->var->data.location;
>
> - for (nir_deref *deref = &instr->sampler->deref;
> - deref->child; deref = deref->child) {
> - switch (deref->child->deref_type) {
> - case nir_deref_type_array: {
> - nir_deref_array *deref_array = nir_deref_as_array(deref->child);
> + if (instr->sampler->deref.child) {
The calc_sampler_offsets() function does this check for you and
becomes a no-op if cuild == NULL so there's no real reason to check it
twice.
Other than my few trivial comments here and the stuff about reworking
the structure position function, this is looking really good! Don't
bother and send a whole v5; just resend the stuff for 5 and 6 (which
may now be one patch).
--Jason
> + unsigned array_elements = 1;
> + nir_ssa_def *indirect = NULL;
>
> - assert(deref_array->deref_array_type != nir_deref_array_type_wildcard);
> + builder->cursor = nir_before_instr(&instr->instr);
> + calc_sampler_offsets(&instr->sampler->deref, instr, &array_elements,
> + &indirect, builder, &location);
>
> - 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;
> - }
> + if (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);
>
> - /* 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);
> - }
> + 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;
> + 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,
> + /* 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_rewrite_src(&instr->instr,
> &instr->src[instr->num_srcs - 1].src,
> - &deref_array->indirect);
> + nir_src_for_ssa(indirect));
>
> - instr->sampler_array_size = glsl_get_length(deref->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);
> - break;
> + instr->sampler_array_size = array_elements;
> }
> + }
>
> - default:
> - unreachable("Invalid deref type");
> - break;
> - }
> + 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 +163,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 +176,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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list