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

Alejandro PiƱeiro apinheiro at igalia.com
Sat Aug 11 11:13:04 UTC 2018


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
+    */
+   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