From 6981069fc805da1afc867ca3c905075d146d7ff9 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 17 Apr 2019 17:25:29 -0700 Subject: [PATCH] i965: Ignore uniform storage for samplers or images, use binding info gl_nir_lower_samplers_as_deref creates new top level sampler and image uniforms which have been split from structure uniforms. i965 assumed that it could walk through gl_uniform_storage slots by starting at var->data.location and walking forward based on a simple slot count. This assumed that structure types were walked in a particular order. With samplers and images split out of structures, it becomes impossible to assign meaningful locations. Consider: struct S { sampler2D a; sampler2D b; } s[2]; The gl_uniform_storage locations for these follow this map: 0 => a[0], 1 => b[0], 2 => a[0], 3 => b[0]. But the new split variables look like: sampler2D lowered_a[2]; sampler2D lowered_b[2]; and there is no way to know that there's effectively a stride to get to the location for successive elements of a[] or b[]. So, working with location becomes effectively impossible. Ultimately, the point of looking at uniform storage was to pull out the bindings from the opaque index fields. gl_nir_lower_samplers_as_derefs can obtain this information while doing the splitting, however, and sets up var->data.binding to have the desired values. We move gl_nir_lower_samplers before brw_nir_lower_image_load_store so gl_nir_lower_samplers_as_derefs has the opportunity to set proper image bindings. Then, we make the uniform handling code skip sampler(-array) variables, and handle image param setup based on var->data.binding. Fixes Piglit tests/spec/glsl-1.10/execution/samplers/uniform-struct, this time without regressing dEQP-GLES2.functional.uniform_api.random.3. Fixes: f003859f97c nir: Make gl_nir_lower_samplers use gl_nir_lower_samplers_as_deref Reviewed-by: Jason Ekstrand --- src/mesa/drivers/dri/i965/brw_link.cpp | 3 -- .../drivers/dri/i965/brw_nir_uniforms.cpp | 38 ++++++++++++------- src/mesa/drivers/dri/i965/brw_program.c | 5 ++- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index 66581b21f61..95d87dc56fd 100644 --- a/src/mesa/drivers/dri/i965/brw_link.cpp +++ b/src/mesa/drivers/dri/i965/brw_link.cpp @@ -323,9 +323,6 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg) brw_shader_gather_info(prog->nir, prog); - NIR_PASS_V(prog->nir, gl_nir_lower_samplers, shProg); - prog->info.textures_used = prog->nir->info.textures_used; - prog->info.textures_used_by_txf = prog->nir->info.textures_used_by_txf; NIR_PASS_V(prog->nir, gl_nir_lower_atomics, shProg, false); NIR_PASS_V(prog->nir, nir_lower_atomics_to_ssbo, prog->nir->info.num_abos); diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp index f608d728402..9252d30a557 100644 --- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp @@ -80,15 +80,15 @@ setup_vec4_image_param(uint32_t *params, uint32_t idx, } static void -brw_setup_image_uniform_values(gl_shader_stage stage, - struct brw_stage_prog_data *stage_prog_data, - unsigned param_start_index, - const gl_uniform_storage *storage) +brw_setup_image_uniform_values(nir_variable *var, + struct brw_stage_prog_data *prog_data) { - uint32_t *param = &stage_prog_data->param[param_start_index]; + unsigned param_start_index = var->data.driver_location / 4; + uint32_t *param = &prog_data->param[param_start_index]; + unsigned num_images = MAX2(1, var->type->arrays_of_arrays_size()); - for (unsigned i = 0; i < MAX2(storage->array_elements, 1); i++) { - const unsigned image_idx = storage->opaque[stage].index + i; + for (unsigned i = 0; i < num_images; i++) { + const unsigned image_idx = var->data.binding + i; /* Upload the brw_image_param structure. The order is expected to match * the BRW_IMAGE_PARAM_*_OFFSET defines. @@ -150,6 +150,14 @@ brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var, struct brw_stage_prog_data *stage_prog_data, bool is_scalar) { + if (var->type->without_array()->is_sampler()) + return; + + if (var->type->without_array()->is_image()) { + brw_setup_image_uniform_values(var, stage_prog_data); + return; + } + /* The data for our (non-builtin) uniforms is stored in a series of * gl_uniform_storage structs for each subcomponent that * glGetUniformLocation() could name. We know it's been set up in the same @@ -162,15 +170,17 @@ brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var, struct gl_uniform_storage *storage = &prog->sh.data->UniformStorage[var->data.location + u]; - if (storage->builtin || storage->type->is_sampler()) + /* We already handled samplers and images via the separate top-level + * variables created by gl_nir_lower_samplers_as_deref(), but they're + * still part of the structure's storage, and so we'll see them while + * walking it to set up the other regular fields. Just skip over them. + */ + if (storage->builtin || + storage->type->is_sampler() || + storage->type->is_image()) continue; - if (storage->type->is_image()) { - brw_setup_image_uniform_values(stage, stage_prog_data, - uniform_index, storage); - uniform_index += - BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1); - } else { + { gl_constant_value *components = storage->storage; unsigned vector_count = (MAX2(storage->array_elements, 1) * storage->type->matrix_columns); diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c index b8681fe579e..defb465a2e2 100644 --- a/src/mesa/drivers/dri/i965/brw_program.c +++ b/src/mesa/drivers/dri/i965/brw_program.c @@ -105,7 +105,6 @@ brw_create_nir(struct brw_context *brw, } else { nir = prog_to_nir(prog, options); NIR_PASS_V(nir, nir_lower_regs_to_ssa); /* turn registers into SSA */ - NIR_PASS_V(nir, gl_nir_lower_samplers, NULL); } nir_validate_shader(nir, "before brw_preprocess_nir"); @@ -120,6 +119,10 @@ brw_create_nir(struct brw_context *brw, nir = brw_preprocess_nir(brw->screen->compiler, nir, softfp64); + NIR_PASS_V(nir, gl_nir_lower_samplers, shader_prog); + prog->info.textures_used = nir->info.textures_used; + prog->info.textures_used_by_txf = nir->info.textures_used_by_txf; + NIR_PASS_V(nir, brw_nir_lower_image_load_store, devinfo); NIR_PASS_V(nir, gl_nir_lower_buffers, shader_prog); -- 2.30.2