[Mesa-dev] [PATCH 05/10] nir/lower_samplers: don't assume a deref for both texture and sampler srcs

[Jason Ekstrand]

On August 9, 2018 07:44:51 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
> ---
> src/compiler/glsl/gl_nir_lower_samplers.c | 83 ++++++++++++++++++++-----------
> 1 file changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/src/compiler/glsl/gl_nir_lower_samplers.c 
> b/src/compiler/glsl/gl_nir_lower_samplers.c
> index 43fe318a835..1b50b10d345 100644
> --- a/src/compiler/glsl/gl_nir_lower_samplers.c
> +++ b/src/compiler/glsl/gl_nir_lower_samplers.c
> @@ -103,48 +103,75 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
>       shader_program->data->UniformStorage[location].opaque[stage].index;
> }
>
> +static void
> +lower_tex_src_to_offset(nir_builder *b,
> +                        nir_tex_instr *instr, unsigned src_idx,
> +                        unsigned *index, unsigned *array_size,
> +                        const struct gl_shader_program *shader_program)
> +{
> +   nir_ssa_def *indirect;
> +   unsigned base_offset, array_elements;
> +   nir_tex_src *src = &instr->src[src_idx];
> +   bool is_sampler = src->src_type == nir_tex_src_sampler_deref;
> +
> +   calc_sampler_offsets(b, src->src.ssa, shader_program, &base_offset,
> +                        &indirect, &array_elements);

We can likely roll lower_tex_src_to_offset and calc_sampler_offsets 
together.  May make things a bit better.

> +   if (indirect) {
> +      nir_instr_rewrite_src(&instr->instr, &src->src,
> +                            nir_src_for_ssa(indirect));
> +
> +      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 (index)
> +      *index = base_offset;
> +
> +   if (array_size)
> +      *array_size = array_elements;
> +}
> +
> static bool
> lower_sampler(nir_builder *b, nir_tex_instr *instr,
>               const struct gl_shader_program *shader_program)
> {
>    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;
> +   if (texture_idx >= 0) {
> +      unsigned texture_index;
> +      unsigned texture_array_size;
>
> -   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);
> +      b->cursor = nir_before_instr(&instr->instr);
>
> -   b->cursor = nir_before_instr(&instr->instr);
> +      lower_tex_src_to_offset(b, instr, texture_idx,
> +                              &texture_index, &texture_array_size,
> +                              shader_program);
>
> -   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);
> +      instr->texture_index = texture_index;
> +      instr->texture_array_size = texture_array_size;

Can we pass these into lower_tex_src_to_offset directly?

> +   }
>
> -   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) {
> +      unsigned sampler_index;
> +
> +      lower_tex_src_to_offset(b, instr, sampler_idx,
> +                              &sampler_index, NULL,
> +                              shader_program);
> +      instr->sampler_index = sampler_index;
>    }
>
> +   if (texture_idx < 0 && sampler_idx < 0)
> +      return false;
> +
>    return true;
> }
>
> --
> 2.14.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev