[Mesa-dev] [RFCv3 04/11] nir: allow pre-resolved sampler uniform locations

[Rob Clark]

So I've been looking a bit more at this, and skipping the
shader_program->UniformStorage[location].opaque[stage].index offset
is, I think, not the right thing to do.

The original issue I was having with this seems to be a disagreement
about location idx into UniformStorage.  In
parcel_out_uniform_storage::handle_samplers we end up w/
&prog->UniformStorage[id] where id comes from UniformHash, but id
doesn't match up with var->data.location in NIR (at least in some
cases, seems to work sometimes but that might just be a happy
accident, ie. when both just happen to be zero).

BR,
-R

On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark <robdclark at gmail.com> wrote:
> From: Rob Clark <robclark at freedesktop.org>
>
> With TGSI, the ir_variable::data.location gets fixed up to be a stage
> local location (rather than program global).  In this case we need to
> skip the UniformStorage[location] lookup.
>
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
>  src/compiler/nir/nir_lower_samplers.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/src/compiler/nir/nir_lower_samplers.c b/src/compiler/nir/nir_lower_samplers.c
> index 96e8291..c95a474 100644
> --- a/src/compiler/nir/nir_lower_samplers.c
> +++ b/src/compiler/nir/nir_lower_samplers.c
> @@ -129,14 +129,18 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>        instr->sampler_array_size = array_elements;
>     }
>
> -   if (location > shader_program->NumUniformStorage - 1 ||
> -       !shader_program->UniformStorage[location].opaque[stage].active) {
> -      assert(!"cannot return a sampler");
> -      return;
> -   }
> +   if (!shader_program) {
> +      instr->sampler_index += location;
> +   } else {
> +      if (location > shader_program->NumUniformStorage - 1 ||
> +          !shader_program->UniformStorage[location].opaque[stage].active) {
> +         assert(!"cannot return a sampler");
> +         return;
> +      }
>
> -   instr->sampler_index +=
> -      shader_program->UniformStorage[location].opaque[stage].index;
> +      instr->sampler_index +=
> +         shader_program->UniformStorage[location].opaque[stage].index;
> +   }
>
>     instr->sampler = NULL;
>  }
> @@ -176,6 +180,11 @@ lower_impl(nir_function_impl *impl, const struct gl_shader_program *shader_progr
>     nir_foreach_block(impl, lower_block_cb, &state);
>  }
>
> +/* Call with a null 'shader_program' if uniform locations are
> + * already local to the shader, ie. skipping the
> + * shader_program->UniformStorage[location].opaque[stage].index
> + * lookup
> + */
>  void
>  nir_lower_samplers(nir_shader *shader,
>                     const struct gl_shader_program *shader_program)
> --
> 2.5.0
>