[Mesa-dev] [PATCH] nir/lower_samplers: don't assume a deref for both texture and sampler srcs
Jason Ekstrand
jason at jlekstrand.net
Sat Aug 11 13:03:47 UTC 2018
On August 11, 2018 06:13:17 Alejandro Piñeiro <apinheiro at igalia.com> wrote:
> After commit "nir: Use derefs in nir_lower_samplers"
> (75286c2d083cdbdfb202a93349e567df0441d5f7) assumes one deref for both
> the texture and the sampler. However there are cases (on OpenGL, using
> ARB_gl_spirv) where SPIR-V is not providing a sampler, like for
> texture query levels ops. Although we could make spirv_to_nir to
> provide a sampler deref for those cases, it is not really needed, and
> wrong from the Vulkan point of view.
>
> This patch fixes the following (borrowed) tests run on SPIR-V mode:
> arb_compute_shader/execution/basic-texelFetch.shader_test
> arb_gpu_shader5/execution/sampler_array_indexing/fs-simple-texture-size.shader_test
> arb_texture_query_levels/execution/fs-baselevel.shader_test
> arb_texture_query_levels/execution/fs-maxlevel.shader_test
> arb_texture_query_levels/execution/fs-miptree.shader_test
> arb_texture_query_levels/execution/fs-nomips.shader_test
> arb_texture_query_levels/execution/vs-baselevel.shader_test
> arb_texture_query_levels/execution/vs-maxlevel.shader_test
> arb_texture_query_levels/execution/vs-miptree.shader_test
> arb_texture_query_levels/execution/vs-nomips.shader_test
> glsl-1.30/execution/fs-textureSize-compare.shader_test
>
> v2: merge lower_tex_src_to_offset and calc_sampler_offsets together,
> update texture/sampler index and texture_array_size directly on
> lower_tex_src_to_offset (Jason)
> ---
> src/compiler/glsl/gl_nir_lower_samplers.c | 111 ++++++++++++++++--------------
> 1 file changed, 58 insertions(+), 53 deletions(-)
>
> diff --git a/src/compiler/glsl/gl_nir_lower_samplers.c
> b/src/compiler/glsl/gl_nir_lower_samplers.c
> index 43fe318a835..e001792d689 100644
> --- a/src/compiler/glsl/gl_nir_lower_samplers.c
> +++ b/src/compiler/glsl/gl_nir_lower_samplers.c
> @@ -31,21 +31,20 @@
> #include "main/compiler.h"
> #include "main/mtypes.h"
>
> -/* Calculate the sampler index based on array indicies and also
> - * calculate the base uniform location for struct members.
> - */
> static void
> -calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
> - const struct gl_shader_program *shader_program,
> - unsigned *base_index, nir_ssa_def **index,
> - unsigned *array_elements)
> +lower_tex_src_to_offset(nir_builder *b,
> + nir_tex_instr *instr, unsigned src_idx,
> + const struct gl_shader_program *shader_program)
> {
> - *base_index = 0;
> - *index = NULL;
> - *array_elements = 1;
> + nir_ssa_def *index = NULL;
> + unsigned base_index = 0;
> + unsigned array_elements = 1;
> + nir_tex_src *src = &instr->src[src_idx];
> + bool is_sampler = src->src_type == nir_tex_src_sampler_deref;
> unsigned location = 0;
>
> - nir_deref_instr *deref = nir_instr_as_deref(ptr->parent_instr);
> + /* We compute first the offsets */
> + nir_deref_instr *deref = nir_instr_as_deref(src->src.ssa->parent_instr);
> while (deref->deref_type != nir_deref_type_var) {
> assert(deref->parent.is_ssa);
> nir_deref_instr *parent =
> @@ -61,22 +60,22 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
> nir_const_value *const_deref_index =
> nir_src_as_const_value(deref->arr.index);
>
> - if (const_deref_index && *index == NULL) {
> + if (const_deref_index && index == NULL) {
> /* We're still building a direct index */
> - *base_index += const_deref_index->u32[0] * *array_elements;
> + base_index += const_deref_index->u32[0] * array_elements;
> } else {
> - if (*index == NULL) {
> + if (index == NULL) {
> /* We used to be direct but not anymore */
> - *index = nir_imm_int(b, *base_index);
> - *base_index = 0;
> + index = nir_imm_int(b, base_index);
> + base_index = 0;
> }
>
> - *index = nir_iadd(b, *index,
> - nir_imul(b, nir_imm_int(b, *array_elements),
> - nir_ssa_for_src(b, deref->arr.index, 1)));
> + index = nir_iadd(b, index,
> + nir_imul(b, nir_imm_int(b, array_elements),
> + nir_ssa_for_src(b, deref->arr.index,
> 1)));
> }
>
> - *array_elements *= glsl_get_length(parent->type);
> + array_elements *= glsl_get_length(parent->type);
> break;
> }
>
> @@ -87,8 +86,8 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
> deref = parent;
> }
>
> - if (*index)
> - *index = nir_umin(b, *index, nir_imm_int(b, *array_elements - 1));
> + if (index)
> + index = nir_umin(b, index, nir_imm_int(b, array_elements - 1));
>
> /* We hit the deref_var. This is the end of the line */
> assert(deref->deref_type == nir_deref_type_var);
> @@ -99,8 +98,31 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
> assert(location < shader_program->data->NumUniformStorage &&
> shader_program->data->UniformStorage[location].opaque[stage].active);
>
> - *base_index +=
> + base_index +=
> shader_program->data->UniformStorage[location].opaque[stage].index;
> +
> + /* We have the offsets, we apply them, rewriting/removing instr if
> + * needed
> + */
We're removing it rewriting the source, not the instruction. With that fixed,
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> + if (index) {
> + nir_instr_rewrite_src(&instr->instr, &src->src,
> + nir_src_for_ssa(index));
> +
> + src->src_type = is_sampler ?
> + nir_tex_src_sampler_offset :
> + nir_tex_src_texture_offset;
> +
> + instr->texture_array_size = array_elements;
> + } else {
> + nir_tex_instr_remove_src(instr, src_idx);
> + }
> +
> + if (is_sampler) {
> + instr->sampler_index = base_index;
> + } else {
> + instr->texture_index = base_index;
> + instr->texture_array_size = array_elements;
> + }
> }
>
> static bool
> @@ -109,42 +131,25 @@ lower_sampler(nir_builder *b, nir_tex_instr *instr,
> {
> int texture_idx =
> nir_tex_instr_src_index(instr, nir_tex_src_texture_deref);
> - int sampler_idx =
> - nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
> -
> - if (texture_idx < 0)
> - return false;
>
> - assert(texture_idx >= 0 && sampler_idx >= 0);
> - assert(instr->src[texture_idx].src.is_ssa);
> - assert(instr->src[sampler_idx].src.is_ssa);
> - assert(instr->src[texture_idx].src.ssa == instr->src[sampler_idx].src.ssa);
> + if (texture_idx >= 0) {
> + b->cursor = nir_before_instr(&instr->instr);
>
> - b->cursor = nir_before_instr(&instr->instr);
> -
> - unsigned base_offset, array_elements;
> - nir_ssa_def *indirect;
> - calc_sampler_offsets(b, instr->src[texture_idx].src.ssa, shader_program,
> - &base_offset, &indirect, &array_elements);
> + lower_tex_src_to_offset(b, instr, texture_idx,
> + shader_program);
> + }
>
> - instr->texture_index = base_offset;
> - instr->sampler_index = base_offset;
> - if (indirect) {
> - nir_instr_rewrite_src(&instr->instr, &instr->src[texture_idx].src,
> - nir_src_for_ssa(indirect));
> - instr->src[texture_idx].src_type = nir_tex_src_texture_offset;
> - nir_instr_rewrite_src(&instr->instr, &instr->src[sampler_idx].src,
> - nir_src_for_ssa(indirect));
> - instr->src[sampler_idx].src_type = nir_tex_src_sampler_offset;
> + int sampler_idx =
> + nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
>
> - instr->texture_array_size = array_elements;
> - } else {
> - nir_tex_instr_remove_src(instr, texture_idx);
> - /* The sampler index may have changed */
> - sampler_idx = nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
> - nir_tex_instr_remove_src(instr, sampler_idx);
> + if (sampler_idx >= 0) {
> + lower_tex_src_to_offset(b, instr, sampler_idx,
> + shader_program);
> }
>
> + if (texture_idx < 0 && sampler_idx < 0)
> + return false;
> +
> return true;
> }
>
> --
> 2.14.1
More information about the mesa-dev
mailing list