st/mesa: Lower vars to ssa and constant prop before gl_nir_lower_buffers
authorKristian H. Kristensen <hoegsberg@google.com>
Wed, 13 Nov 2019 22:49:53 +0000 (14:49 -0800)
committerKristian H. Kristensen <hoegsberg@google.com>
Mon, 6 Jan 2020 21:01:19 +0000 (13:01 -0800)
The gl_nir_lower_buffers pass relies on recognizing the same literal
constants as the GLSL compiler so that constant buffer array indices
are constant in nir as well.  Without this, get_block_array_index()
would see

  vec1 32 ssa_723 = deref_var &const_temp@1 (function_temp int)
  vec1 32 ssa_724 = load_const (0x00000001 /* 0.000000 */)
  ...
  vec1 32 ssa_5 = deref_var &const_temp@1 (function_temp int)
  vec1 32 ssa_6 = intrinsic load_deref (ssa_5) (0) /* access=0 */
  vec1 32 ssa_7 = deref_var &blockB (ssbo BlockB[1])
  vec1 32 ssa_8 = deref_array &(*ssa_7)[ssa_6] (ssbo BlockB) /* &blockB[ssa_6] */

instead of a literal 1, and ultimately generate the block name
BlockB[0].  That used to work, since we before the previous commits
we'd compact the block binding points and names. Thus, there would
always be a BlockB[0].

Now, if an entry in a block array isn't used, we don't generate that
block name, which means that if entry 0 isn't used BlockB[0] isn't
present and then get_block_array_index() fails to find the block.

In most cases we would have dealt with this in the call to
st_nir_opts() in st_nir_link_shaders(), but in the num_shaders == 1
case (for example, compute) we would call gl_nir_lower_buffers()
before we lowered GLSL constants.  Move that corner case up next to
where we call st_nir_link_shaders() so we call st_nir_opts() at the
same point in the flow for all shaders.

Fixes: dEQP-GLES31.functional.ssbo.layout.random.all_per_block_buffers.18
Signed-off-by: Kristian H. Kristensen <hoegsberg@google.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
src/mesa/state_tracker/st_glsl_to_nir.cpp

index a277c0eef3177929eb37f5901d78596d58ce20aa..3663410ae1b71c711150710d184cdba8d42f5f25 100644 (file)
@@ -729,6 +729,12 @@ st_link_nir(struct gl_context *ctx,
       st_nir_link_shaders(linked_shader[i]->Program->nir,
                           linked_shader[i + 1]->Program->nir);
    }
+   /* Linking shaders also optimizes them. Separate shaders, compute shaders
+    * and shaders with a fixed-func VS or FS that don't need linking are
+    * optimized here.
+    */
+   if (num_shaders == 1)
+      st_nir_opts(linked_shader[0]->Program->nir);
 
    if (!shader_program->data->spirv)
       nir_build_program_resource_list(ctx, shader_program, false);
@@ -737,14 +743,11 @@ st_link_nir(struct gl_context *ctx,
       struct gl_linked_shader *shader = linked_shader[i];
       nir_shader *nir = shader->Program->nir;
 
+      /* This needs to run after the initial pass of nir_lower_vars_to_ssa, so
+       * that the buffer indices are constants in nir where they where
+       * constants in GLSL. */
       NIR_PASS_V(nir, gl_nir_lower_buffers, shader_program);
 
-      /* Linked shaders are optimized in st_nir_link_shaders. Separate shaders
-       * and shaders with a fixed-func VS or FS are optimized here.
-       */
-      if (num_shaders == 1)
-         st_nir_opts(nir);
-
       /* Remap the locations to slots so those requiring two slots will occupy
        * two locations. For instance, if we have in the IR code a dvec3 attr0 in
        * location 0 and vec4 attr1 in location 1, in NIR attr0 will use