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 <jason@jlekstrand.net>

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index 66581b21f61fb947dac1d26e5bebb02c2f25c795..95d87dc56fd3125f7bb619acd9c5190d86e2a087 100644 (file)
--- 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 f608d7284021a6bfefd9d8ad380b19ebe4dad747..9252d30a557e7a148297e5dbb72cf0b808aed155 100644 (file)
--- 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 b8681fe579e4f7f3377a3b584a8117e964c4af00..defb465a2e2167b1c7823a279e13f9826501c464 100644 (file)
--- 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);