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

[Alejandro Piñeiro]

Hi Jason, I'm CCing you for this one as you basically provided the
pseudocode I used as reference. So I assume that you already know what's
going on, hoping that would make the review easier.


On 09/08/18 15:43, Alejandro Piñeiro 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);
> +   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;
> +   }
>  
> -   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;
>  }
>