Commit 6981069f authored by Kenneth Graunke's avatar Kenneth Graunke

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: f003859f nir: Make gl_nir_lower_samplers use gl_nir_lower_samplers_as_deref
Reviewed-by: Jason Ekstrand's avatarJason Ekstrand <>
parent 47303b46
Pipeline #32464 passed with stages
in 11 minutes and 19 seconds
......@@ -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,
......@@ -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())
if (var->type->without_array()->is_image()) {
brw_setup_image_uniform_values(var, stage_prog_data);
/* 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->>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() ||
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) *
......@@ -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);
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment