From 6c1c13e90e67c716ff97ba8c45a5a04c2b57b4a2 Mon Sep 17 00:00:00 2001 From: "Kristian H. Kristensen" Date: Wed, 13 Nov 2019 14:49:53 -0800 Subject: [PATCH] st/mesa: Lower vars to ssa and constant prop before gl_nir_lower_buffers MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Jason Ekstrand Reviewed-by: Marek Olšák Reviewed-by: Timothy Arceri --- src/mesa/state_tracker/st_glsl_to_nir.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index a277c0eef31..3663410ae1b 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -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 -- 2.30.2